style: Invalidate for CSSOM changes in a more fine-grained way.

Also, for changes in CSS declarations, like changing
cssRules[i].style.color or something, we end up avoiding a lot of the
work we were doing.

This page still trips us in the sense that they add a stylesheet, then
call getBoundingClientRect(), then insert more rules in the stylesheet,
which causes us to rebuild a lot of the cascade data.

We could try to detect appends to the last stylesheet on the list or
something I guess, and avoid rebuilding the cascade data in some cases.

Depends on D85615

Differential Revision: https://phabricator.services.mozilla.com/D85616

components/style/invalidation/stylesheets.rs

@@ -16,10 +16,26 @@ use crate::selector_map::{MaybeCaseInsensitiveHashMap, PrecomputedHashMap};
 use crate::selector_parser::{SelectorImpl, Snapshot, SnapshotMap};
 use crate::shared_lock::SharedRwLockReadGuard;
 use crate::stylesheets::{CssRule, StylesheetInDocument};
+use crate::stylesheets::{EffectiveRules, EffectiveRulesIterator};
 use crate::Atom;
 use crate::LocalName as SelectorLocalName;
 use selectors::parser::{Component, LocalName, Selector};
 
+/// The kind of change that happened for a given rule.
+#[repr(u32)]
+#[derive(Clone, Copy, Debug, Eq, Hash, MallocSizeOf, PartialEq)]
+pub enum RuleChangeKind {
+    /// The rule was inserted.
+    Insertion,
+    /// The rule was removed.
+    Removal,
+    /// Some change in the rule which we don't know about, and could have made
+    /// the rule change in any way.
+    Generic,
+    /// A change in the declarations of a style rule.
+    StyleRuleDeclarations,
+}
+
 /// A style sheet invalidation represents a kind of element or subtree that may
 /// need to be restyled. Whether it represents a whole subtree or just a single
 /// element is determined by the given InvalidationKind in
@@ -162,7 +178,8 @@ impl StylesheetInvalidationSet {
         have_invalidations
     }
 
