Bug 1322945 - Change skip_root to unstyled_children_only and use StyleNewChildren in more places. r=heycam

I noticed that our current behavior in ContentRangeInserted is incorrect. Unlike
ContentInserted (where this code lived originally), ContentRangeInserted takes a
start and end element. I'm not sure if we ever take that path for new content that
needs style, but it seemed sketchy. And generally, it seems nice to just always
style new content the same way (though we still need to style NAC by the subtree
root, since it hasn't been attached to the parent yet).

For situations where there is indeed only one unstyled child, the traversal
overhead should be neglible, since we special-case the single-element in
parallel.rs to avoid calling into rayon.

Being more explicit about what we want here also makes us more robust against
the other handful of callpaths that can take us into
nsCSSFrameConstructor::{ContentRangeInserted,ContentAppended}. Currently we
can call StyleNewSubtree on an already-styled element via RecreateFramesForContent,
which triggers an assertion in the servo traversal.

MozReview-Commit-ID: DqCGh90deHH

components/style/build_gecko.rs

@@ -220,7 +220,7 @@ mod bindings {
             "mozilla::ConsumeStyleBehavior",
             "mozilla::LazyComputeBehavior",
             "mozilla::css::SheetParsingMode",
-            "mozilla::SkipRootBehavior",
+            "mozilla::TraversalRootBehavior",
             "mozilla::DisplayItemClip",  // Needed because bindgen generates
                                          // specialization tests for this even
                                          // though it shouldn't.
@@ -444,7 +444,7 @@ mod bindings {
             "ThreadSafePrincipalHolder",
             "ConsumeStyleBehavior",
             "LazyComputeBehavior",
-            "SkipRootBehavior",
+            "TraversalRootBehavior",
             "FontFamilyList",
             "FontFamilyType",
             "ServoElementSnapshot",

components/style/gecko_bindings/bindings.rs

@@ -70,7 +70,7 @@ use gecko_bindings::structs::ThreadSafeURIHolder;
 use gecko_bindings::structs::ThreadSafePrincipalHolder;
 use gecko_bindings::structs::ConsumeStyleBehavior;
 use gecko_bindings::structs::LazyComputeBehavior;
-use gecko_bindings::structs::SkipRootBehavior;
+use gecko_bindings::structs::TraversalRootBehavior;
 use gecko_bindings::structs::FontFamilyList;
 use gecko_bindings::structs::FontFamilyType;
 use gecko_bindings::structs::ServoElementSnapshot;
@@ -1197,7 +1197,7 @@ extern "C" {
 extern "C" {
     pub fn Servo_TraverseSubtree(root: RawGeckoElementBorrowed,
                                  set: RawServoStyleSetBorrowed,
-                                 skip_root: SkipRootBehavior);
+                                 behavior: TraversalRootBehavior);
 }
 extern "C" {
     pub fn Servo_AssertTreeIsClean(root: RawGeckoElementBorrowed);

components/style/gecko_bindings/structs_debug.rs

@@ -2493,7 +2493,7 @@ pub mod root {
         }
         #[repr(i32)]
         #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
-        pub enum SkipRootBehavior { Skip = 0, DontSkip = 1, }
+        pub enum TraversalRootBehavior { Normal = 0, UnstyledChildrenOnly = 1, }
         pub mod a11y {
             #[allow(unused_imports)]
             use self::super::super::super::root;

components/style/gecko_bindings/structs_release.rs

@@ -2475,7 +2475,7 @@ pub mod root {
         }
         #[repr(i32)]
         #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
-        pub enum SkipRootBehavior { Skip = 0, DontSkip = 1, }
+        pub enum TraversalRootBehavior { Normal = 0, UnstyledChildrenOnly = 1, }
         pub mod a11y {
             #[allow(unused_imports)]
             use self::super::super::super::root;

components/style/parallel.rs

@@ -28,13 +28,17 @@ pub fn traverse_dom<N, C>(root: N::ConcreteElement,
         STYLE_SHARING_CACHE_MISSES.store(0, Ordering::SeqCst);
     }
 
-    // Handle root skipping. We don't currently support it in conjunction with
+    // Handle Gecko's eager initial styling. We don't currently support it
-    // bottom-up traversal. If we did, we'd need to put it on the context to make
+    // in conjunction with bottom-up traversal. If we did, we'd need to put
-    // it available to the bottom-up phase.
+    // it on the context to make it available to the bottom-up phase.
-    debug_assert!(!token.should_skip_root() || !C::needs_postorder_traversal());
+    let (nodes, depth) = if token.traverse_unstyled_children_only() {
-    let (nodes, depth) = if token.should_skip_root() {
+        debug_assert!(!C::needs_postorder_traversal());
         let mut children = vec![];
-        C::traverse_children(root, |kid| children.push(kid.to_unsafe()));
+        for kid in root.as_node().children() {
+            if kid.as_element().map_or(false, |el| el.get_data().is_none()) {
+                children.push(kid.to_unsafe());
+            }
+        }
         (children, known_root_dom_depth.map(|x| x + 1))
     } else {
         (vec![root.as_node().to_unsafe()], known_root_dom_depth)

components/style/sequential.rs

@@ -42,8 +42,12 @@ pub fn traverse_dom<N, C>(root: N::ConcreteElement,
     };
     let context = C::new(shared, root.as_node().opaque());
 
-    if token.should_skip_root() {
+    if token.traverse_unstyled_children_only() {
-        C::traverse_children(root, |kid| doit::<N, C>(&context, kid, &mut data));
+        for kid in root.as_node().children() {
+            if kid.as_element().map_or(false, |el| el.get_data().is_none()) {
+                doit::<N, C>(&context, kid, &mut data);
+            }
+        }
     } else {
         doit::<N, C>(&context, root.as_node(), &mut data);
     }

components/style/traversal.rs

@@ -104,7 +104,7 @@ pub struct PerLevelTraversalData {
 /// to pass information from the pre-traversal into the primary traversal.
 pub struct PreTraverseToken {
     traverse: bool,
-    skip_root: bool,
+    unstyled_children_only: bool,
 }
 
 impl PreTraverseToken {
@@ -112,8 +112,8 @@ impl PreTraverseToken {
         self.traverse
     }
 
-    pub fn should_skip_root(&self) -> bool {
+    pub fn traverse_unstyled_children_only(&self) -> bool {
-        self.skip_root
+        self.unstyled_children_only
     }
 }
 
@@ -140,16 +140,16 @@ pub trait DomTraversalContext<N: TNode> {
     /// a traversal is needed. Returns a token that allows the caller to prove
     /// that the call happened.
     ///
-    /// The skip_root parameter is used in Gecko to style newly-appended children
+    /// The unstyled_children_only parameter is used in Gecko to style newly-
-    /// without restyling the parent.
+    /// appended children without restyling the parent.
-    fn pre_traverse(root: N::ConcreteElement, stylist: &Stylist, skip_root: bool)
+    fn pre_traverse(root: N::ConcreteElement, stylist: &Stylist,
+                    unstyled_children_only: bool)
                     -> PreTraverseToken
     {
-        // If we should skip the root, traverse unconditionally.
+        if unstyled_children_only {
-        if skip_root {
             return PreTraverseToken {
                 traverse: true,
-                skip_root: true,
+                unstyled_children_only: true,
             };
         }
 
@@ -168,7 +168,7 @@ pub trait DomTraversalContext<N: TNode> {
 
         PreTraverseToken {
             traverse: Self::node_needs_traversal(root.as_node()),
-            skip_root: false,
+            unstyled_children_only: false,
         }
     }
 

ports/geckolib/glue.rs

@@ -118,7 +118,7 @@ fn create_shared_context(mut per_doc_data: &mut AtomicRefMut<PerDocumentStyleDat
 }
 
 fn traverse_subtree(element: GeckoElement, raw_data: RawServoStyleSetBorrowed,
-                    skip_root: bool) {
+                    unstyled_children_only: bool) {
     // Force the creation of our lazily-constructed initial computed values on
     // the main thread, since it's not safe to call elsewhere.
     //
@@ -139,7 +139,7 @@ fn traverse_subtree(element: GeckoElement, raw_data: RawServoStyleSetBorrowed,
 
     let mut per_doc_data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut();
 
-    let token = RecalcStyleOnly::pre_traverse(element, &per_doc_data.stylist, skip_root);
+    let token = RecalcStyleOnly::pre_traverse(element, &per_doc_data.stylist, unstyled_children_only);
     if !token.should_traverse() {
         error!("Unnecessary call to traverse_subtree");
         return;
@@ -160,10 +160,11 @@ fn traverse_subtree(element: GeckoElement, raw_data: RawServoStyleSetBorrowed,
 #[no_mangle]
 pub extern "C" fn Servo_TraverseSubtree(root: RawGeckoElementBorrowed,
                                         raw_data: RawServoStyleSetBorrowed,
-                                        skip_root: structs::SkipRootBehavior) -> () {
+                                        behavior: structs::TraversalRootBehavior) -> () {
     let element = GeckoElement(root);
     debug!("Servo_TraverseSubtree: {:?}", element);
-    traverse_subtree(element, raw_data, skip_root == structs::SkipRootBehavior::Skip);
+    traverse_subtree(element, raw_data,
+                     behavior == structs::TraversalRootBehavior::UnstyledChildrenOnly);
 }
 
 #[no_mangle]