From b040d7933358acfec1099e938ce7021199458b5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 14 Aug 2017 12:08:13 +0200 Subject: [PATCH 1/2] style: Avoid multiple selector walk-ups during SelectorMap insertion. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MozReview-Commit-ID: 5b2X0GL2753 Signed-off-by: Emilio Cobos Álvarez --- components/style/selector_map.rs | 185 +++++++++++++++---------------- tests/unit/style/stylist.rs | 34 +----- 2 files changed, 92 insertions(+), 127 deletions(-) diff --git a/components/style/selector_map.rs b/components/style/selector_map.rs index 2e2c27840b0..a76da79e457 100644 --- a/components/style/selector_map.rs +++ b/components/style/selector_map.rs @@ -15,7 +15,6 @@ use rule_tree::CascadeLevel; use selector_parser::SelectorImpl; use selectors::matching::{matches_selector, MatchingContext, ElementSelectorFlags}; use selectors::parser::{Component, Combinator, SelectorIter}; -use selectors::parser::LocalName as LocalNameSelector; use smallvec::{SmallVec, VecLike}; use std::collections::{HashMap, HashSet}; use std::collections::hash_map; @@ -102,7 +101,7 @@ pub struct SelectorMap { /// A hash from local name to rules which contain that local name selector. pub local_name_hash: PrecomputedHashMap>, /// Rules that don't have ID, class, or element selectors. - pub other: Vec, + pub other: SmallVec<[T; 1]>, /// The number of entries in this map. pub count: usize, } @@ -119,7 +118,7 @@ impl SelectorMap { id_hash: MaybeCaseInsensitiveHashMap::new(), class_hash: MaybeCaseInsensitiveHashMap::new(), local_name_hash: HashMap::default(), - other: Vec::new(), + other: SmallVec::new(), count: 0, } } @@ -231,37 +230,42 @@ impl SelectorMap { pub fn insert(&mut self, entry: T, quirks_mode: QuirksMode) { self.count += 1; - if let Some(id_name) = get_id_name(entry.selector()) { - self.id_hash.entry(id_name, quirks_mode).or_insert_with(SmallVec::new).push(entry); - return; - } - - if let Some(class_name) = get_class_name(entry.selector()) { - self.class_hash.entry(class_name, quirks_mode).or_insert_with(SmallVec::new).push(entry); - return; - } - - if let Some(LocalNameSelector { name, lower_name }) = get_local_name(entry.selector()) { - // If the local name in the selector isn't lowercase, insert it into - // the rule hash twice. This means that, during lookup, we can always - // find the rules based on the local name of the element, regardless - // of whether it's an html element in an html document (in which case - // we match against lower_name) or not (in which case we match against - // name). - // - // In the case of a non-html-element-in-html-document with a - // lowercase localname and a non-lowercase selector, the rulehash - // lookup may produce superfluous selectors, but the subsequent - // selector matching work will filter them out. - if name != lower_name { - find_push(&mut self.local_name_hash, lower_name, entry.clone()); + let vector = match find_bucket(entry.selector()) { + Bucket::ID(id) => { + self.id_hash + .entry(id.clone(), quirks_mode) + .or_insert_with(SmallVec::new) } - find_push(&mut self.local_name_hash, name, entry); + Bucket::Class(class) => { + self.class_hash + .entry(class.clone(), quirks_mode) + .or_insert_with(SmallVec::new) + } + Bucket::LocalName { name, lower_name } => { + // If the local name in the selector isn't lowercase, insert it + // into the rule hash twice. This means that, during lookup, we + // can always find the rules based on the local name of the + // element, regardless of whether it's an html element in an + // html document (in which case we match against lower_name) or + // not (in which case we match against name). + // + // In the case of a non-html-element-in-html-document with a + // lowercase localname and a non-lowercase selector, the + // rulehash lookup may produce superfluous selectors, but the + // subsequent selector matching work will filter them out. + if name != lower_name { + find_push(&mut self.local_name_hash, lower_name.clone(), entry.clone()); + } + self.local_name_hash + .entry(name.clone()) + .or_insert_with(SmallVec::new) + } + Bucket::Universal => { + &mut self.other + } + }; - return; - } - - self.other.push(entry); + vector.push(entry); } /// Looks up entries by id, class, local name, and other (in order). @@ -375,74 +379,65 @@ impl SelectorMap { } } -/// Searches a compound selector from left to right. If the compound selector -/// is a pseudo-element, it's ignored. -/// -/// The first non-None value returned from |f| is returned. -#[inline(always)] -fn find_from_left( - mut iter: SelectorIter, - mut f: F -) -> Option -where - F: FnMut(&Component) -> Option, -{ - for ss in &mut iter { - if let Some(r) = f(ss) { - return Some(r) - } - } +enum Bucket<'a> { + ID(&'a Atom), + Class(&'a Atom), + LocalName { name: &'a LocalName, lower_name: &'a LocalName, }, + Universal, +} - // Effectively, pseudo-elements are ignored, given only state pseudo-classes - // may appear before them. - if iter.next_sequence() == Some(Combinator::PseudoElement) { - for ss in &mut iter { - if let Some(r) = f(ss) { - return Some(r) +fn specific_bucket_for<'a>( + component: &'a Component +) -> Bucket<'a> { + match *component { + Component::ID(ref id) => Bucket::ID(id), + Component::Class(ref class) => Bucket::Class(class), + Component::LocalName(ref selector) => { + Bucket::LocalName { + name: &selector.name, + lower_name: &selector.lower_name, } } + _ => Bucket::Universal + } +} + +/// Searches a compound selector from left to right, and returns the appropriate +/// bucket for it. +#[inline(always)] +fn find_bucket<'a>(mut iter: SelectorIter<'a, SelectorImpl>) -> Bucket<'a> { + let mut current_bucket = Bucket::Universal; + + loop { + // We basically want to find the most specific bucket, + // where: + // + // id > class > local name > universal. + // + for ss in &mut iter { + let new_bucket = specific_bucket_for(ss); + match new_bucket { + Bucket::ID(..) => return new_bucket, + Bucket::Class(..) => { + current_bucket = new_bucket; + } + Bucket::LocalName { .. } => { + if matches!(current_bucket, Bucket::Universal) { + current_bucket = new_bucket; + } + } + Bucket::Universal => {}, + } + } + + // Effectively, pseudo-elements are ignored, given only state + // pseudo-classes may appear before them. + if iter.next_sequence() != Some(Combinator::PseudoElement) { + break; + } } - None -} - -/// Retrieve the first ID name in the selector, or None otherwise. -#[inline(always)] -pub fn get_id_name(iter: SelectorIter) - -> Option { - find_from_left(iter, |ss| { - if let Component::ID(ref id) = *ss { - return Some(id.clone()); - } - None - }) -} - -/// Retrieve the FIRST class name in the selector, or None otherwise. -#[inline(always)] -pub fn get_class_name(iter: SelectorIter) - -> Option { - find_from_left(iter, |ss| { - if let Component::Class(ref class) = *ss { - return Some(class.clone()); - } - None - }) -} - -/// Retrieve the name if it is a type selector, or None otherwise. -#[inline(always)] -pub fn get_local_name(iter: SelectorIter) - -> Option> { - find_from_left(iter, |ss| { - if let Component::LocalName(ref n) = *ss { - return Some(LocalNameSelector { - name: n.name.clone(), - lower_name: n.lower_name.clone(), - }) - } - None - }) + return current_bucket } #[inline] diff --git a/tests/unit/style/stylist.rs b/tests/unit/style/stylist.rs index 82c3dc26d2b..3ae3715e9c7 100644 --- a/tests/unit/style/stylist.rs +++ b/tests/unit/style/stylist.rs @@ -167,36 +167,6 @@ fn test_rule_ordering_same_specificity() { "The rule that comes later should win."); } - -#[test] -fn test_get_id_name() { - let (rules_list, _) = get_mock_rules(&[".intro", "#top"]); - assert_eq!(selector_map::get_id_name(rules_list[0][0].selector.iter()), None); - assert_eq!(selector_map::get_id_name(rules_list[1][0].selector.iter()), Some(Atom::from("top"))); -} - -#[test] -fn test_get_class_name() { - let (rules_list, _) = get_mock_rules(&[".intro.foo", "#top"]); - assert_eq!(selector_map::get_class_name(rules_list[0][0].selector.iter()), Some(Atom::from("intro"))); - assert_eq!(selector_map::get_class_name(rules_list[1][0].selector.iter()), None); -} - -#[test] -fn test_get_local_name() { - let (rules_list, _) = get_mock_rules(&["img.foo", "#top", "IMG", "ImG"]); - let check = |i: usize, names: Option<(&str, &str)>| { - assert!(selector_map::get_local_name(rules_list[i][0].selector.iter()) - == names.map(|(name, lower_name)| LocalNameSelector { - name: LocalName::from(name), - lower_name: LocalName::from(lower_name) })) - }; - check(0, Some(("img", "img"))); - check(1, None); - check(2, Some(("IMG", "img"))); - check(3, Some(("ImG", "img"))); -} - #[test] fn test_insert() { let (rules_list, _) = get_mock_rules(&[".intro.foo", "#top"]); @@ -204,8 +174,8 @@ fn test_insert() { selector_map.insert(rules_list[1][0].clone(), QuirksMode::NoQuirks); assert_eq!(1, selector_map.id_hash.get(&Atom::from("top"), QuirksMode::NoQuirks).unwrap()[0].source_order); selector_map.insert(rules_list[0][0].clone(), QuirksMode::NoQuirks); - assert_eq!(0, selector_map.class_hash.get(&Atom::from("intro"), QuirksMode::NoQuirks).unwrap()[0].source_order); - assert!(selector_map.class_hash.get(&Atom::from("foo"), QuirksMode::NoQuirks).is_none()); + assert_eq!(0, selector_map.class_hash.get(&Atom::from("foo"), QuirksMode::NoQuirks).unwrap()[0].source_order); + assert!(selector_map.class_hash.get(&Atom::from("intro"), QuirksMode::NoQuirks).is_none()); } fn mock_stylist() -> Stylist { From 4c80cccbd2aa79281a648c8e2f785f05aab8ea0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 14 Aug 2017 16:27:00 +0200 Subject: [PATCH 2/2] stylo: Cleanup a bit of the Stylist clear setup. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This moves us to clear on rebuild, which allows us to remove yet another place where we track stylist dirtiness. Bug: 1390255 Reviewed-by: heycam MozReview-Commit-ID: nihQbUAbh8 Signed-off-by: Emilio Cobos Álvarez --- components/style/gecko/data.rs | 17 +- components/style/gecko/generated/bindings.rs | 3 - .../invalidation/element/invalidation_map.rs | 4 +- components/style/selector_map.rs | 9 + components/style/stylesheet_set.rs | 29 ++-- components/style/stylesheets/mod.rs | 2 +- components/style/stylesheets/origin.rs | 14 -- components/style/stylist.rs | 161 ++++-------------- ports/geckolib/glue.rs | 21 +-- 9 files changed, 65 insertions(+), 195 deletions(-) diff --git a/components/style/gecko/data.rs b/components/style/gecko/data.rs index 1ac69f2f29e..7d99e4af89f 100644 --- a/components/style/gecko/data.rs +++ b/components/style/gecko/data.rs @@ -17,7 +17,7 @@ use properties::ComputedValues; use servo_arc::Arc; use shared_lock::{Locked, StylesheetGuards, SharedRwLockReadGuard}; use stylesheet_set::StylesheetSet; -use stylesheets::{Origin, PerOrigin, StylesheetContents, StylesheetInDocument}; +use stylesheets::{PerOrigin, StylesheetContents, StylesheetInDocument}; use stylist::{ExtraStyleData, Stylist}; /// Little wrapper to a Gecko style sheet. @@ -164,14 +164,14 @@ impl PerDocumentStyleDataImpl { let author_style_disabled = self.stylesheets.author_style_disabled(); - let iter = self.stylesheets.flush(document_element); + let (iter, dirty_origins) = self.stylesheets.flush(document_element); self.stylist.rebuild( iter, &StylesheetGuards::same(guard), /* ua_sheets = */ None, - /* stylesheets_changed = */ true, author_style_disabled, &mut self.extra_style_data, + dirty_origins, ); } @@ -187,17 +187,6 @@ impl PerDocumentStyleDataImpl { self.stylist.device().default_computed_values_arc() } - /// Clear the stylist. This will be a no-op if the stylist is - /// already cleared; the stylist handles that. - pub fn clear_stylist(&mut self) { - self.stylist.clear(); - } - - /// Clear the stylist's data for the specified origin. - pub fn clear_stylist_origin(&mut self, origin: &Origin) { - self.stylist.clear_origin(origin); - } - /// Returns whether visited links are enabled. fn visited_links_enabled(&self) -> bool { unsafe { bindings::Gecko_AreVisitedLinksEnabled() } diff --git a/components/style/gecko/generated/bindings.rs b/components/style/gecko/generated/bindings.rs index 8541250224a..91a2d0f0ae7 100644 --- a/components/style/gecko/generated/bindings.rs +++ b/components/style/gecko/generated/bindings.rs @@ -1987,9 +1987,6 @@ extern "C" { pub fn Servo_StyleSet_Init(pres_context: RawGeckoPresContextOwned) -> *mut RawServoStyleSet; } -extern "C" { - pub fn Servo_StyleSet_Clear(set: RawServoStyleSetBorrowed); -} extern "C" { pub fn Servo_StyleSet_RebuildCachedData(set: RawServoStyleSetBorrowed); } diff --git a/components/style/invalidation/element/invalidation_map.rs b/components/style/invalidation/element/invalidation_map.rs index 81f966a5df3..3b0601a3f1d 100644 --- a/components/style/invalidation/element/invalidation_map.rs +++ b/components/style/invalidation/element/invalidation_map.rs @@ -195,8 +195,8 @@ impl InvalidationMap { pub fn clear(&mut self) { self.class_to_selector.clear(); self.id_to_selector.clear(); - self.state_affecting_selectors = SelectorMap::new(); - self.other_attribute_affecting_selectors = SelectorMap::new(); + self.state_affecting_selectors.clear(); + self.other_attribute_affecting_selectors.clear(); self.has_id_attribute_selectors = false; self.has_class_attribute_selectors = false; } diff --git a/components/style/selector_map.rs b/components/style/selector_map.rs index a76da79e457..139a2fd95f6 100644 --- a/components/style/selector_map.rs +++ b/components/style/selector_map.rs @@ -123,6 +123,15 @@ impl SelectorMap { } } + /// Clears the hashmap retaining storage. + pub fn clear(&mut self) { + self.id_hash.clear(); + self.class_hash.clear(); + self.local_name_hash.clear(); + self.other.clear(); + self.count = 0; + } + /// Returns whether there are any entries in the map. pub fn is_empty(&self) -> bool { self.count == 0 diff --git a/components/style/stylesheet_set.rs b/components/style/stylesheet_set.rs index 60f35dd2788..76d6b592a86 100644 --- a/components/style/stylesheet_set.rs +++ b/components/style/stylesheet_set.rs @@ -8,7 +8,7 @@ use dom::TElement; use invalidation::stylesheets::StylesheetInvalidationSet; use shared_lock::SharedRwLockReadGuard; use std::slice; -use stylesheets::{Origin, PerOrigin, StylesheetInDocument}; +use stylesheets::{OriginSet, PerOrigin, StylesheetInDocument}; use stylist::Stylist; /// Entry for a StylesheetSet. We don't bother creating a constructor, because @@ -169,21 +169,23 @@ where pub fn flush( &mut self, document_element: Option, - ) -> StylesheetIterator + ) -> (StylesheetIterator, OriginSet) where E: TElement, { debug!("StylesheetSet::flush"); debug_assert!(self.has_changed()); - for (data, _) in self.invalidation_data.iter_mut_origins() { + let mut origins = OriginSet::empty(); + for (data, origin) in self.invalidation_data.iter_mut_origins() { if data.dirty { data.invalidations.flush(document_element); data.dirty = false; + origins |= origin; } } - self.iter() + (self.iter(), origins) } /// Returns an iterator over the current list of stylesheets. @@ -191,24 +193,15 @@ where StylesheetIterator(self.entries.iter()) } - /// Mark the stylesheets as dirty, because something external may have - /// invalidated it. - /// - /// FIXME(emilio): Make this more granular. - pub fn force_dirty(&mut self) { - for (data, _) in self.invalidation_data.iter_mut_origins() { + /// Mark the stylesheets for the specified origin as dirty, because + /// something external may have invalidated it. + pub fn force_dirty(&mut self, origins: OriginSet) { + for origin in origins.iter() { + let data = self.invalidation_data.borrow_mut_for_origin(&origin); data.invalidations.invalidate_fully(); data.dirty = true; } } - - /// Mark the stylesheets for the specified origin as dirty, because - /// something external may have invalidated it. - pub fn force_dirty_origin(&mut self, origin: &Origin) { - let data = self.invalidation_data.borrow_mut_for_origin(origin); - data.invalidations.invalidate_fully(); - data.dirty = true; - } } struct InvalidationData { diff --git a/components/style/stylesheets/mod.rs b/components/style/stylesheets/mod.rs index e52fcf17887..0f87efc2963 100644 --- a/components/style/stylesheets/mod.rs +++ b/components/style/stylesheets/mod.rs @@ -44,7 +44,7 @@ pub use self::memory::{MallocSizeOf, MallocSizeOfFn, MallocSizeOfWithGuard}; #[cfg(feature = "gecko")] pub use self::memory::{MallocSizeOfWithRepeats, SizeOfState}; pub use self::namespace_rule::NamespaceRule; -pub use self::origin::{Origin, OriginSet, PerOrigin, PerOriginClear}; +pub use self::origin::{Origin, OriginSet, PerOrigin}; pub use self::page_rule::PageRule; pub use self::rule_parser::{State, TopLevelRuleParser}; pub use self::rule_list::{CssRules, CssRulesHelpers}; diff --git a/components/style/stylesheets/origin.rs b/components/style/stylesheets/origin.rs index 2e178d00e8b..1eb3c4932f1 100644 --- a/components/style/stylesheets/origin.rs +++ b/components/style/stylesheets/origin.rs @@ -168,20 +168,6 @@ impl PerOrigin { } } -/// An object that can be cleared. -pub trait PerOriginClear { - /// Clears the object. - fn clear(&mut self); -} - -impl PerOriginClear for PerOrigin where T: PerOriginClear { - fn clear(&mut self) { - self.user_agent.clear(); - self.user.clear(); - self.author.clear(); - } -} - /// Iterator over `PerOrigin`, from highest level (author) to lowest /// (user agent). /// diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 760f820c967..b0332d71097 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -40,7 +40,7 @@ use style_traits::viewport::ViewportConstraints; #[cfg(feature = "gecko")] use stylesheets::{CounterStyleRule, FontFaceRule}; use stylesheets::{CssRule, StyleRule}; -use stylesheets::{StylesheetInDocument, Origin, OriginSet, PerOrigin, PerOriginClear}; +use stylesheets::{StylesheetInDocument, Origin, OriginSet, PerOrigin}; use stylesheets::UserAgentStylesheets; use stylesheets::keyframes_rule::KeyframesAnimation; use stylesheets::viewport_rule::{self, MaybeNew, ViewportRule}; @@ -182,52 +182,6 @@ impl Stylist { } } - /// Clear the stylist's state, effectively resetting it to more or less - /// the state Stylist::new creates. - /// - /// We preserve the state of the following members: - /// device: Someone might have set this on us. - /// quirks_mode: Again, someone might have set this on us. - /// num_rebuilds: clear() followed by rebuild() should just increment this - /// rule_tree: So we can re-use rule nodes across rebuilds. - /// - /// We don't just use struct update syntax with Stylist::new(self.device) - /// beause for some of our members we can clear them instead of creating new - /// objects. This does cause unfortunate code duplication with - /// Stylist::new. - pub fn clear(&mut self) { - self.cascade_data.clear(); - self.precomputed_pseudo_element_decls.clear(); - self.viewport_constraints = None; - - // XXX(heycam) Why do this, if we are preserving the Device? - self.is_device_dirty = true; - } - - /// Clear the stylist's state for the specified origin. - pub fn clear_origin(&mut self, origin: &Origin) { - self.cascade_data.borrow_mut_for_origin(origin).clear(); - - if *origin == Origin::UserAgent { - // We only collect these declarations from UA sheets. - self.precomputed_pseudo_element_decls.clear(); - } - - // The stored `ViewportConstraints` contains data from rules across - // all origins. - self.viewport_constraints = None; - - // XXX(heycam) Why do this, if we are preserving the Device? - self.is_device_dirty = true; - } - - /// Returns whether any origin's `CascadeData` has been cleared. - fn any_origin_cleared(&self) -> bool { - self.cascade_data - .iter_origins() - .any(|(d, _)| d.is_cleared) - } - /// Rebuild the stylist for the given document stylesheets, and optionally /// with a set of user agent stylesheets. /// @@ -239,40 +193,17 @@ impl Stylist { doc_stylesheets: I, guards: &StylesheetGuards, ua_stylesheets: Option<&UserAgentStylesheets>, - stylesheets_changed: bool, author_style_disabled: bool, - extra_data: &mut PerOrigin + extra_data: &mut PerOrigin, + mut origins_to_rebuild: OriginSet, ) -> bool where I: Iterator + Clone, S: StylesheetInDocument + ToMediaListKey + 'static, { - debug_assert!(!self.any_origin_cleared() || self.is_device_dirty); - - // Determine the origins that actually need updating. - // - // XXX(heycam): What is the relationship between `stylesheets_changed` - // and the `is_cleared` fields on each origin's `CascadeData`? Can - // we avoid passing in `stylesheets_changed`? - let mut to_update: PerOrigin = Default::default(); - - // If we're provided with a list of UA and user style sheets, then - // we must update those cascade levels. (Servo does this, but Gecko - // just includes the UA and User sheets in `doc_stylesheets`.) - if ua_stylesheets.is_some() { - to_update.user_agent = true; - to_update.user = true; - } - - for (data, origin) in self.cascade_data.iter_mut_origins() { - if data.is_cleared { - data.is_cleared = false; - *to_update.borrow_mut_for_origin(&origin) = true; - } - } - - if !(self.is_device_dirty || stylesheets_changed) { - return false; + debug_assert!(!origins_to_rebuild.is_empty() || self.is_device_dirty); + if self.is_device_dirty { + origins_to_rebuild = OriginSet::all(); } self.num_rebuilds += 1; @@ -307,13 +238,13 @@ impl Stylist { } } - // XXX(heycam): We should probably just move the `extra_data` to be - // stored on the `Stylist` instead of Gecko's `PerDocumentStyleData`. - // That would let us clear it inside `clear()` and `clear_origin()`. - for (update, origin) in to_update.iter_origins() { - if *update { - extra_data.borrow_mut_for_origin(&origin).clear(); - } + for origin in origins_to_rebuild.iter() { + extra_data.borrow_mut_for_origin(&origin).clear(); + self.cascade_data.borrow_mut_for_origin(&origin).clear(); + } + + if origins_to_rebuild.contains(Origin::UserAgent.into()) { + self.precomputed_pseudo_element_decls.clear(); } if let Some(ua_stylesheets) = ua_stylesheets { @@ -337,11 +268,10 @@ impl Stylist { // Only add stylesheets for origins we are updating, and only add // Author level sheets if author style is not disabled. let sheets_to_add = doc_stylesheets.filter(|s| { - match s.contents(guards.author).origin { - Origin::UserAgent => to_update.user_agent, - Origin::Author => to_update.author && !author_style_disabled, - Origin::User => to_update.user, - } + let sheet_origin = s.contents(guards.author).origin; + + origins_to_rebuild.contains(sheet_origin.into()) && + (!matches!(sheet_origin, Origin::Author) || !author_style_disabled) }); for stylesheet in sheets_to_add { @@ -367,16 +297,19 @@ impl Stylist { I: Iterator + Clone, S: StylesheetInDocument + ToMediaListKey + 'static, { - debug_assert!(!self.any_origin_cleared() || self.is_device_dirty); - // We have to do a dirtiness check before clearing, because if // we're not actually dirty we need to no-op here. if !(self.is_device_dirty || stylesheets_changed) { return false; } - self.clear(); - self.rebuild(doc_stylesheets, guards, ua_stylesheets, stylesheets_changed, - author_style_disabled, extra_data) + self.rebuild( + doc_stylesheets, + guards, + ua_stylesheets, + author_style_disabled, + extra_data, + OriginSet::all(), + ) } fn add_stylesheet( @@ -529,11 +462,7 @@ impl Stylist { pub fn might_have_attribute_dependency(&self, local_name: &LocalName) -> bool { - if self.any_origin_cleared() || self.is_device_dirty { - // We can't tell what attributes are in our style rules until - // we rebuild. - true - } else if *local_name == local_name!("style") { + if *local_name == local_name!("style") { self.cascade_data .iter_origins() .any(|(d, _)| d.style_attribute_dependency) @@ -550,13 +479,7 @@ impl Stylist { /// Returns whether the given ElementState bit might be relied upon by a /// selector of some rule in the stylist. pub fn might_have_state_dependency(&self, state: ElementState) -> bool { - if self.any_origin_cleared() || self.is_device_dirty { - // We can't tell what states our style rules rely on until - // we rebuild. - true - } else { - self.has_state_dependency(state) - } + self.has_state_dependency(state) } /// Returns whether the given ElementState bit is relied upon by a selector @@ -1440,14 +1363,17 @@ impl ExtraStyleData { } /// Add the given @counter-style rule. - fn add_counter_style(&mut self, guard: &SharedRwLockReadGuard, - rule: &Arc>) { + fn add_counter_style( + &mut self, + guard: &SharedRwLockReadGuard, + rule: &Arc>, + ) { let name = rule.read_with(guard).mName.raw::().into(); self.counter_styles.insert(name, rule.clone()); } } -impl PerOriginClear for ExtraStyleData { +impl ExtraStyleData { fn clear(&mut self) { #[cfg(feature = "gecko")] { @@ -1689,11 +1615,6 @@ struct CascadeData { /// The total number of declarations. num_declarations: usize, - - /// If true, the `CascadeData` is in a cleared state (e.g. just-constructed, - /// or had `clear()` called on it with no following `rebuild()` on the - /// `Stylist`). - is_cleared: bool, } impl CascadeData { @@ -1712,7 +1633,6 @@ impl CascadeData { rules_source_order: 0, num_selectors: 0, num_declarations: 0, - is_cleared: true, } } @@ -1727,28 +1647,21 @@ impl CascadeData { fn has_rules_for_pseudo(&self, pseudo: &PseudoElement) -> bool { self.pseudos_map.get(pseudo).is_some() } -} -impl PerOriginClear for CascadeData { fn clear(&mut self) { - if self.is_cleared { - return; - } - - self.element_map = SelectorMap::new(); - self.pseudos_map = Default::default(); - self.animations = Default::default(); + self.element_map.clear(); + self.pseudos_map.clear(); + self.animations.clear(); self.invalidation_map.clear(); self.attribute_dependencies.clear(); self.style_attribute_dependency = false; self.state_dependencies = ElementState::empty(); self.mapped_ids.clear(); - self.selectors_for_cache_revalidation = SelectorMap::new(); + self.selectors_for_cache_revalidation.clear(); self.effective_media_query_results.clear(); self.rules_source_order = 0; self.num_selectors = 0; self.num_declarations = 0; - self.is_cleared = true; } } diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 9c620b35e70..15fea3f5d63 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -117,7 +117,7 @@ use style::style_adjuster::StyleAdjuster; use style::stylesheets::{CssRule, CssRules, CssRuleType, CssRulesHelpers, DocumentRule}; use style::stylesheets::{FontFeatureValuesRule, ImportRule, KeyframesRule, MallocSizeOfWithGuard}; use style::stylesheets::{MediaRule, NamespaceRule, Origin, OriginSet, PageRule, SizeOfState, StyleRule}; -use style::stylesheets::{StylesheetContents, StylesheetInDocument, SupportsRule}; +use style::stylesheets::{StylesheetContents, SupportsRule}; use style::stylesheets::StylesheetLoader as StyleStylesheetLoader; use style::stylesheets::keyframes_rule::{Keyframe, KeyframeSelector, KeyframesStepValue}; use style::stylesheets::supports_rule::parse_condition_or_declaration; @@ -891,9 +891,7 @@ pub extern "C" fn Servo_StyleSet_AppendStyleSheet( let mut data = &mut *data; let guard = global_style_data.shared_lock.read(); let sheet = unsafe { GeckoStyleSheet::new(sheet) }; - let origin = sheet.contents(&guard).origin; data.stylesheets.append_stylesheet(&data.stylist, sheet, &guard); - data.clear_stylist_origin(&origin); } #[no_mangle] @@ -942,9 +940,7 @@ pub extern "C" fn Servo_StyleSet_PrependStyleSheet( let mut data = &mut *data; let guard = global_style_data.shared_lock.read(); let sheet = unsafe { GeckoStyleSheet::new(sheet) }; - let origin = sheet.contents(&guard).origin; data.stylesheets.prepend_stylesheet(&data.stylist, sheet, &guard); - data.clear_stylist_origin(&origin); } #[no_mangle] @@ -958,14 +954,12 @@ pub extern "C" fn Servo_StyleSet_InsertStyleSheetBefore( let mut data = &mut *data; let guard = global_style_data.shared_lock.read(); let sheet = unsafe { GeckoStyleSheet::new(sheet) }; - let origin = sheet.contents(&guard).origin; data.stylesheets.insert_stylesheet_before( &data.stylist, sheet, unsafe { GeckoStyleSheet::new(before_sheet) }, &guard, ); - data.clear_stylist_origin(&origin); } #[no_mangle] @@ -978,9 +972,7 @@ pub extern "C" fn Servo_StyleSet_RemoveStyleSheet( let mut data = &mut *data; let guard = global_style_data.shared_lock.read(); let sheet = unsafe { GeckoStyleSheet::new(sheet) }; - let origin = sheet.contents(&guard).origin; data.stylesheets.remove_stylesheet(&data.stylist, sheet, &guard); - data.clear_stylist_origin(&origin); } #[no_mangle] @@ -1002,10 +994,7 @@ pub extern "C" fn Servo_StyleSet_NoteStyleSheetsChanged( changed_origins: OriginFlags, ) { let mut data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut(); - for origin in OriginSet::from(changed_origins).iter() { - data.stylesheets.force_dirty_origin(&origin); - data.clear_stylist_origin(&origin); - } + data.stylesheets.force_dirty(OriginSet::from(changed_origins)); data.stylesheets.set_author_style_disabled(author_style_disabled); } @@ -1939,12 +1928,6 @@ pub extern "C" fn Servo_StyleSet_RebuildCachedData(raw_data: RawServoStyleSetBor data.stylist.device_mut().rebuild_cached_data(); } -#[no_mangle] -pub extern "C" fn Servo_StyleSet_Clear(raw_data: RawServoStyleSetBorrowed) { - let mut data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut(); - data.clear_stylist(); -} - #[no_mangle] pub extern "C" fn Servo_StyleSet_Drop(data: RawServoStyleSetOwned) { let _ = data.into_box::();