servo: Merge #20229 - style: Separate the XBL and shadow dom styling bits (from emilio:finally); r=xidorn

Bug: 1441022
Reviewed-by: xidorn
MozReview-Commit-ID: 2W0BmZ8wWXg
Source-Repo: https://github.com/servo/servo
Source-Revision: 6272233c50071534ddbab118b64ecdb8fdda7c8a

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

servo/components/layout_thread/dom_wrapper.rs

@@ -74,6 +74,7 @@ use style::selector_parser::{AttrValue as SelectorAttrValue, NonTSPseudoClass, P
 use style::selector_parser::{PseudoElement, SelectorImpl, extended_filtering};
 use style::shared_lock::{SharedRwLock as StyleSharedRwLock, Locked as StyleLocked};
 use style::str::is_whitespace;
+use style::stylist::CascadeData;
 
 pub unsafe fn drop_style_and_layout_data(data: OpaqueStyleAndLayoutData) {
     let ptr = data.ptr.as_ptr() as *mut StyleData;
@@ -166,6 +167,13 @@ impl<'lr> TShadowRoot for ShadowRoot<'lr> {
     fn host(&self) -> ServoLayoutElement<'lr> {
         match self.0 { }
     }
+
+    fn style_data<'a>(&self) -> &'a CascadeData
+    where
+        Self: 'a,
+    {
+        match self.0 { }
+    }
 }
 
 impl<'ln> TNode for ServoLayoutNode<'ln> {

servo/components/style/data.rs

@@ -260,18 +260,16 @@ impl ElementData {
             return InvalidationResult::empty();
         }
 
-        let mut xbl_stylists = SmallVec::<[_; 3]>::new();
-        // FIXME(emilio): This is wrong, needs to account for ::slotted rules
-        // that may apply to elements down the tree.
-        let cut_off_inheritance =
+        let mut non_document_styles = SmallVec::<[_; 3]>::new();
+        let matches_doc_author_rules =
             element.each_applicable_non_document_style_rule_data(|data, quirks_mode| {
-                xbl_stylists.push((data, quirks_mode))
+                non_document_styles.push((data, quirks_mode))
             });
 
         let mut processor = StateAndAttrInvalidationProcessor::new(
             shared_context,
-            &xbl_stylists,
-            cut_off_inheritance,
+            &non_document_styles,
+            matches_doc_author_rules,
             element,
             self,
             nth_index_cache,

servo/components/style/dom.rs

@@ -330,6 +330,11 @@ pub trait TShadowRoot : Sized + Copy + Clone {
 
     /// Get the shadow host that hosts this ShadowRoot.
     fn host(&self) -> <Self::ConcreteNode as TNode>::ConcreteElement;
+
+    /// Get the style data for this ShadowRoot.
+    fn style_data<'a>(&self) -> &'a CascadeData
+    where
+        Self: 'a;
 }
 
 /// The element trait, the main abstraction the style crate acts over.
@@ -760,7 +765,8 @@ pub trait TElement
 
     /// Implements Gecko's `nsBindingManager::WalkRules`.
     ///
-    /// Returns whether to cut off the inheritance.
+    /// Returns whether to cut off the binding inheritance, that is, whether
+    /// document rules should _not_ apply.
     fn each_xbl_cascade_data<'a, F>(&self, _: F) -> bool
     where
         Self: 'a,
@@ -778,15 +784,22 @@ pub trait TElement
         Self: 'a,
         F: FnMut(&'a CascadeData, QuirksMode),
     {
-        let cut_off_inheritance = self.each_xbl_cascade_data(&mut f);
+        let mut doc_rules_apply = !self.each_xbl_cascade_data(&mut f);
+
+        if let Some(shadow) = self.containing_shadow() {
+            doc_rules_apply = false;
+            f(shadow.style_data(), self.as_node().owner_doc().quirks_mode());
+        }
 
         let mut current = self.assigned_slot();
         while let Some(slot) = current {
-            slot.each_xbl_cascade_data(&mut f);
+            // Slots can only have assigned nodes when in a shadow tree.
+            let data = slot.containing_shadow().unwrap().style_data();
+            f(data, self.as_node().owner_doc().quirks_mode());
             current = slot.assigned_slot();
         }
 
-        cut_off_inheritance
+        doc_rules_apply
     }
 
     /// Does a rough (and cheap) check for whether or not transitions might need to be updated that

servo/components/style/gecko/wrapper.rs

@@ -147,6 +147,30 @@ impl<'lr> TShadowRoot for GeckoShadowRoot<'lr> {
     fn host(&self) -> GeckoElement<'lr> {
         GeckoElement(unsafe { &*self.0._base.mHost.mRawPtr })
     }
+
+    #[inline]
+    fn style_data<'a>(&self) -> &'a CascadeData
+    where
+        Self: 'a,
+    {
+        debug_assert!(!self.0.mServoStyles.mPtr.is_null());
+
+        let author_styles = unsafe {
+            &*(self.0.mServoStyles.mPtr
+                as *const structs::RawServoAuthorStyles
+                as *const bindings::RawServoAuthorStyles)
+        };
+
+        let author_styles =
+            AuthorStyles::<GeckoStyleSheet>::from_ffi(author_styles);
+
+        debug_assert!(
+            author_styles.quirks_mode == self.as_node().owner_doc().quirks_mode() ||
+            author_styles.stylesheets.is_empty()
+        );
+
+        &author_styles.data
+    }
 }
 
 /// A simple wrapper over a non-null Gecko node (`nsINode`) pointer.
@@ -1457,26 +1481,6 @@ impl<'le> TElement for GeckoElement<'le> {
         // rule_hash_target, that is, our originating element.
         let mut current = Some(self.rule_hash_target());
         while let Some(element) = current {
-            // TODO(emilio): Deal with Shadow DOM separately than with XBL
-            // (right now we still rely on get_xbl_binding_parent()).
-            //
-            // That will allow to clean up a bunch in
-            // push_applicable_declarations.
-            if let Some(shadow) = element.shadow_root() {
-                debug_assert!(!shadow.0.mServoStyles.mPtr.is_null());
-                let author_styles = unsafe {
-                    &*(shadow.0.mServoStyles.mPtr
-                        as *const structs::RawServoAuthorStyles
-                        as *const bindings::RawServoAuthorStyles)
-                };
-
-                let author_styles: &'a _ = AuthorStyles::<GeckoStyleSheet>::from_ffi(author_styles);
-                f(&author_styles.data, author_styles.quirks_mode);
-                if element != *self {
-                    break;
-                }
-            }
-
             if let Some(binding) = element.xbl_binding() {
                 binding.each_xbl_cascade_data(&mut f);
 

servo/components/style/invalidation/element/state_and_attributes.rs

@@ -57,7 +57,7 @@ where
 pub struct StateAndAttrInvalidationProcessor<'a, 'b: 'a, E: TElement> {
     shared_context: &'a SharedStyleContext<'b>,
     shadow_rule_datas: &'a [(&'b CascadeData, QuirksMode)],
-    cut_off_inheritance: bool,
+    matches_document_author_rules: bool,
     element: E,
     data: &'a mut ElementData,
     matching_context: MatchingContext<'a, E::Impl>,
@@ -68,7 +68,7 @@ impl<'a, 'b: 'a, E: TElement> StateAndAttrInvalidationProcessor<'a, 'b, E> {
     pub fn new(
         shared_context: &'a SharedStyleContext<'b>,
         shadow_rule_datas: &'a [(&'b CascadeData, QuirksMode)],
-        cut_off_inheritance: bool,
+        matches_document_author_rules: bool,
         element: E,
         data: &'a mut ElementData,
         nth_index_cache: &'a mut NthIndexCache,
@@ -84,7 +84,7 @@ impl<'a, 'b: 'a, E: TElement> StateAndAttrInvalidationProcessor<'a, 'b, E> {
         Self {
             shared_context,
             shadow_rule_datas,
-            cut_off_inheritance,
+            matches_document_author_rules,
             element,
             data,
             matching_context,
@@ -248,7 +248,7 @@ where
                 invalidates_self: false,
             };
 
-            let document_origins = if self.cut_off_inheritance {
+            let document_origins = if !self.matches_document_author_rules {
                 Origin::UserAgent.into()
             } else {
                 OriginSet::all()

servo/components/style/stylesheet_set.rs

@@ -263,7 +263,7 @@ where
         Self {
             entries: vec![],
             data_validity: DataValidity::Valid,
-            dirty: false,
+            dirty: true,
         }
     }
 }
@@ -597,6 +597,11 @@ where
         self.collection.dirty
     }
 
+    /// Whether the collection is empty.
+    pub fn is_empty(&self) -> bool {
+        self.collection.len() == 0
+    }
+
     fn collection_for(
         &mut self,
         _sheet: &S,

servo/components/style/stylist.rs

@@ -7,7 +7,7 @@
 use {Atom, LocalName, Namespace, WeakAtom};
 use applicable_declarations::{ApplicableDeclarationBlock, ApplicableDeclarationList};
 use context::{CascadeInputs, QuirksMode};
-use dom::TElement;
+use dom::{TElement, TShadowRoot};
 use element_state::{DocumentState, ElementState};
 use font_metrics::FontMetricsProvider;
 #[cfg(feature = "gecko")]
@@ -599,11 +599,12 @@ impl Stylist {
 
         let mut maybe = false;
 
-        let cut_off = element.each_applicable_non_document_style_rule_data(|data, quirks_mode| {
-            maybe = maybe || f(&*data, quirks_mode);
-        });
+        let doc_author_rules_apply =
+            element.each_applicable_non_document_style_rule_data(|data, quirks_mode| {
+                maybe = maybe || f(&*data, quirks_mode);
+            });
 
-        if maybe || cut_off {
+        if maybe || !doc_author_rules_apply {
             return maybe;
         }
 
@@ -1251,6 +1252,8 @@ impl Stylist {
             }
         }
 
+        let mut match_document_author_rules = matches_author_rules;
+
         // XBL / Shadow DOM rules, which are author rules too.
         //
         // TODO(emilio): Cascade order here is wrong for Shadow DOM. In
@@ -1268,26 +1271,43 @@ impl Stylist {
             }
 
             for slot in slots.iter().rev() {
-                slot.each_xbl_cascade_data(|cascade_data, _quirks_mode| {
-                    if let Some(map) = cascade_data.slotted_rules(pseudo_element) {
-                        map.get_all_matching_rules(
-                            element,
-                            rule_hash_target,
-                            applicable_declarations,
-                            context,
-                            flags_setter,
-                            CascadeLevel::AuthorNormal
-                        );
-                    }
-                });
+                let styles = slot.containing_shadow().unwrap().style_data();
+                if let Some(map) = styles.slotted_rules(pseudo_element) {
+                    map.get_all_matching_rules(
+                        element,
+                        rule_hash_target,
+                        applicable_declarations,
+                        context,
+                        flags_setter,
+                        CascadeLevel::AuthorNormal,
+                    );
+                }
+            }
+
+            // TODO(emilio): We need to look up :host rules if the element is a
+            // shadow host, when we implement that.
+            if let Some(containing_shadow) = rule_hash_target.containing_shadow() {
+                let cascade_data = containing_shadow.style_data();
+                if let Some(map) = cascade_data.normal_rules(pseudo_element) {
+                    map.get_all_matching_rules(
+                        element,
+                        rule_hash_target,
+                        applicable_declarations,
+                        context,
+                        flags_setter,
+                        CascadeLevel::AuthorNormal,
+                    );
+                }
+
+                match_document_author_rules = false;
             }
         }
 
-        // FIXME(emilio): It looks very wrong to match XBL / Shadow DOM rules
-        // even for getDefaultComputedStyle!
+        // FIXME(emilio): It looks very wrong to match XBL rules even for
+        // getDefaultComputedStyle!
         //
         // Also, this doesn't account for the author_styles_enabled stuff.
-        let cut_off_inheritance = element.each_xbl_cascade_data(|cascade_data, quirks_mode| {
+        let cut_xbl_binding_inheritance = element.each_xbl_cascade_data(|cascade_data, quirks_mode| {
             if let Some(map) = cascade_data.normal_rules(pseudo_element) {
                 // NOTE(emilio): This is needed because the XBL stylist may
                 // think it has a different quirks mode than the document.
@@ -1314,7 +1334,9 @@ impl Stylist {
             }
         });
 
-        if matches_author_rules && !only_default_rules && !cut_off_inheritance {
+        match_document_author_rules &= !cut_xbl_binding_inheritance;
+
+        if match_document_author_rules && !only_default_rules {
             // Author normal rules.
             if let Some(map) = self.cascade_data.author.normal_rules(pseudo_element) {
                 map.get_all_matching_rules(