From 5e7d429c0adaafafe5fd0573ea166192cd670b83 Mon Sep 17 00:00:00 2001 From: enordin Date: Thu, 5 Dec 2019 00:13:49 +0000 Subject: [PATCH] style: Refactor InvalidationMap flags to use bitflags. Differential Revision: https://phabricator.services.mozilla.com/D55862 --- components/style/gecko/snapshot.rs | 2 +- .../invalidation/element/invalidation_map.rs | 52 +++++++++---------- .../element/state_and_attributes.rs | 11 ++-- 3 files changed, 35 insertions(+), 30 deletions(-) diff --git a/components/style/gecko/snapshot.rs b/components/style/gecko/snapshot.rs index 02707682b4d..fa9914f6222 100644 --- a/components/style/gecko/snapshot.rs +++ b/components/style/gecko/snapshot.rs @@ -71,7 +71,7 @@ impl GeckoElementSnapshot { } /// Returns true if the snapshot recorded an attribute change which isn't a - /// class or id change. + /// class / id #[inline] pub fn other_attr_changed(&self) -> bool { self.mOtherAttributeChanged() diff --git a/components/style/invalidation/element/invalidation_map.rs b/components/style/invalidation/element/invalidation_map.rs index e0b9cf8d747..2256a7220f1 100644 --- a/components/style/invalidation/element/invalidation_map.rs +++ b/components/style/invalidation/element/invalidation_map.rs @@ -142,6 +142,19 @@ pub struct DocumentStateDependency { pub state: DocumentState, } +bitflags! { + /// A set of flags that denote whether any invalidations have occurred + /// for a particular attribute selector. + #[derive(MallocSizeOf)] + #[repr(C)] + pub struct InvalidationMapFlags : u8 { + /// Whether [class] or such is used. + const HAS_CLASS_ATTR_SELECTOR = 1 << 0; + /// Whether [id] or such is used. + const HAS_ID_ATTR_SELECTOR = 1 << 1; + } +} + /// A map where we store invalidations. /// /// This is slightly different to a SelectorMap, in the sense of that the same @@ -164,16 +177,9 @@ pub struct InvalidationMap { pub document_state_selectors: Vec, /// A map of other attribute affecting selectors. pub other_attribute_affecting_selectors: SelectorMap, - /// Whether there are attribute rules of the form `[class~="foo"]` that may - /// match. In that case, we need to look at - /// `other_attribute_affecting_selectors` too even if only the `class` has - /// changed. - pub has_class_attribute_selectors: bool, - /// Whether there are attribute rules of the form `[id|="foo"]` that may - /// match. In that case, we need to look at - /// `other_attribute_affecting_selectors` too even if only the `id` has - /// changed. - pub has_id_attribute_selectors: bool, + /// A set of flags that contain whether various special attributes are used + /// in this invalidation map. + pub flags: InvalidationMapFlags, } impl InvalidationMap { @@ -185,8 +191,7 @@ impl InvalidationMap { state_affecting_selectors: SelectorMap::new(), document_state_selectors: Vec::new(), other_attribute_affecting_selectors: SelectorMap::new(), - has_class_attribute_selectors: false, - has_id_attribute_selectors: false, + flags: InvalidationMapFlags::empty(), } } @@ -210,8 +215,7 @@ impl InvalidationMap { self.state_affecting_selectors.clear(); self.document_state_selectors.clear(); self.other_attribute_affecting_selectors.clear(); - self.has_id_attribute_selectors = false; - self.has_class_attribute_selectors = false; + self.flags = InvalidationMapFlags::empty(); } /// Adds a selector to this `InvalidationMap`. Returns Err(..) to @@ -238,8 +242,7 @@ impl InvalidationMap { state: ElementState::empty(), document_state: &mut document_state, other_attributes: false, - has_id_attribute_selectors: false, - has_class_attribute_selectors: false, + flags: &mut self.flags, }; // Visit all the simple selectors in this sequence. @@ -255,9 +258,6 @@ impl InvalidationMap { index += 1; // Account for the simple selector. } - self.has_id_attribute_selectors |= compound_visitor.has_id_attribute_selectors; - self.has_class_attribute_selectors |= compound_visitor.has_class_attribute_selectors; - for class in compound_visitor.classes { self.class_to_selector .try_entry(class, quirks_mode)? @@ -349,11 +349,8 @@ struct CompoundSelectorDependencyCollector<'a> { /// [id] attribute selectors). other_attributes: bool, - /// Whether there were attribute selectors with the id attribute. - has_id_attribute_selectors: bool, - - /// Whether there were attribute selectors with the class attribute. - has_class_attribute_selectors: bool, + /// The invalidation map flags, that we set when some attribute selectors are present. + flags: &'a mut InvalidationMapFlags, } impl<'a> SelectorVisitor for CompoundSelectorDependencyCollector<'a> { @@ -398,8 +395,11 @@ impl<'a> SelectorVisitor for CompoundSelectorDependencyCollector<'a> { }; if may_match_in_no_namespace { - self.has_id_attribute_selectors |= *local_name_lower == local_name!("id"); - self.has_class_attribute_selectors |= *local_name_lower == local_name!("class"); + if *local_name_lower == local_name!("id") { + self.flags.insert(InvalidationMapFlags::HAS_ID_ATTR_SELECTOR) + } else if *local_name_lower == local_name!("class") { + self.flags.insert(InvalidationMapFlags::HAS_CLASS_ATTR_SELECTOR) + } } true diff --git a/components/style/invalidation/element/state_and_attributes.rs b/components/style/invalidation/element/state_and_attributes.rs index e678413218c..024599219bc 100644 --- a/components/style/invalidation/element/state_and_attributes.rs +++ b/components/style/invalidation/element/state_and_attributes.rs @@ -42,6 +42,7 @@ where descendant_invalidations: &'a mut DescendantInvalidationLists<'selectors>, sibling_invalidations: &'a mut InvalidationVector<'selectors>, invalidates_self: bool, + attr_selector_flags: InvalidationMapFlags, } /// An invalidation processor for style changes due to state and attribute @@ -155,6 +156,8 @@ where return false; } + let mut attr_selector_flags = InvalidationMapFlags::empty(); + // If we the visited state changed, we force a restyle here. Matching // doesn't depend on the actual visited state at all, so we can't look // at matching results to decide what to do for this case. @@ -172,6 +175,7 @@ where let mut classes_removed = SmallVec::<[Atom; 8]>::new(); let mut classes_added = SmallVec::<[Atom; 8]>::new(); if snapshot.class_changed() { + attr_selector_flags.insert(InvalidationMapFlags::HAS_CLASS_ATTR_SELECTOR); // TODO(emilio): Do this more efficiently! snapshot.each_class(|c| { if !element.has_class(c, CaseSensitivity::CaseSensitive) { @@ -189,6 +193,7 @@ where let mut id_removed = None; let mut id_added = None; if snapshot.id_changed() { + attr_selector_flags.insert(InvalidationMapFlags::HAS_ID_ATTR_SELECTOR); let old_id = snapshot.id_attr(); let current_id = element.id(); @@ -199,7 +204,7 @@ where } if log_enabled!(::log::Level::Debug) { - debug!("Collecting changes for: {:?}", element); + debug!("Collecting changes for: {:?}, flags {:?}", element, attr_selector_flags); if !state_changes.is_empty() { debug!(" > state: {:?}", state_changes); } @@ -247,6 +252,7 @@ where descendant_invalidations, sibling_invalidations, invalidates_self: false, + attr_selector_flags, }; let document_origins = if !matches_document_author_rules { @@ -357,8 +363,7 @@ where } let should_examine_attribute_selector_map = self.snapshot.other_attr_changed() || - (self.snapshot.class_changed() && map.has_class_attribute_selectors) || - (self.snapshot.id_changed() && map.has_id_attribute_selectors); + map.flags.intersects(self.attr_selector_flags); if should_examine_attribute_selector_map { self.collect_dependencies_in_map(&map.other_attribute_affecting_selectors)