Bug 1357583: Add a bunch of logging, shortcuts, and look also at the rightmost selector while invalidating sheets. r=heycam

MozReview-Commit-ID: 2XGcOCTa7MV

components/style/data.rs

@@ -388,6 +388,11 @@ impl StoredRestyleHint {
         self.0.insert(other.0)
     }
 
+    /// Contains whether the whole subtree is invalid.
+    pub fn contains_subtree(&self) -> bool {
+        self.0.contains(&RestyleHint::subtree())
+    }
+
     /// Insert another restyle hint, effectively resulting in the union of both.
     pub fn insert_from(&mut self, other: &Self) {
         self.0.insert_from(&other.0)

components/style/invalidation/mod.rs

@@ -71,6 +71,7 @@ impl StylesheetInvalidationSet {
 
     /// Mark the DOM tree styles' as fully invalid.
     pub fn invalidate_fully(&mut self) {
+        debug!("StylesheetInvalidationSet::invalidate_fully");
         self.invalid_scopes.clear();
         self.fully_invalid = true;
     }
@@ -84,9 +85,15 @@ impl StylesheetInvalidationSet {
         stylesheet: &Stylesheet,
         guard: &SharedRwLockReadGuard)
     {
-        if self.fully_invalid ||
+        debug!("StylesheetInvalidationSet::collect_invalidations_for");
-           stylesheet.disabled() ||
+        if self.fully_invalid {
+            debug!(" > Fully invalid already");
+            return;
+        }
+
+        if stylesheet.disabled() ||
            !stylesheet.is_effective_for_device(stylist.device(), guard) {
+            debug!(" > Stylesheet was not effective");
             return; // Nothing to do here.
         }
 
@@ -94,6 +101,9 @@ impl StylesheetInvalidationSet {
             stylesheet.rules.read_with(guard),
             stylist,
             guard);
+
+        debug!(" > resulting invalidations: {:?}", self.invalid_scopes);
+        debug!(" > fully_invalid: {}", self.fully_invalid);
     }
 
     /// Clears the invalidation set, invalidating elements as needed if
@@ -126,13 +136,25 @@ impl StylesheetInvalidationSet {
             return false;
         }
 
+        if let Some(ref r) = data.get_restyle() {
+            if r.hint.contains_subtree() {
+                debug!("process_invalidations_in_subtree: {:?} was already invalid",
+                       element);
+                return false;
+            }
+        }
+
         if self.fully_invalid {
+            debug!("process_invalidations_in_subtree: fully_invalid({:?})",
+                   element);
             data.ensure_restyle().hint.insert(StoredRestyleHint::subtree());
             return true;
         }
 
         for scope in &self.invalid_scopes {
             if scope.matches(element) {
+                debug!("process_invalidations_in_subtree: {:?} matched {:?}",
+                       element, scope);
                 data.ensure_restyle().hint.insert(StoredRestyleHint::subtree());
                 return true;
             }
@@ -151,6 +173,8 @@ impl StylesheetInvalidationSet {
         }
 
         if any_children_invalid {
+            debug!("Children of {:?} changed, setting dirty descendants",
+                   element);
             unsafe { element.set_dirty_descendants() }
         }
 
@@ -171,6 +195,27 @@ impl StylesheetInvalidationSet {
         }
     }
 
+    fn scan_component(
+        component: &Component<SelectorImpl>,
+        scope: &mut Option<InvalidationScope>)
+    {
+        match *component {
+            Component::Class(ref class) => {
+                if scope.as_ref().map_or(true, |s| !s.is_id()) {
+                    *scope = Some(InvalidationScope::Class(class.clone()));
+                }
+            }
+            Component::ID(ref id) => {
+                if scope.is_none() {
+                    *scope = Some(InvalidationScope::ID(id.clone()));
+                }
+            }
+            _ => {
+                // Ignore everything else, at least for now.
+            }
+        }
+    }
+
     /// Collect a style scopes for a given selector.
     ///
     /// We look at the outermost class or id selector to the left of an ancestor
@@ -179,29 +224,30 @@ impl StylesheetInvalidationSet {
     /// We prefer id scopes to class scopes, and outermost scopes to innermost
     /// scopes (to reduce the amount of traversal we need to do).
     fn collect_scopes(&mut self, selector: &Selector<SelectorImpl>) {
+        debug!("StylesheetInvalidationSet::collect_scopes({:?})", selector);
+
         let mut scope: Option<InvalidationScope> = None;
 
-        let iter = selector.inner.complex.iter_ancestors();
+        let mut scan = true;
-        for component in iter {
+        let mut iter = selector.inner.complex.iter();
-            match *component {
+
-                Component::Class(ref class) => {
+        loop {
-                    if scope.as_ref().map_or(true, |s| !s.is_id()) {
+            for component in &mut iter {
-                        scope = Some(InvalidationScope::Class(class.clone()));
+                if scan {
-                    }
+                    Self::scan_component(component, &mut scope);
                 }
-                Component::ID(ref id) => {
+            }
-                    if scope.is_none() {
+            match iter.next_sequence() {
-                        scope = Some(InvalidationScope::ID(id.clone()));
+                None => break,
-                    }
+                Some(combinator) => {
-                }
+                    scan = combinator.is_ancestor();
-                _ => {
-                    // Ignore everything else, at least for now.
                 }
             }
         }
 
         match scope {
             Some(s) => {
+                debug!(" > Found scope: {:?}", s);
                 self.invalid_scopes.insert(s);
             }
             None => {
@@ -226,6 +272,7 @@ impl StylesheetInvalidationSet {
         -> bool
     {
         use stylesheets::CssRule::*;
+        debug!("StylesheetInvalidationSet::collect_invalidations_for_rule");
         debug_assert!(!self.fully_invalid, "Not worth to be here!");
 
         match *rule {
@@ -281,6 +328,7 @@ impl StylesheetInvalidationSet {
             Viewport(..) => {
                 debug!(" > Found unsupported rule, marking the whole subtree \
                        invalid.");
+
                 // TODO(emilio): Can we do better here?
                 //
                 // At least in `@page`, we could check the relevant media, I

components/style/restyle_hints.rs

@@ -368,7 +368,7 @@ impl RestyleHint {
     /// Returns whether this `RestyleHint` represents at least as much restyle
     /// work as the specified one.
     #[inline]
-    pub fn contains(&mut self, other: &Self) -> bool {
+    pub fn contains(&self, other: &Self) -> bool {
         self.match_under_self.contains(other.match_under_self) &&
         (self.match_later_siblings & other.match_later_siblings) == other.match_later_siblings &&
         self.replacements.contains(other.replacements)

components/style/stylesheet_set.rs

@@ -78,6 +78,7 @@ impl StylesheetSet {
         unique_id: u64,
         guard: &SharedRwLockReadGuard)
     {
+        debug!("StylesheetSet::append_stylesheet");
         self.remove_stylesheet_if_present(unique_id);
         self.entries.push(StylesheetSetEntry {
             unique_id: unique_id,
@@ -98,6 +99,7 @@ impl StylesheetSet {
         unique_id: u64,
         guard: &SharedRwLockReadGuard)
     {
+        debug!("StylesheetSet::prepend_stylesheet");
         self.remove_stylesheet_if_present(unique_id);
         self.entries.insert(0, StylesheetSetEntry {
             unique_id: unique_id,
@@ -119,6 +121,7 @@ impl StylesheetSet {
         before_unique_id: u64,
         guard: &SharedRwLockReadGuard)
     {
+        debug!("StylesheetSet::insert_stylesheet_before");
         self.remove_stylesheet_if_present(unique_id);
         let index = self.entries.iter().position(|x| {
             x.unique_id == before_unique_id
@@ -136,6 +139,7 @@ impl StylesheetSet {
 
     /// Remove a given stylesheet from the set.
     pub fn remove_stylesheet(&mut self, unique_id: u64) {
+        debug!("StylesheetSet::remove_stylesheet");
         self.remove_stylesheet_if_present(unique_id);
         self.dirty = true;
         // FIXME(emilio): We can do better!
@@ -144,6 +148,7 @@ impl StylesheetSet {
 
     /// Notes that the author style has been disabled for this document.
     pub fn set_author_style_disabled(&mut self, disabled: bool) {
+        debug!("StylesheetSet::set_author_style_disabled");
         if self.author_style_disabled == disabled {
             return;
         }
@@ -164,6 +169,7 @@ impl StylesheetSet {
                     -> StylesheetIterator
         where E: TElement,
     {
+        debug!("StylesheetSet::flush");
         debug_assert!(self.dirty);
 
         self.dirty = false;