servo: Merge #18638 - style: Lazily tweak the traversal root to account for sibling invalidations (from emilio:invalidation-lazy); r=heycam

Bug: 1403078
Reviewed-by: heycam
MozReview-Commit-ID: Ij3nMOKu5FO
Signed-off-by: Emilio Cobos Álvarez <emilio@crisal.io>

Source-Repo: https://github.com/servo/servo
Source-Revision: 3dbb97922d75f7ba4c542cb67e9c30c28c7a0699

--HG--
extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear
extra : subtree_revision : beada9b873383acc78c6f5365f68c34dd3e01f39

servo/components/layout_thread/lib.rs

@@ -1268,15 +1268,11 @@ impl LayoutThread {
             None
         };
 
-        // NB: Type inference falls apart here for some reason, so we need to be very verbose. :-(
         let traversal = RecalcStyleAndConstructFlows::new(layout_context);
         let token = {
-            let context = <RecalcStyleAndConstructFlows as
-                           DomTraversal<ServoLayoutElement>>::shared_context(&traversal);
-            <RecalcStyleAndConstructFlows as
-             DomTraversal<ServoLayoutElement>>::pre_traverse(element,
-                                                             context,
-                                                             TraversalFlags::empty())
+            let shared =
+                <RecalcStyleAndConstructFlows as DomTraversal<ServoLayoutElement>>::shared_context(&traversal);
+            RecalcStyleAndConstructFlows::pre_traverse(element, shared)
         };
 
         if token.should_traverse() {
@@ -1287,7 +1283,10 @@ impl LayoutThread {
                     || {
                 // Perform CSS selector matching and flow construction.
                 driver::traverse_dom::<ServoLayoutElement, RecalcStyleAndConstructFlows>(
-                    &traversal, element, token, thread_pool);
+                    &traversal,
+                    token,
+                    thread_pool,
+                );
             });
             // TODO(pcwalton): Measure energy usage of text shaping, perhaps?
             let text_shaping_time =

servo/components/style/driver.rs

