From ef4ea7cc497225615fd0d316c0b44f377871ad8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 5 Mar 2018 12:50:04 +0100 Subject: [PATCH] style: Separate the XBL and shadow dom styling bits. Bug: 1441022 Reviewed-by: xidorn MozReview-Commit-ID: 2W0BmZ8wWXg --- components/layout_thread/dom_wrapper.rs | 8 +++ components/style/data.rs | 12 ++-- components/style/dom.rs | 21 ++++-- components/style/gecko/wrapper.rs | 44 +++++++------ .../element/state_and_attributes.rs | 8 +-- components/style/stylesheet_set.rs | 7 +- components/style/stylist.rs | 64 +++++++++++++------ 7 files changed, 107 insertions(+), 57 deletions(-) diff --git a/components/layout_thread/dom_wrapper.rs b/components/layout_thread/dom_wrapper.rs index 3161de39b00..5cf29e8e159 100644 --- a/components/layout_thread/dom_wrapper.rs +++ b/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> { diff --git a/components/style/data.rs b/components/style/data.rs index 275c8c4294d..e1af10308ad 100644 --- a/components/style/data.rs +++ b/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, diff --git a/components/style/dom.rs b/components/style/dom.rs index 9cd6e29feeb..59c31572c56 100644 --- a/components/style/dom.rs +++ b/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) -> ::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 diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 34310df5522..f3e60970173 100644 --- a/components/style/gecko/wrapper.rs +++ b/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::::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::::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); diff --git a/components/style/invalidation/element/state_and_attributes.rs b/components/style/invalidation/element/state_and_attributes.rs index 10a7f07aef4..0c0bba24e89 100644 --- a/components/style/invalidation/element/state_and_attributes.rs +++ b/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() diff --git a/components/style/stylesheet_set.rs b/components/style/stylesheet_set.rs index 56a384353bf..fbff5501882 100644 --- a/components/style/stylesheet_set.rs +++ b/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, diff --git a/components/style/stylist.rs b/components/style/stylist.rs index cd4ff265009..b5cb97b6719 100644 --- a/components/style/stylist.rs +++ b/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(