Bug 1367225 - Make replace_rules returning boolean. r?birtles draft
authorHiroyuki Ikezoe <hikezoe@mozilla.com>
Wed, 24 May 2017 13:44:48 +0900
changeset 583500 09d9d0bd21bcf0fab951038c75115b07f8d12448
parent 583499 78ca9145472c444090b94e2cb6cf1d92ee6857ab
child 583501 f6176095a39f603984f9823c361ddf866de65ec1
push id60412
push userhikezoe@mozilla.com
push dateWed, 24 May 2017 04:47:52 +0000
reviewersbirtles
bugs1367225
milestone55.0a1
Bug 1367225 - Make replace_rules returning boolean. r?birtles We only use whether the return value is IMPORTANT_RULES_CHANGED or not, so we can just return true if an important rules was changed in the function. Also, we can just return false in case of animation rules changes sine for animation we can ensure there is no importan rules. Because of these changes, replace_rule_node does not borrow |result| so that we can drop a scope there. MozReview-Commit-ID: 68e5ReTs6Ly
servo/components/style/matching.rs
servo/components/style/traversal.rs
--- a/servo/components/style/matching.rs
+++ b/servo/components/style/matching.rs
@@ -958,88 +958,84 @@ pub trait MatchMethods : TElement {
         self.accumulate_damage_for(shared_context,
                                    restyle,
                                    old_values,
                                    new_values,
                                    pseudo)
     }
 
     /// Updates the rule nodes without re-running selector matching, using just
-    /// the rule tree. Returns RulesChanged which indicates whether the rule nodes changed
-    /// and whether the important rules changed.
+    /// the rule tree. Returns true if an !important rule was replaced.
     fn replace_rules(&self,
                      replacements: RestyleReplacements,
                      context: &StyleContext<Self>,
                      data: &mut ElementData)
