Bug 1502893 - Don't match document author rules if not needed for revalidation. r=heycam,firefox-style-system-reviewers
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 05 Nov 2018 00:05:12 +0000
changeset 444374 9f0e2fbf4d1593dafd43b43c6fcd5f89d8a0a94a
parent 444373 59b139264533a23ab6355ceed360f4bca33ea899
child 444375 dd629ef17f6246da8e71cd8d94b9115d64a3c31e
push id72293
push useremilio@crisal.io
push dateMon, 05 Nov 2018 09:23:13 +0000
treeherderautoland@9f0e2fbf4d15 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam, firefox-style-system-reviewers
bugs1502893
milestone65.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1502893 - Don't match document author rules if not needed for revalidation. r=heycam,firefox-style-system-reviewers When you're in a ShadowRoot and can share style with a sibling, the sharing code is smart enough to skip document author rules. But then it could get confused if you also include document rules, since revalidation selectors are matched against these. This is not a correctness issue, because we're matching more than what we need, and avoid sharing if we failed. Also fix the detection for user rules in any_applicable_rule_data. Differential Revision: https://phabricator.services.mozilla.com/D10117
layout/style/crashtests/1502893.html
layout/style/crashtests/crashtests.list
servo/components/style/stylist.rs
new file mode 100644
--- /dev/null
+++ b/layout/style/crashtests/1502893.html
@@ -0,0 +1,29 @@
+<!DOCTYPE html>
+<style>
+  #animate_0[onrepeat=''] {}
+</style>
+<animate id="animate_0" />
+<animate id="animate_1" />
+<script>
+  window.CustomElement0 = class extends HTMLElement {
+    constructor() {
+      super();
+      this.attachShadow({
+        mode: 'open'
+      })
+    }
+    connectedCallback() {
+      this.shadowRoot.prepend(o2)
+      this.setAttribute('contenteditable', 'true')
+    }
+  }
+
+  customElements.define('custom-element-0', CustomElement0)
+  o1 = document.createElement('custom-element-0')
+  o2 = document.getElementById('animate_0')
+  o3 = document.getElementById('animate_1')
+  document.documentElement.appendChild(o1)
+  document.replaceChild(document.documentElement, document.documentElement)
+  o1.shadowRoot.prepend(o3)
+  o3.offsetTop;
+</script>
--- a/layout/style/crashtests/crashtests.list
+++ b/layout/style/crashtests/crashtests.list
@@ -289,8 +289,9 @@ load 1455108.html
 load 1457288.html
 load 1457985.html
 pref(dom.webcomponents.shadowdom.enabled,true) load 1468640.html
 load 1469076.html
 load 1475003.html
 load 1479681.html
 load 1488817.html
 load 1490012.html
+load 1502893.html
--- a/servo/components/style/stylist.rs
+++ b/servo/components/style/stylist.rs
@@ -600,21 +600,21 @@ impl Stylist {
 
         let mut maybe = false;
 
         let doc_author_rules_apply =
             element.each_applicable_non_document_style_rule_data(|data, _, _| {
                 maybe = maybe || f(&*data);
             });
 
-        if maybe || !doc_author_rules_apply {
-            return maybe;
+        if maybe || f(&self.cascade_data.user) {
+            return true;
         }
 
-        f(&self.cascade_data.author) || f(&self.cascade_data.user)
+        doc_author_rules_apply && f(&self.cascade_data.author)
     }
 
     /// Computes the style for a given "precomputed" pseudo-element, taking the
     /// universal rules and applying them.
     pub fn precomputed_values_for_pseudo<E>(
         &self,
         guards: &StylesheetGuards,
         pseudo: &PseudoElement,
@@ -1486,17 +1486,43 @@ impl Stylist {
         );
 
         // Note that, by the time we're revalidating, we're guaranteed that the
         // candidate and the entry have the same id, classes, and local name.
         // This means we're guaranteed to get the same rulehash buckets for all
         // the lookups, which means that the bitvecs are comparable. We verify
         // this in the caller by asserting that the bitvecs are same-length.
         let mut results = SmallBitVec::new();
-        for (data, _) in self.cascade_data.iter_origins() {
+
+        let matches_document_rules =
+            element.each_applicable_non_document_style_rule_data(|data, quirks_mode, host| {
+                matching_context.with_shadow_host(host, |matching_context| {
+                    data.selectors_for_cache_revalidation.lookup(
+                        element,
+                        quirks_mode,
+                        |selector_and_hashes| {
+                            results.push(matches_selector(
+                                &selector_and_hashes.selector,
+                                selector_and_hashes.selector_offset,
+                                Some(&selector_and_hashes.hashes),
+                                &element,
+                                matching_context,
+                                flags_setter,
+                            ));
+                            true
+                        },
+                    );
+                })
+            });
+
+        for (data, origin) in self.cascade_data.iter_origins() {
+            if origin == Origin::Author && !matches_document_rules {
+                continue;
+            }
+
             data.selectors_for_cache_revalidation.lookup(
                 element,
                 self.quirks_mode,
                 |selector_and_hashes| {
                     results.push(matches_selector(
                         &selector_and_hashes.selector,
                         selector_and_hashes.selector_offset,
                         Some(&selector_and_hashes.hashes),
@@ -1504,35 +1530,16 @@ impl Stylist {
                         &mut matching_context,
                         flags_setter,
                     ));
                     true
                 },
             );
         }
 
-        element.each_applicable_non_document_style_rule_data(|data, quirks_mode, host| {
-            matching_context.with_shadow_host(host, |matching_context| {
-                data.selectors_for_cache_revalidation.lookup(
-                    element,
-                    quirks_mode,
-                    |selector_and_hashes| {
-                        results.push(matches_selector(
-                            &selector_and_hashes.selector,
-                            selector_and_hashes.selector_offset,
-                            Some(&selector_and_hashes.hashes),
-                            &element,
-                            matching_context,
-                            flags_setter,
-                        ));
-                        true
-                    },
-                );
-            })
-        });
 
         results
     }
 
     /// Computes styles for a given declaration with parent_style.
     ///
     /// FIXME(emilio): the lack of pseudo / cascade flags look quite dubious,
     /// hopefully this is only used for some canvas font stuff.