-    fn is_empty(&self) -> bool {
+    /// Returns whether there's no invalidation to process.
+    pub fn is_empty(&self) -> bool {
         !self.fully_invalid &&
             self.classes.is_empty() &&
             self.ids.is_empty() &&
@@ -484,6 +501,75 @@ impl StylesheetInvalidationSet {
         true
     }
 
+    /// Collects invalidations for a given CSS rule, if not fully invalid
+    /// already.
+    ///
+    /// TODO(emilio): we can't check whether the rule is inside a non-effective
+    /// subtree, we potentially could do that.
+    pub fn rule_changed<S>(
+        &mut self,
+        stylesheet: &S,
+        rule: &CssRule,
+        guard: &SharedRwLockReadGuard,
+        device: &Device,
+        quirks_mode: QuirksMode,
+        change_kind: RuleChangeKind,
+    ) where
+        S: StylesheetInDocument,
+    {
+        use crate::stylesheets::CssRule::*;
+
+        debug!("StylesheetInvalidationSet::rule_changed");
+        if self.fully_invalid {
+            return;
+        }
+
+        if !stylesheet.enabled() || !stylesheet.is_effective_for_device(device, guard) {
+            debug!(" > Stylesheet was not effective");
+            return; // Nothing to do here.
+        }
+
+        let is_generic_change = change_kind == RuleChangeKind::Generic;
+
+        match *rule {
+            Namespace(..) => {
+                // It's not clear what handling changes for this correctly would
+                // look like.
+            },
+            CounterStyle(..) |
+            Page(..) |
+            Viewport(..) |
+            FontFeatureValues(..) |
+            FontFace(..) |
+            Keyframes(..) |
+            Style(..) => {
+                if is_generic_change {
+                    // TODO(emilio): We need to do this for selector / keyframe
+                    // name / font-face changes, because we don't have the old
+                    // selector / name.  If we distinguish those changes
+                    // specially, then we can at least use this invalidation for
+                    // style declaration changes.
+                    return self.invalidate_fully();
+                }
+
+                self.collect_invalidations_for_rule(rule, guard, device, quirks_mode)
+            },
+            Document(..) | Import(..) | Media(..) | Supports(..) => {
+                if !is_generic_change &&
+                    !EffectiveRules::is_effective(guard, device, quirks_mode, rule)
+                {
+                    return;
+                }
+
+                let rules =
+                    EffectiveRulesIterator::effective_children(device, quirks_mode, guard, rule);
+                for rule in rules {
+                    self.collect_invalidations_for_rule(rule, guard, device, quirks_mode)
+                }
+            },
+        }
+    }
+
     /// Collects invalidations for a given CSS rule.
     fn collect_invalidations_for_rule(
         &mut self,

components/style/stylesheet_set.rs

@@ -5,11 +5,11 @@
 //! A centralized set of stylesheets for a document.
 
 use crate::dom::TElement;
-use crate::invalidation::stylesheets::StylesheetInvalidationSet;
+use crate::invalidation::stylesheets::{StylesheetInvalidationSet, RuleChangeKind};
 use crate::media_queries::Device;
 use crate::selector_parser::SnapshotMap;
 use crate::shared_lock::SharedRwLockReadGuard;
-use crate::stylesheets::{Origin, OriginSet, OriginSetIterator, PerOrigin, StylesheetInDocument};
+use crate::stylesheets::{CssRule, Origin, OriginSet, OriginSetIterator, PerOrigin, StylesheetInDocument};
 use std::{mem, slice};
 
 /// Entry for a StylesheetSet.
@@ -438,6 +438,56 @@ macro_rules! sheet_set_methods {
             let collection = self.collection_for(&sheet, guard);
             collection.remove(&sheet)
         }
+
+        /// Notify the set that a rule from a given stylesheet has changed
+        /// somehow.
+        pub fn rule_changed(
+            &mut self,
+            device: Option<&Device>,
+            sheet: &S,
+            rule: &CssRule,
+            guard: &SharedRwLockReadGuard,
+            change_kind: RuleChangeKind,
+        ) {
+            if let Some(device) = device {
+                let quirks_mode = sheet.quirks_mode(guard);
+                self.invalidations.rule_changed(
+                    sheet,
+                    rule,
+                    guard,
+                    device,
+                    quirks_mode,
+                    change_kind,
+                );
+            }
+
+            let validity = match change_kind {
+                // Insertion / Removals need to rebuild both the cascade and
+                // invalidation data. For generic changes this is conservative,
+                // could be optimized on a per-case basis.
+                RuleChangeKind::Generic |
+                RuleChangeKind::Insertion |
+                RuleChangeKind::Removal => DataValidity::FullyInvalid,
+                // TODO(emilio): This, in theory, doesn't need to invalidate
+                // style data, if the rule we're modifying is actually in the
+                // CascadeData already.
+                //
+                // But this is actually a bit tricky to prove, because when we
+                // copy-on-write a stylesheet we don't bother doing a rebuild,
+                // so we may still have rules from the original stylesheet
+                // instead of the cloned one that we're modifying. So don't
+                // bother for now and unconditionally rebuild, it's no worse
+                // than what we were already doing anyway.
+                //
+                // Maybe we could record whether we saw a clone in this flush,
+                // and if so do the conservative thing, otherwise just
+                // early-return.
+                RuleChangeKind::StyleRuleDeclarations => DataValidity::FullyInvalid,
+            };
+
+            let collection = self.collection_for(&sheet, guard);
+            collection.set_data_validity_at_least(validity);
+        }
     };
 }
 
@@ -485,6 +535,7 @@ where
 
     /// Returns whether the given set has changed from the last flush.
     pub fn has_changed(&self) -> bool {
+        !self.invalidations.is_empty() ||
         self.collections
             .iter_origins()
             .any(|(collection, _)| collection.dirty)

components/style/stylesheets/mod.rs

@@ -56,7 +56,7 @@ pub use self::page_rule::PageRule;
 pub use self::rule_list::{CssRules, CssRulesHelpers};
 pub use self::rule_parser::{InsertRuleContext, State, TopLevelRuleParser};
 pub use self::rules_iterator::{AllRules, EffectiveRules};
-pub use self::rules_iterator::{NestedRuleIterationCondition, RulesIterator};
+pub use self::rules_iterator::{NestedRuleIterationCondition, EffectiveRulesIterator, RulesIterator};
 pub use self::style_rule::StyleRule;
 pub use self::stylesheet::{AllowImportRules, SanitizationData, SanitizationKind};
 pub use self::stylesheet::{DocumentStyleSheet, Namespaces, Stylesheet};

components/style/stylesheets/rules_iterator.rs

@@ -35,15 +35,15 @@ where
         device: &'a Device,
         quirks_mode: QuirksMode,
         guard: &'a SharedRwLockReadGuard<'b>,
-        rules: &'a [CssRule],
+        rules: slice::Iter<'a, CssRule>,
     ) -> Self {
         let mut stack = SmallVec::new();
-        stack.push(rules.iter());
+        stack.push(rules);
         Self {
-            device: device,
-            quirks_mode: quirks_mode,
-            guard: guard,
-            stack: stack,
+            device,
+            quirks_mode,
+            guard,
+            stack,
             _phantom: ::std::marker::PhantomData,
         }
     }
@@ -54,6 +54,61 @@ where
     }
 }
 
