Auto merge of #18271 - legnaleurc:propagate_dirty_bits, r=emilio

Propagate dirty bits after invalidation if needed.

<!-- Please describe your changes on the following line: -->

Follow up for [bug 1388298](https://bugzil.la/1388298).

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18271)
<!-- Reviewable:end -->

components/style/data.rs

@@ -6,6 +6,7 @@
 
 use context::{SharedStyleContext, StackLimitChecker};
 use dom::TElement;
+use invalidation::element::invalidator::InvalidationResult;
 use invalidation::element::restyle_hints::RestyleHint;
 use properties::ComputedValues;
 use properties::longhands::display::computed_value as display;
@@ -329,10 +330,10 @@ impl ElementData {
         element: E,
         shared_context: &SharedStyleContext,
         stack_limit_checker: Option<&StackLimitChecker>,
-    ) {
+    ) -> InvalidationResult {
         // In animation-only restyle we shouldn't touch snapshot at all.
         if shared_context.traversal_flags.for_animation_only() {
-            return;
+            return InvalidationResult::empty();
         }
 
         use invalidation::element::invalidator::TreeStyleInvalidator;
@@ -345,17 +346,20 @@ impl ElementData {
                 element.handled_snapshot(),
                 element.implemented_pseudo_element());
 
-        if element.has_snapshot() && !element.handled_snapshot() {
-            let invalidator = TreeStyleInvalidator::new(
-                element,
-                Some(self),
-                shared_context,
-                stack_limit_checker,
-            );
-            invalidator.invalidate();
-            unsafe { element.set_handled_snapshot() }
-            debug_assert!(element.handled_snapshot());
+        if !element.has_snapshot() || element.handled_snapshot() {
+            return InvalidationResult::empty();
         }
+
+        let invalidator = TreeStyleInvalidator::new(
+            element,
+            Some(self),
+            shared_context,
+            stack_limit_checker,
+        );
+        let result = invalidator.invalidate();
+        unsafe { element.set_handled_snapshot() }
+        debug_assert!(element.handled_snapshot());
+        result
     }
 
     /// Returns true if this element has styles.

components/style/invalidation/element/invalidator.rs

@@ -121,7 +121,7 @@ impl fmt::Debug for Invalidation {
 }
 
 /// The result of processing a single invalidation for a given element.
-struct InvalidationResult {
+struct SingleInvalidationResult {
     /// Whether the element itself was invalidated.
     invalidated_self: bool,
     /// Whether the invalidation matched, either invalidating the element or
@@ -129,6 +129,42 @@ struct InvalidationResult {
     matched: bool,
 }
 
+/// The result of a whole invalidation process for a given element.
+pub struct InvalidationResult {
+    /// Whether the element itself was invalidated.
+    invalidated_self: bool,
+    /// Whether the element's descendants were invalidated.
+    invalidated_descendants: bool,
+    /// Whether the element's siblings were invalidated.
+    invalidated_siblings: bool,
+}
+
+impl InvalidationResult {
+    /// Create an emtpy result.
+    pub fn empty() -> Self {
+        Self {
+            invalidated_self: false,
+            invalidated_descendants: false,
+            invalidated_siblings: false,
+        }
+    }
+
+    /// Whether the invalidation has invalidate the element itself.
+    pub fn has_invalidated_self(&self) -> bool {
+        self.invalidated_self
+    }
+
+    /// Whether the invalidation has invalidate desendants.
+    pub fn has_invalidated_descendants(&self) -> bool {
+        self.invalidated_descendants
+    }
+
+    /// Whether the invalidation has invalidate siblings.
+    pub fn has_invalidated_siblings(&self) -> bool {
+        self.invalidated_siblings
+    }
+}
+
 impl<'a, 'b: 'a, E> TreeStyleInvalidator<'a, 'b, E>
     where E: TElement,
 {
@@ -148,7 +184,7 @@ impl<'a, 'b: 'a, E> TreeStyleInvalidator<'a, 'b, E>
     }
 
     /// Perform the invalidation pass.
-    pub fn invalidate(mut self) {
+    pub fn invalidate(mut self) -> InvalidationResult {
         debug!("StyleTreeInvalidator::invalidate({:?})", self.element);
         debug_assert!(self.element.has_snapshot(), "Why bothering?");
         debug_assert!(self.data.is_some(), "How exactly?");
@@ -161,7 +197,7 @@ impl<'a, 'b: 'a, E> TreeStyleInvalidator<'a, 'b, E>
         let snapshot = wrapper.snapshot().expect("has_snapshot lied");
 
         if !snapshot.has_attrs() && state_changes.is_empty() {
-            return;
+            return InvalidationResult::empty();
         }
 
         // If we are sensitive to visitedness and the visited state changed, we
@@ -258,8 +294,10 @@ impl<'a, 'b: 'a, E> TreeStyleInvalidator<'a, 'b, E>
         debug!("Collected invalidations (self: {}): ", invalidated_self);
         debug!(" > descendants: {:?}", descendant_invalidations);
         debug!(" > siblings: {:?}", sibling_invalidations);
-        self.invalidate_descendants(&descendant_invalidations);
-        self.invalidate_siblings(&mut sibling_invalidations);
+        let invalidated_descendants = self.invalidate_descendants(&descendant_invalidations);
+        let invalidated_siblings = self.invalidate_siblings(&mut sibling_invalidations);
+
+        InvalidationResult { invalidated_self, invalidated_descendants, invalidated_siblings }
     }
 
     /// Go through later DOM siblings, invalidating style as needed using the
@@ -591,7 +629,7 @@ impl<'a, 'b: 'a, E> TreeStyleInvalidator<'a, 'b, E>
         descendant_invalidations: &mut InvalidationVector,
         sibling_invalidations: &mut InvalidationVector,
         invalidation_kind: InvalidationKind,
-    ) -> InvalidationResult {
+    ) -> SingleInvalidationResult {
         debug!("TreeStyleInvalidator::process_invalidation({:?}, {:?}, {:?})",
                self.element, invalidation, invalidation_kind);
 
@@ -765,7 +803,7 @@ impl<'a, 'b: 'a, E> TreeStyleInvalidator<'a, 'b, E>
             }
         }
 
-        InvalidationResult { invalidated_self, matched, }
+        SingleInvalidationResult { invalidated_self, matched, }
     }
 }
 

ports/geckolib/glue.rs

@@ -3745,6 +3745,17 @@ pub extern "C" fn Servo_ProcessInvalidations(set: RawServoStyleSetBorrowed,
     let mut data = data.as_mut().map(|d| &mut **d);
 
     if let Some(ref mut data) = data {
-        data.invalidate_style_if_needed(element, &shared_style_context, None);
+        let result = data.invalidate_style_if_needed(element, &shared_style_context, None);
+        if result.has_invalidated_siblings() {
+            let parent = element.traversal_parent().expect("How could we invalidate siblings without a common parent?");
+            unsafe {
+                parent.set_dirty_descendants();
+                bindings::Gecko_NoteDirtySubtreeForInvalidation(parent.0);
+            }
+        } else if result.has_invalidated_descendants() {
+            unsafe { bindings::Gecko_NoteDirtySubtreeForInvalidation(element.0) };
+        } else if result.has_invalidated_self() {
+            unsafe { bindings::Gecko_NoteDirtyElement(element.0) };
+        }
     }
 }