From f74675639d615429ef5600353e993324e04cf9e4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= <emilio@crisal.io>
Date: Sun, 6 Aug 2017 13:13:07 +0200
Subject: [PATCH] style: Less ID revalidation selectors.

Avoid adding id selectors that are in the rule hash keyed by that ID to the list
of revalidation selectors.

This partially fixes bug 1369611 (we could look at the rule hash itself to avoid
inserting some more into the list of revalidation selectors).
---
 components/style/stylist.rs | 20 ++++++++++++++------
 tests/unit/style/stylist.rs | 11 ++++-------
 2 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/components/style/stylist.rs b/components/style/stylist.rs
index 18c6ce1be8a..a488ae461e9 100644
--- a/components/style/stylist.rs
+++ b/components/style/stylist.rs
@@ -1484,16 +1484,23 @@ struct StylistSelectorVisitor<'a> {
     state_dependencies: &'a mut ElementState,
 }
 
-fn component_needs_revalidation(c: &Component<SelectorImpl>) -> bool {
+fn component_needs_revalidation(
+    c: &Component<SelectorImpl>,
+    passed_rightmost_selector: bool,
+) -> bool {
     match *c {
+        Component::ID(_) => {
+            // TODO(emilio): This could also check that the ID is not already in
+            // the rule hash. In that case, we could avoid making this a
+            // revalidation selector too.
+            //
+            // See https://bugzilla.mozilla.org/show_bug.cgi?id=1369611
+            passed_rightmost_selector
+        }
         Component::AttributeInNoNamespaceExists { .. } |
         Component::AttributeInNoNamespace { .. } |
         Component::AttributeOther(_) |
         Component::Empty |
-        // FIXME(bz) We really only want to do this for some cases of id
-        // selectors.  See
-        // https://bugzilla.mozilla.org/show_bug.cgi?id=1369611
-        Component::ID(_) |
         Component::FirstChild |
         Component::LastChild |
         Component::OnlyChild |
@@ -1557,7 +1564,8 @@ impl<'a> SelectorVisitor for StylistSelectorVisitor<'a> {
 
     fn visit_simple_selector(&mut self, s: &Component<SelectorImpl>) -> bool {
         self.needs_revalidation =
-            self.needs_revalidation || component_needs_revalidation(s);
+            self.needs_revalidation ||
+            component_needs_revalidation(s, self.passed_rightmost_selector);
 
         match *s {
             Component::NonTSPseudoClass(ref p) => {
diff --git a/tests/unit/style/stylist.rs b/tests/unit/style/stylist.rs
index 53269b40522..dcc288a20b4 100644
--- a/tests/unit/style/stylist.rs
+++ b/tests/unit/style/stylist.rs
@@ -83,13 +83,12 @@ fn test_revalidation_selectors() {
         "div > span",
 
         // ID selectors.
-        "#foo1", // FIXME(bz) This one should not be a revalidation
-                // selector once we fix
-                // https://bugzilla.mozilla.org/show_bug.cgi?id=1369611
+        "#foo1",
         "#foo2::before",
         "#foo3 > span",
-        "#foo1 > span", // FIXME(bz) This one should not be a
-                        // revalidation selector either.
+        "#foo1 > span", // FIXME(bz): This one should not be a
+                        // revalidation selector, since #foo1 should be in the
+                        // rule hash.
 
         // Attribute selectors.
         "div[foo]",
@@ -131,8 +130,6 @@ fn test_revalidation_selectors() {
 
     let reference = parse_selectors(&[
         // ID selectors.
-        "#foo1",
-        "#foo2::before",
         "#foo3 > span",
         "#foo1 > span",