-                     -> RulesChanged {
+                     -> bool {
         use properties::PropertyDeclarationBlock;
         use shared_lock::Locked;
 
         let element_styles = &mut data.styles_mut();
         let primary_rules = &mut element_styles.primary.rules;
-        let mut result = RulesChanged::empty();
+        let mut result = false;
+
+        let replace_rule_node = |level: CascadeLevel,
+                                 pdb: Option<&Arc<Locked<PropertyDeclarationBlock>>>,
+                                 path: &mut StrongRuleNode| -> bool {
+            let new_node = context.shared.stylist.rule_tree()
+                .update_rule_at_level(level, pdb, path, &context.shared.guards);
+            match new_node {
+                Some(n) => {
+                    *path = n;
+                    level.is_important()
+                },
+                None => false,
+            }
+        };
 
-        {
-            let mut replace_rule_node = |level: CascadeLevel,
-                                         pdb: Option<&Arc<Locked<PropertyDeclarationBlock>>>,
-                                         path: &mut StrongRuleNode| {
-                let new_node = context.shared.stylist.rule_tree()
-                    .update_rule_at_level(level, pdb, path, &context.shared.guards);
-                if let Some(n) = new_node {
-                    *path = n;
-                    if level.is_important() {
-                        result.insert(IMPORTANT_RULES_CHANGED);
-                    } else {
-                        result.insert(NORMAL_RULES_CHANGED);
-                    }
-                }
+        // Animation restyle hints are processed prior to other restyle
+        // hints in the animation-only traversal.
+        //
+        // Non-animation restyle hints will be processed in a subsequent
+        // normal traversal.
+        if replacements.intersects(RestyleReplacements::for_animations()) {
+            debug_assert!(context.shared.traversal_flags.for_animation_only());
+
+            if replacements.contains(RESTYLE_SMIL) {
+                replace_rule_node(CascadeLevel::SMILOverride,
+                                  self.get_smil_override(),
+                                  primary_rules);
+            }
+
+            let replace_rule_node_for_animation = |level: CascadeLevel,
+                                                   primary_rules: &mut StrongRuleNode| {
+                let animation_rule = self.get_animation_rule_by_cascade(level);
+                replace_rule_node(level,
+                                  animation_rule.as_ref(),
+                                  primary_rules);
             };
 
-            // Animation restyle hints are processed prior to other restyle
-            // hints in the animation-only traversal.
-            //
-            // Non-animation restyle hints will be processed in a subsequent
-            // normal traversal.
-            if replacements.intersects(RestyleReplacements::for_animations()) {
-                debug_assert!(context.shared.traversal_flags.for_animation_only());
-
-                if replacements.contains(RESTYLE_SMIL) {
-                    replace_rule_node(CascadeLevel::SMILOverride,
-                                      self.get_smil_override(),
-                                      primary_rules);
-                }
+            // Apply Transition rules and Animation rules if the corresponding restyle hint
+            // is contained.
+            if replacements.contains(RESTYLE_CSS_TRANSITIONS) {
+                replace_rule_node_for_animation(CascadeLevel::Transitions,
+                                                primary_rules);
+            }
 
-                let mut replace_rule_node_for_animation = |level: CascadeLevel,
-                                                           primary_rules: &mut StrongRuleNode| {
-                    let animation_rule = self.get_animation_rule_by_cascade(level);
-                    replace_rule_node(level,
-                                      animation_rule.as_ref(),
-                                      primary_rules);
-                };
-
-                // Apply Transition rules and Animation rules if the corresponding restyle hint
-                // is contained.
-                if replacements.contains(RESTYLE_CSS_TRANSITIONS) {
-                    replace_rule_node_for_animation(CascadeLevel::Transitions,
-                                                    primary_rules);
-                }
-
-                if replacements.contains(RESTYLE_CSS_ANIMATIONS) {
-                    replace_rule_node_for_animation(CascadeLevel::Animations,
-                                                    primary_rules);
-                }
-            } else if replacements.contains(RESTYLE_STYLE_ATTRIBUTE) {
-                let style_attribute = self.style_attribute();
-                replace_rule_node(CascadeLevel::StyleAttributeNormal,
-                                  style_attribute,
-                                  primary_rules);
-                replace_rule_node(CascadeLevel::StyleAttributeImportant,
-                                  style_attribute,
-                                  primary_rules);
+            if replacements.contains(RESTYLE_CSS_ANIMATIONS) {
+                replace_rule_node_for_animation(CascadeLevel::Animations,
+                                                primary_rules);
             }
+        } else if replacements.contains(RESTYLE_STYLE_ATTRIBUTE) {
+            let style_attribute = self.style_attribute();
+            result |= replace_rule_node(CascadeLevel::StyleAttributeNormal,
+                                        style_attribute,
+                                        primary_rules);
+            result |= replace_rule_node(CascadeLevel::StyleAttributeImportant,
+                                        style_attribute,
+                                        primary_rules);
         }
 
         result
     }
 
     /// Attempts to share a style with another node. This method is unsafe
     /// because it depends on the `style_sharing_candidate_cache` having only
     /// live nodes in it, and we have no way to guarantee that at the type
--- a/servo/components/style/traversal.rs
+++ b/servo/components/style/traversal.rs
@@ -765,21 +765,21 @@ fn compute_style<E, D>(_traversal: &D,
             // Perform the matching and cascading.
             element.match_and_cascade(
                 context,
                 data,
                 StyleSharingBehavior::Allow
             )
         }
         CascadeWithReplacements(flags) => {
-            let rules_changed = element.replace_rules(flags, context, data);
+            let important_rules_changed = element.replace_rules(flags, context, data);
             element.cascade_primary_and_pseudos(
                 context,
                 data,
-                rules_changed.important_rules_changed()
+                important_rules_changed
             )
         }
         CascadeOnly => {
             element.cascade_primary_and_pseudos(
                 context,
                 data,
                 /* important_rules_changed = */ false
             )