From 19743a67ba8f8f48bbc26fb4a809473a0474d32b Mon Sep 17 00:00:00 2001
From: Bobby Holley <bobbyholley@gmail.com>
Date: Wed, 5 Apr 2017 18:55:09 -0700
Subject: [PATCH] Rearrange compute_style and eliminate MatchResults.

This simplifies things by avoiding the computation of MatchResults when we
don't need them.

We continue to compute rule_nodes_changed, even though we don't use it
for the moment, since we will need it soon for various incremental restyle
optimizations.

MozReview-Commit-ID: 4qsUYaD5Bs2
---
 components/style/matching.rs  | 27 +++------------------
 components/style/traversal.rs | 44 ++++++++++++++++-------------------
 2 files changed, 23 insertions(+), 48 deletions(-)

diff --git a/components/style/matching.rs b/components/style/matching.rs
index ed77ae54557..a6dd30c40e5 100644
--- a/components/style/matching.rs
+++ b/components/style/matching.rs
@@ -30,7 +30,7 @@ use stylist::ApplicableDeclarationBlock;
 
 /// Determines the amount of relations where we're going to share style.
 #[inline]
-fn relations_are_shareable(relations: &StyleRelations) -> bool {
+pub fn relations_are_shareable(relations: &StyleRelations) -> bool {
     use selectors::matching::*;
     !relations.intersects(AFFECTED_BY_ID_SELECTOR |
                           AFFECTED_BY_PSEUDO_ELEMENTS | AFFECTED_BY_STATE |
@@ -58,24 +58,6 @@ fn create_common_style_affecting_attributes_from_element<E: TElement>(element: &
     flags
 }
 
-/// The results returned from running selector matching on an element.
-pub struct MatchResults {
-    /// A set of style relations (different hints about what rules matched or
-    /// could have matched). This is necessary if the style will be shared.
-    /// If None, the style will not be shared.
-    pub primary_relations: Option<StyleRelations>,
-    /// Whether the rule nodes changed during selector matching.
-    pub rule_nodes_changed: bool,
-}
-
-impl MatchResults {
-    /// Returns true if the primary rule node is shareable with other nodes.
-    pub fn primary_is_shareable(&self) -> bool {
-        self.primary_relations.as_ref()
-            .map_or(false, relations_are_shareable)
-    }
-}
-
 /// Information regarding a style sharing candidate.
 ///
 /// Note that this information is stored in TLS and cleared after the traversal,
@@ -799,7 +781,7 @@ pub trait MatchMethods : TElement {
     fn match_element(&self,
                      context: &mut StyleContext<Self>,
                      data: &mut ElementData)
-                     -> MatchResults
+                     -> (StyleRelations, bool)
     {
         let mut applicable_declarations =
             Vec::<ApplicableDeclarationBlock>::with_capacity(16);
@@ -887,10 +869,7 @@ pub trait MatchMethods : TElement {
             primary_relations |= AFFECTED_BY_PSEUDO_ELEMENTS;
         }
 
-        MatchResults {
-            primary_relations: Some(primary_relations),
-            rule_nodes_changed: rule_nodes_changed,
-        }
+        (primary_relations, rule_nodes_changed)
     }
 
     /// Applies selector flags to an element, deferring mutations of the parent
diff --git a/components/style/traversal.rs b/components/style/traversal.rs
index a26404792d5..09792333d8d 100644
--- a/components/style/traversal.rs
+++ b/components/style/traversal.rs
@@ -10,7 +10,8 @@ use atomic_refcell::{AtomicRefCell, AtomicRefMut};
 use context::{SharedStyleContext, StyleContext, ThreadLocalStyleContext};
 use data::{ElementData, ElementStyles, StoredRestyleHint};
 use dom::{DirtyDescendants, NodeInfo, TElement, TNode};
-use matching::{MatchMethods, MatchResults};
+use matching::MatchMethods;
+use matching::relations_are_shareable;
 use restyle_hints::{RESTYLE_DESCENDANTS, RESTYLE_SELF};
 use selector_parser::RestyleDamage;
 use servo_config::opts;
@@ -586,7 +587,7 @@ fn compute_style<E, D>(_traversal: &D,
         }
     }
 
-    let match_results = match kind {
+    match kind {
         MatchAndCascade => {
             // Ensure the bloom filter is up to date.
             let dom_depth =
@@ -605,35 +606,30 @@ fn compute_style<E, D>(_traversal: &D,
 
             // Perform CSS selector matching.
             context.thread_local.statistics.elements_matched += 1;
-            element.match_element(context, &mut data)
+            let (primary_relations, _rule_nodes_changed) =
+                element.match_element(context, &mut data);
+
+            // Cascade properties and compute values.
+            element.cascade_element(context, &mut data);
+
+            // If the style is shareable, add it to the LRU cache.
+            if relations_are_shareable(&primary_relations) {
+                context.thread_local
+                       .style_sharing_candidate_cache
+                       .insert_if_possible(&element,
+                                           data.styles().primary.values(),
+                                           primary_relations);
+            }
         }
         CascadeWithReplacements(hint) => {
-            let rule_nodes_changed =
+            let _rule_nodes_changed =
                 element.cascade_with_replacements(hint, context, &mut data);
-            MatchResults {
-                primary_relations: None,
-                rule_nodes_changed: rule_nodes_changed,
-            }
+            element.cascade_element(context, &mut data);
         }
         CascadeOnly => {
-            MatchResults {
-                primary_relations: None,
-                rule_nodes_changed: false,
-            }
+            element.cascade_element(context, &mut data);
         }
     };
-
-    // Cascade properties and compute values.
-    element.cascade_element(context, &mut data);
-
-    // If the style is shareable, add it to the LRU cache.
-    if match_results.primary_is_shareable() {
-        context.thread_local
-               .style_sharing_candidate_cache
-               .insert_if_possible(&element,
-                                   data.styles().primary.values(),
-                                   match_results.primary_relations.unwrap());
-    }
 }
 
 fn preprocess_children<E, D>(traversal: &D,