+fn children_of_rule<'a, C>(
+    rule: &'a CssRule,
+    device: &'a Device,
+    quirks_mode: QuirksMode,
+    guard: &'a SharedRwLockReadGuard<'_>,
+    effective: &mut bool,
+) -> slice::Iter<'a, CssRule>
+where
+    C: NestedRuleIterationCondition + 'static,
+{
+    *effective = true;
+    match *rule {
+        CssRule::Namespace(_) |
+        CssRule::Style(_) |
+        CssRule::FontFace(_) |
+        CssRule::CounterStyle(_) |
+        CssRule::Viewport(_) |
+        CssRule::Keyframes(_) |
+        CssRule::Page(_) |
+        CssRule::FontFeatureValues(_) => [].iter(),
+        CssRule::Import(ref import_rule) => {
+            let import_rule = import_rule.read_with(guard);
+            if !C::process_import(guard, device, quirks_mode, import_rule) {
+                *effective = false;
+                return [].iter();
+            }
+            import_rule.stylesheet.rules(guard).iter()
+        },
+        CssRule::Document(ref doc_rule) => {
+            let doc_rule = doc_rule.read_with(guard);
+            if !C::process_document(guard, device, quirks_mode, doc_rule) {
+                *effective = false;
+                return [].iter();
+            }
+            doc_rule.rules.read_with(guard).0.iter()
+        },
+        CssRule::Media(ref lock) => {
+            let media_rule = lock.read_with(guard);
+            if !C::process_media(guard, device, quirks_mode, media_rule) {
+                *effective = false;
+                return [].iter();
+            }
+            media_rule.rules.read_with(guard).0.iter()
+        },
+        CssRule::Supports(ref lock) => {
+            let supports_rule = lock.read_with(guard);
+            if !C::process_supports(guard, device, quirks_mode, supports_rule) {
+                *effective = false;
+                return [].iter();
+            }
+            supports_rule.rules.read_with(guard).0.iter()
+        },
+    }
+}
+
 impl<'a, 'b, C> Iterator for RulesIterator<'a, 'b, C>
 where
     'b: 'a,
