Bug 1381635 - stylo: Don't ignore visited state when deciding to share style contexts; r?bz draft
authorManish Goregaokar <manishearth@gmail.com>
Tue, 18 Jul 2017 10:00:54 -0700
changeset 610844 f3102c1e2b979b1b10324c29e44a1726de5767db
parent 610746 3604eed1eb097a3039a6424e6583b8b5cd827396
child 637959 9a1e0378a039db363574da65c1d1f175e54e0feb
push id69004
push userbmo:manishearth@gmail.com
push dateTue, 18 Jul 2017 19:39:24 +0000
reviewersbz
bugs1381635
milestone56.0a1
Bug 1381635 - stylo: Don't ignore visited state when deciding to share style contexts; r?bz MozReview-Commit-ID: DqqnZbaqw0G
layout/style/test/stylo-failures.md
servo/components/style/sharing/checks.rs
servo/components/style/sharing/mod.rs
--- a/layout/style/test/stylo-failures.md
+++ b/layout/style/test/stylo-failures.md
@@ -62,18 +62,16 @@ to mochitest command.
 * Unit should be preserved after parsing servo/servo#15346
   * test_units_time.html [1]
 * getComputedStyle style doesn't contain custom properties bug 1336891
   * test_variables.html `custom property name` [2]
 * test_css_supports.html: issues around @supports syntax servo/servo#15482 [2]
 * test_author_specified_style.html: support serializing color as author specified bug 1348165 [27]
 * browser_newtab_share_rule_processors.js: agent style sheet sharing [1]
 * :visited support (bug 1328509)
-  * test_visited_reftests.html `inherit-keyword-1.xhtml` [2]
-  * test_visited_reftests.html `selector-descendant-2.xhtml`: bug 1381635 [4]
   * ... `mathml-links.html` [2]
 
 ## Assertions
 
 ## Need Gecko change
 
 * test_specified_value_serialization.html `-webkit-radial-gradient`: bug 1380259 [1]
 
--- a/servo/components/style/sharing/checks.rs
+++ b/servo/components/style/sharing/checks.rs
@@ -5,17 +5,16 @@
 //! Different checks done during the style sharing process in order to determine
 //! quickly whether it's worth to share style, and whether two different
 //! elements can indeed share the same style.
 
 use Atom;
 use bloom::StyleBloom;
 use context::{SelectorFlagsMap, SharedStyleContext};
 use dom::TElement;
-use element_state::*;
 use sharing::{StyleSharingCandidate, StyleSharingTarget};
 use stylearc::Arc;
 
 /// Whether, given two elements, they have pointer-equal computed values.
 ///
 /// Both elements need to be styled already.
 ///
 /// This is used to know whether we can share style across cousins (if the two
@@ -63,30 +62,16 @@ pub fn have_same_presentational_hints<E>
 pub fn have_same_class<E>(target: &mut StyleSharingTarget<E>,
                           candidate: &mut StyleSharingCandidate<E>)
                           -> bool
     where E: TElement,
 {
     target.class_list() == candidate.class_list()
 }
 
-/// Compare element and candidate state, but ignore visitedness.  Styles don't
-/// actually changed based on visitedness (since both possibilities are computed
-/// up front), so it's safe to share styles if visitedness differs.
-pub fn have_same_state_ignoring_visitedness<E>(element: E,
-                                               candidate: &StyleSharingCandidate<E>)
-                                               -> bool
-    where E: TElement,
-{
-    let state_mask = !IN_VISITED_OR_UNVISITED_STATE;
-    let state = element.get_state() & state_mask;
-    let candidate_state = candidate.element.get_state() & state_mask;
-    state == candidate_state
-}
-
 /// Whether a given element and a candidate match the same set of "revalidation"
 /// selectors.
 ///
 /// Revalidation selectors are those that depend on the DOM structure, like
 /// :first-child, etc, or on attributes that we don't check off-hand (pretty
 /// much every attribute selector except `id` and `class`.
 #[inline]
 pub fn revalidate<E>(target: &mut StyleSharingTarget<E>,
--- a/servo/components/style/sharing/mod.rs
+++ b/servo/components/style/sharing/mod.rs
@@ -638,17 +638,20 @@ impl<E: TElement> StyleSharingCandidateC
             miss!(Link)
         }
 
         if target.matches_user_and_author_rules() !=
             candidate.element.matches_user_and_author_rules() {
             miss!(UserAndAuthorRules)
         }
 
-        if !checks::have_same_state_ignoring_visitedness(target.element, candidate) {
+        // We do not ignore visited state here, because Gecko
+        // needs to store extra bits on visited style contexts,
+        // so these contexts cannot be shared
+        if target.element.get_state() != candidate.get_state() {
             miss!(State)
         }
 
         let element_id = target.element.get_id();
         let candidate_id = candidate.element.get_id();
         if element_id != candidate_id {
             // It's possible that there are no styles for either id.
             if checks::may_have_rules_for_ids(shared, element_id.as_ref(),