@@ -27,15 +27,15 @@ use traversal::{DomTraversal, PerLevelTraversalData, PreTraverseToken};
 /// then transfer control over to the parallel traversal.
 pub fn traverse_dom<E, D>(
     traversal: &D,
-    root: E,
-    token: PreTraverseToken,
+    token: PreTraverseToken<E>,
     pool: Option<&rayon::ThreadPool>
 )
 where
     E: TElement,
     D: DomTraversal<E>,
 {
-    debug_assert!(token.should_traverse());
+    let root =
+        token.traversal_root().expect("Should've ensured we needed to traverse");
 
     let dump_stats = traversal.shared_context().options.dump_style_statistics;
     let start_time = if dump_stats { Some(time::precise_time_s()) } else { None };

servo/components/style/gecko/wrapper.rs

@@ -488,23 +488,6 @@ impl<'le> GeckoElement<'le> {
         self.flags() & (NODE_NEEDS_FRAME as u32) != 0
     }
 
-    /// Returns true if a traversal starting from this element requires a post-traversal.
-    pub fn needs_post_traversal(&self) -> bool {
-        debug!("needs_post_traversal: dd={}, aodd={}, lfcd={}, lfc={}, data={:?}",
-               self.has_dirty_descendants(),
-               self.has_animation_only_dirty_descendants(),
-               self.descendants_need_frames(),
-               self.needs_frame(),
-               self.borrow_data().unwrap());
-
-        let has_flag =
-            self.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 |
-                            NODE_NEEDS_FRAME as u32) != 0;
-        has_flag || self.borrow_data().unwrap().contains_restyle_data()
-    }
-
     /// Returns true if this element has a shadow root.
     fn has_shadow_root(&self) -> bool {
         self.get_extended_slots().map_or(false, |slots| !slots.mShadowRoot.mRawPtr.is_null())

servo/components/style/traversal.rs

@@ -34,10 +34,17 @@ pub struct PerLevelTraversalData {
 
 /// We use this structure, rather than just returning a boolean from pre_traverse,
 /// to enfore that callers process root invalidations before starting the traversal.
-pub struct PreTraverseToken(bool);
-impl PreTraverseToken {
+pub struct PreTraverseToken<E: TElement>(Option<E>);
+impl<E: TElement> PreTraverseToken<E> {
     /// Whether we should traverse children.
-    pub fn should_traverse(&self) -> bool { self.0 }
+    pub fn should_traverse(&self) -> bool {
+        self.0.is_some()
+    }
+
+    /// Returns the traversal root for the current traversal.
+    pub(crate) fn traversal_root(self) -> Option<E> {
+        self.0
+    }
 }
 
 #[cfg(feature = "servo")]
@@ -139,36 +146,48 @@ pub trait DomTraversal<E: TElement> : Sync {
     fn pre_traverse(
         root: E,
         shared_context: &SharedStyleContext,
-        traversal_flags: TraversalFlags,
-    ) -> PreTraverseToken {
+    ) -> PreTraverseToken<E> {
+        let traversal_flags = shared_context.traversal_flags;
+
         // If this is an unstyled-only traversal, the caller has already verified
         // that there's something to traverse, and we don't need to do any
         // invalidation since we're not doing any restyling.
         if traversal_flags.contains(traversal_flags::UnstyledOnly) {
-            return PreTraverseToken(true)
+            return PreTraverseToken(Some(root))
         }
 
-        let flags = shared_context.traversal_flags;
         let mut data = root.mutate_data();
         let mut data = data.as_mut().map(|d| &mut **d);
-
-        if let Some(ref mut data) = data {
-            // Invalidate our style, and the one of our siblings and descendants
-            // as needed.
-            data.invalidate_style_if_needed(root, shared_context, None);
-
-            // Make sure we don't have any stale RECONSTRUCTED_ANCESTOR bits from
-            // the last traversal (at a potentially-higher root). From the
-            // perspective of this traversal, the root cannot have reconstructed
-            // ancestors.
-            data.set_reconstructed_ancestor(false);
-        };
-
         let parent = root.traversal_parent();
         let parent_data = parent.as_ref().and_then(|p| p.borrow_data());
+
+        if let Some(ref mut data) = data {
+            // Make sure we don't have any stale RECONSTRUCTED_ANCESTOR bits
+            // from the last traversal (at a potentially-higher root).
+            //
+            // From the perspective of this traversal, the root cannot have
+            // reconstructed ancestors.
+            data.set_reconstructed_ancestor(false);
+
+            if !traversal_flags.for_animation_only() {
+                // Invalidate our style, and that of our siblings and
+                // descendants as needed.
+                let invalidation_result =
+                    data.invalidate_style_if_needed(root, shared_context, None);
+
+                if invalidation_result.has_invalidated_siblings() {
+                    let actual_root =
+                        parent.expect("How in the world can you invalidate \
+                                       siblings without a parent?");
+                    unsafe { actual_root.set_dirty_descendants() }
+                    return PreTraverseToken(Some(actual_root));
+                }
+            }
+        }
+
         let should_traverse = Self::element_needs_traversal(
             root,
-            flags,
+            traversal_flags,
             data.as_mut().map(|d| &**d),
             parent_data.as_ref().map(|d| &**d)
         );
@@ -176,10 +195,10 @@ pub trait DomTraversal<E: TElement> : Sync {
         // If we're not going to traverse at all, we may need to clear some state
         // off the root (which would normally be done at the end of recalc_style_at).
         if !should_traverse && data.is_some() {
-            clear_state_after_traversing(root, data.unwrap(), flags);
+            clear_state_after_traversing(root, data.unwrap(), traversal_flags);
         }
 
-        PreTraverseToken(should_traverse)
+        PreTraverseToken(if should_traverse { Some(root) } else { None })
     }
 
     /// Returns true if traversal should visit a text node. The style system

servo/ports/geckolib/glue.rs

@@ -235,16 +235,19 @@ fn traverse_subtree(element: GeckoElement,
 
     let global_style_data = &*GLOBAL_STYLE_DATA;
     let guard = global_style_data.shared_lock.read();
-    let shared_style_context = create_shared_context(&global_style_data,
-                                                     &guard,
-                                                     &per_doc_data,
-                                                     traversal_flags,
-                                                     snapshots);
+    let shared_style_context = create_shared_context(
+        &global_style_data,
+        &guard,
+        &per_doc_data,
+        traversal_flags,
+        snapshots,
+    );
 
+    let token = RecalcStyleOnly::pre_traverse(
+        element,
+        &shared_style_context,
+    );
 
-    let token = RecalcStyleOnly::pre_traverse(element,
-                                              &shared_style_context,
-                                              traversal_flags);
     if !token.should_traverse() {
         return;
     }
@@ -259,13 +262,13 @@ fn traverse_subtree(element: GeckoElement,
     };
 
     let traversal = RecalcStyleOnly::new(shared_style_context);
-    driver::traverse_dom(&traversal, element, token, thread_pool);
+    driver::traverse_dom(&traversal, token, thread_pool);
 }
 
 /// Traverses the subtree rooted at `root` for restyling.
 ///
-/// Returns whether a Gecko post-traversal (to perform lazy frame construction,
-/// or consume any RestyleData, or drop any ElementData) is required.
+/// Returns whether the root was restyled. Whether anything else was restyled or
+/// not can be inferred from the dirty bits in the rest of the tree.
 #[no_mangle]
 pub extern "C" fn Servo_TraverseSubtree(
     root: RawGeckoElementBorrowed,
@@ -309,7 +312,9 @@ pub extern "C" fn Servo_TraverseSubtree(
            element.needs_frame(),
            element.borrow_data().unwrap());
 
-    element.needs_post_traversal()
+    let element_was_restyled =
+        element.borrow_data().unwrap().contains_restyle_data();
+    element_was_restyled
 }
 
 /// Checks whether the rule tree has crossed its threshold for unused nodes, and