@@ -62,78 +117,28 @@ where
     type Item = &'a CssRule;
 
     fn next(&mut self) -> Option<Self::Item> {
-        let mut nested_iter_finished = false;
         while !self.stack.is_empty() {
-            if nested_iter_finished {
-                self.stack.pop();
-                nested_iter_finished = false;
-                continue;
-            }
-
-            let rule;
-            let sub_iter = {
+            let rule = {
                 let nested_iter = self.stack.last_mut().unwrap();
-                rule = match nested_iter.next() {
+                match nested_iter.next() {
                     Some(r) => r,
                     None => {
-                        nested_iter_finished = true;
+                        self.stack.pop();
                         continue;
                     },
-                };
-
-                match *rule {
-                    CssRule::Namespace(_) |
-                    CssRule::Style(_) |
-                    CssRule::FontFace(_) |
-                    CssRule::CounterStyle(_) |
-                    CssRule::Viewport(_) |
-                    CssRule::Keyframes(_) |
-                    CssRule::Page(_) |
-                    CssRule::FontFeatureValues(_) => return Some(rule),
-                    CssRule::Import(ref import_rule) => {
-                        let import_rule = import_rule.read_with(self.guard);
-                        if !C::process_import(
-                            self.guard,
-                            self.device,
-                            self.quirks_mode,
-                            import_rule,
-                        ) {
-                            continue;
-                        }
-                        import_rule.stylesheet.rules(self.guard).iter()
-                    },
-                    CssRule::Document(ref doc_rule) => {
-                        let doc_rule = doc_rule.read_with(self.guard);
-                        if !C::process_document(self.guard, self.device, self.quirks_mode, doc_rule)
-                        {
-                            continue;
-                        }
-                        doc_rule.rules.read_with(self.guard).0.iter()
-                    },
-                    CssRule::Media(ref lock) => {
-                        let media_rule = lock.read_with(self.guard);
-                        if !C::process_media(self.guard, self.device, self.quirks_mode, media_rule)
-                        {
-                            continue;
-                        }
-                        media_rule.rules.read_with(self.guard).0.iter()
-                    },
-                    CssRule::Supports(ref lock) => {
-                        let supports_rule = lock.read_with(self.guard);
-                        if !C::process_supports(
-                            self.guard,
-                            self.device,
-                            self.quirks_mode,
-                            supports_rule,
-                        ) {
-                            continue;
-                        }
-                        supports_rule.rules.read_with(self.guard).0.iter()
-                    },
                 }
             };
 
-            self.stack.push(sub_iter);
+            let mut effective = true;
+            let children = children_of_rule::<C>(rule, self.device, self.quirks_mode, self.guard, &mut effective);
+            if !effective {
+                continue;
+            }
+
+            if !children.as_slice().is_empty() {
+                self.stack.push(children);
+            }
+
             return Some(rule);
         }
 
@@ -180,6 +185,36 @@ pub trait NestedRuleIterationCondition {
 /// A struct that represents the condition that a rule applies to the document.
 pub struct EffectiveRules;
 
+impl EffectiveRules {
+    /// Returns whether a given rule is effective.
+    pub fn is_effective(
+        guard: &SharedRwLockReadGuard,
+        device: &Device,
+        quirks_mode: QuirksMode,
+        rule: &CssRule,
+    ) -> bool {
+        match *rule {
+            CssRule::Import(ref import_rule) => {
+            let import_rule = import_rule.read_with(guard);
+                Self::process_import(guard, device, quirks_mode, import_rule)
+            },
+            CssRule::Document(ref doc_rule) => {
+                let doc_rule = doc_rule.read_with(guard);
+                Self::process_document(guard, device, quirks_mode, doc_rule)
+            },
+            CssRule::Media(ref lock) => {
+                let media_rule = lock.read_with(guard);
+                Self::process_media(guard, device, quirks_mode, media_rule)
+            },
+            CssRule::Supports(ref lock) => {
+                let supports_rule = lock.read_with(guard);
+                Self::process_supports(guard, device, quirks_mode, supports_rule)
+            },
+            _ => true,
+        }
+    }
+}
+
 impl NestedRuleIterationCondition for EffectiveRules {
     fn process_import(
         guard: &SharedRwLockReadGuard,
@@ -260,3 +295,17 @@ impl NestedRuleIterationCondition for AllRules {
 ///
 /// NOTE: This iterator recurses into `@import` rules.
 pub type EffectiveRulesIterator<'a, 'b> = RulesIterator<'a, 'b, EffectiveRules>;
+
+impl<'a, 'b> EffectiveRulesIterator<'a, 'b> {
+    /// Returns an iterator over the effective children of a rule, even if
+    /// `rule` itself is not effective.
+    pub fn effective_children(
+        device: &'a Device,
+        quirks_mode: QuirksMode,
+        guard: &'a SharedRwLockReadGuard<'b>,
+        rule: &'a CssRule,
+    ) -> Self {
+        let children = children_of_rule::<AllRules>(rule, device, quirks_mode, guard, &mut false);
+        EffectiveRulesIterator::new(device, quirks_mode, guard, children)
+    }
+}

components/style/stylesheets/stylesheet.rs

@@ -243,7 +243,7 @@ pub trait StylesheetInDocument: ::std::fmt::Debug {
     where
         C: NestedRuleIterationCondition,
     {
-        RulesIterator::new(device, self.quirks_mode(guard), guard, self.rules(guard))
+        RulesIterator::new(device, self.quirks_mode(guard), guard, self.rules(guard).iter())
     }
 
     /// Returns whether the style-sheet applies for the current device.

components/style/stylist.rs

@@ -13,6 +13,7 @@ use crate::font_metrics::FontMetricsProvider;
 use crate::gecko_bindings::structs::{ServoStyleSetSizes, StyleRuleInclusion};
 use crate::invalidation::element::invalidation_map::InvalidationMap;
 use crate::invalidation::media_queries::{EffectiveMediaQueryResults, ToMediaListKey};
+use crate::invalidation::stylesheets::RuleChangeKind;
 use crate::media_queries::Device;
 use crate::properties::{self, CascadeMode, ComputedValues};
 use crate::properties::{AnimationDeclarations, PropertyDeclarationBlock};
@@ -605,6 +606,18 @@ impl Stylist {
             .remove_stylesheet(Some(&self.device), sheet, guard)
     }
 
+    /// Notify of a change of a given rule.
+    pub fn rule_changed(
+        &mut self,
+        sheet: &StylistSheet,
+        rule: &CssRule,
+        guard: &SharedRwLockReadGuard,
+        change_kind: RuleChangeKind,
+    ) {
+        self.stylesheets
+            .rule_changed(Some(&self.device), sheet, rule, guard, change_kind)
+    }
+
     /// Appends a new stylesheet to the current set.
     #[inline]
     pub fn sheet_count(&self, origin: Origin) -> usize {