Bonus fix: Be more robust about clearing descendants bits under displ…

…ay:none subtrees.

MozReview-Commit-ID: 9KQVOpdEjwF

components/style/dom.rs

@@ -513,6 +513,13 @@ pub trait TElement : Eq + PartialEq + Debug + Hash + Sized + Copy + Clone +
     unsafe fn unset_animation_only_dirty_descendants(&self) {
     }
 
+    /// Clear all bits related to dirty descendant.
+    ///
+    /// In Gecko, this corresponds to the regular dirty descendants bit, the
+    /// animation-only dirty descendants bit, and the lazy frame construction
+    /// descendants bit.
+    unsafe fn clear_descendants_bits(&self) { self.unset_dirty_descendants(); }
+
     /// Returns true if this element is a visited link.
     ///
     /// Servo doesn't support visited styles yet.

components/style/gecko/wrapper.rs

@@ -61,6 +61,7 @@ use gecko_bindings::structs::ELEMENT_HAS_ANIMATION_ONLY_DIRTY_DESCENDANTS_FOR_SE
 use gecko_bindings::structs::ELEMENT_HAS_DIRTY_DESCENDANTS_FOR_SERVO;
 use gecko_bindings::structs::ELEMENT_HAS_SNAPSHOT;
 use gecko_bindings::structs::EffectCompositor_CascadeLevel as CascadeLevel;
+use gecko_bindings::structs::NODE_DESCENDANTS_NEED_FRAMES;
 use gecko_bindings::structs::NODE_IS_IN_NATIVE_ANONYMOUS_SUBTREE;
 use gecko_bindings::structs::NODE_IS_NATIVE_ANONYMOUS;
 use gecko_bindings::structs::nsChangeHint;
@@ -1052,6 +1053,12 @@ impl<'le> TElement for GeckoElement<'le> {
         self.unset_flags(ELEMENT_HAS_ANIMATION_ONLY_DIRTY_DESCENDANTS_FOR_SERVO as u32)
     }
 
+    unsafe fn clear_descendants_bits(&self) {
+        self.unset_flags(ELEMENT_HAS_DIRTY_DESCENDANTS_FOR_SERVO as u32 |
+                         ELEMENT_HAS_ANIMATION_ONLY_DIRTY_DESCENDANTS_FOR_SERVO as u32 |
+                         NODE_DESCENDANTS_NEED_FRAMES as u32)
+    }
+
     fn is_visited_link(&self) -> bool {
         use element_state::IN_VISITED_STATE;
         self.get_state().intersects(IN_VISITED_STATE)

components/style/traversal.rs

@@ -597,29 +597,29 @@ where
         data.clear_restyle_flags_and_damage();
     }
 
-    // Optionally clear the descendants bit for the traversal type we're in.
-    if flags.for_animation_only() {
-        if flags.contains(ClearAnimationOnlyDirtyDescendants) {
-            unsafe { element.unset_animation_only_dirty_descendants(); }
-        }
-    } else {
-        // There are two cases when we want to clear the dity descendants bit here
-        // after styling this element. The first case is when we were explicitly
-        // asked to clear the bit by the caller.
-        //
-        // The second case is when this element is the root of a display:none
-        // subtree, even if the style didn't change (since, if the style did change,
+    // Optionally clear the descendants bits.
+    if data.styles.is_display_none() {
+        // When this element is the root of a display:none subtree, we want to clear
+        // the bits even if the style didn't change (since, if the style did change,
         // we'd have already cleared it above).
         //
         // This keeps the tree in a valid state without requiring the DOM to check
         // display:none on the parent when inserting new children (which can be
         // moderately expensive). Instead, DOM implementations can unconditionally
         // set the dirty descendants bit on any styled parent, and let the traversal
         // sort it out.
-        if flags.contains(ClearDirtyDescendants) ||
-           data.styles.is_display_none() {
-            unsafe { element.unset_dirty_descendants(); }
+        //
+        // Note that the NODE_DESCENDANTS_NEED_FRAMES bit should generally only be set
+        // when appending content beneath an element with a frame (i.e. not
+        // display:none), so clearing it here isn't strictly necessary, but good
+        // belt-and-suspenders.
+        unsafe { element.clear_descendants_bits(); }
+    } else if flags.for_animation_only() {
+        if flags.contains(ClearAnimationOnlyDirtyDescendants) {
+            unsafe { element.unset_animation_only_dirty_descendants(); }
         }
+    } else if flags.contains(ClearDirtyDescendants) {
+        unsafe { element.unset_dirty_descendants(); }
     }
 
     context.thread_local.end_element(element);
@@ -849,7 +849,6 @@ where
     }
 
     unsafe {
-        el.unset_dirty_descendants();
-        el.unset_animation_only_dirty_descendants();
+        el.clear_descendants_bits();
     }
 }