Bug 1646811 - servo: Combine AnimationAndTransitionDeclarations and AnimationRules.
authorMartin Robinson <mrobinson@igalia.com>
Thu, 18 Jun 2020 18:14:07 +0000
changeset 536356 e6d0a75ce3952ea7575d77c565e5dcd536b1ea03
parent 536355 0da5430c2b2eb7ad0592dc831be1945750813132
child 536357 61bedf23521b79c8c7e63a263c409f7d07272988
push id37520
push userdluca@mozilla.com
push dateFri, 19 Jun 2020 04:04:08 +0000
treeherdermozilla-central@d1a4f9157858 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1646811
milestone79.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 1646811 - servo: Combine AnimationAndTransitionDeclarations and AnimationRules. These two structs are very similar, so we can combine them. Depends on D80243 Differential Revision: https://phabricator.services.mozilla.com/D80244
servo/components/style/animation.rs
servo/components/style/dom.rs
servo/components/style/properties/declaration_block.rs
servo/components/style/rule_collector.rs
servo/components/style/style_resolver.rs
servo/components/style/stylist.rs
--- a/servo/components/style/animation.rs
+++ b/servo/components/style/animation.rs
@@ -9,16 +9,17 @@
 
 use crate::bezier::Bezier;
 use crate::context::{CascadeInputs, SharedStyleContext};
 use crate::dom::{OpaqueNode, TDocument, TElement, TNode};
 use crate::properties::animated_properties::{AnimationValue, AnimationValueMap};
 use crate::properties::longhands::animation_direction::computed_value::single_value::T as AnimationDirection;
 use crate::properties::longhands::animation_fill_mode::computed_value::single_value::T as AnimationFillMode;
 use crate::properties::longhands::animation_play_state::computed_value::single_value::T as AnimationPlayState;
+use crate::properties::AnimationDeclarations;
 use crate::properties::{
     ComputedValues, Importance, LonghandId, LonghandIdSet, PropertyDeclarationBlock,
     PropertyDeclarationId,
 };
 use crate::rule_tree::CascadeLevel;
 use crate::selector_parser::PseudoElement;
 use crate::shared_lock::{Locked, SharedRwLock};
 use crate::style_resolver::StyleResolverForElement;
@@ -1230,68 +1231,51 @@ impl DocumentAnimationSet {
             .and_then(|set| set.get_value_map_for_active_transitions(time))
             .map(|map| {
                 let block = PropertyDeclarationBlock::from_animation_value_map(&map);
                 Arc::new(shared_lock.wrap(block))
             })
     }
 
     /// Get all the animation declarations for the given key, returning an empty
-    /// `AnimationAndTransitionDeclarations` if there are no animations.
+    /// `AnimationDeclarations` if there are no animations.
     pub(crate) fn get_all_declarations(
         &self,
         key: &AnimationSetKey,
         time: f64,
         shared_lock: &SharedRwLock,
-    ) -> AnimationAndTransitionDeclarations {
+    ) -> AnimationDeclarations {
         let sets = self.sets.read();
         let set = match sets.get(key) {
             Some(set) => set,
             None => return Default::default(),
         };
 
         let animations = set.get_value_map_for_active_animations(time).map(|map| {
             let block = PropertyDeclarationBlock::from_animation_value_map(&map);
             Arc::new(shared_lock.wrap(block))
         });
         let transitions = set.get_value_map_for_active_transitions(time).map(|map| {
             let block = PropertyDeclarationBlock::from_animation_value_map(&map);
             Arc::new(shared_lock.wrap(block))
         });
-        AnimationAndTransitionDeclarations {
+        AnimationDeclarations {
             animations,
             transitions,
         }
     }
 
     /// Cancel all animations for set at the given key.
     pub(crate) fn cancel_all_animations_for_key(&self, key: &AnimationSetKey) {
         if let Some(set) = self.sets.write().get_mut(key) {
             set.cancel_all_animations();
         }
     }
 }
 
-/// A set of property declarations for a node, including animations and
-/// transitions.
-#[derive(Default)]
-pub struct AnimationAndTransitionDeclarations {
-    /// Declarations for animations.
-    pub animations: Option<Arc<Locked<PropertyDeclarationBlock>>>,
-    /// Declarations for transitions.
-    pub transitions: Option<Arc<Locked<PropertyDeclarationBlock>>>,
-}
-
-impl AnimationAndTransitionDeclarations {
-    /// Whether or not this `AnimationAndTransitionDeclarations` is empty.
-    pub(crate) fn is_empty(&self) -> bool {
-        self.animations.is_none() && self.transitions.is_none()
-    }
-}
-
 /// Kick off any new transitions for this node and return all of the properties that are
 /// transitioning. This is at the end of calculating style for a single node.
 pub fn start_transitions_if_applicable(
     context: &SharedStyleContext,
     old_style: &ComputedValues,
     new_style: &Arc<ComputedValues>,
     animation_state: &mut ElementAnimationSet,
 ) -> LonghandIdSet {
--- a/servo/components/style/dom.rs
+++ b/servo/components/style/dom.rs
@@ -10,17 +10,17 @@
 use crate::applicable_declarations::ApplicableDeclarationBlock;
 use crate::context::SharedStyleContext;
 #[cfg(feature = "gecko")]
 use crate::context::{PostAnimationTasks, UpdateAnimationsTasks};
 use crate::data::ElementData;
 use crate::element_state::ElementState;
 use crate::font_metrics::FontMetricsProvider;
 use crate::media_queries::Device;
-use crate::properties::{AnimationRules, ComputedValues, PropertyDeclarationBlock};
+use crate::properties::{AnimationDeclarations, ComputedValues, PropertyDeclarationBlock};
 use crate::selector_parser::{AttrValue, Lang, PseudoElement, SelectorImpl};
 use crate::shared_lock::{Locked, SharedRwLock};
 use crate::stylist::CascadeData;
 use crate::traversal_flags::TraversalFlags;
 use crate::{Atom, LocalName, Namespace, WeakAtom};
 use atomic_refcell::{AtomicRef, AtomicRefMut};
 use selectors::matching::{ElementSelectorFlags, QuirksMode, VisitedHandlingMode};
 use selectors::sink::Push;
@@ -474,22 +474,25 @@ pub trait TElement:
     /// Get this element's SMIL override declarations.
     fn smil_override(&self) -> Option<ArcBorrow<Locked<PropertyDeclarationBlock>>> {
         None
     }
 
     /// Get the combined animation and transition rules.
     ///
     /// FIXME(emilio): Is this really useful?
-    fn animation_rules(&self, context: &SharedStyleContext) -> AnimationRules {
+    fn animation_declarations(&self, context: &SharedStyleContext) -> AnimationDeclarations {
         if !self.may_have_animations() {
-            return AnimationRules(None, None);
+            return Default::default();
         }
 
-        AnimationRules(self.animation_rule(context), self.transition_rule(context))
+        AnimationDeclarations {
+            animations: self.animation_rule(context),
+            transitions: self.transition_rule(context),
+        }
     }
 
     /// Get this element's animation rule.
     fn animation_rule(
         &self,
         _: &SharedStyleContext,
     ) -> Option<Arc<Locked<PropertyDeclarationBlock>>>;
 
--- a/servo/components/style/properties/declaration_block.rs
+++ b/servo/components/style/properties/declaration_block.rs
@@ -23,32 +23,33 @@ use itertools::Itertools;
 use selectors::SelectorList;
 use smallbitvec::{self, SmallBitVec};
 use smallvec::SmallVec;
 use std::fmt::{self, Write};
 use std::iter::{DoubleEndedIterator, Zip};
 use std::slice::Iter;
 use style_traits::{CssWriter, ParseError, ParsingMode, StyleParseErrorKind, ToCss};
 
-/// The animation rules.
-///
-/// The first one is for Animation cascade level, and the second one is for
-/// Transition cascade level.
-pub struct AnimationRules(
-    pub Option<Arc<Locked<PropertyDeclarationBlock>>>,
-    pub Option<Arc<Locked<PropertyDeclarationBlock>>>,
-);
+/// A set of property declarations including animations and transitions.
+#[derive(Default)]
+pub struct AnimationDeclarations {
+    /// Declarations for animations.
+    pub animations: Option<Arc<Locked<PropertyDeclarationBlock>>>,
+    /// Declarations for transitions.
+    pub transitions: Option<Arc<Locked<PropertyDeclarationBlock>>>,
+}
 
-impl AnimationRules {
-    /// Returns whether these animation rules represents an actual rule or not.
-    pub fn is_empty(&self) -> bool {
-        self.0.is_none() && self.1.is_none()
+impl AnimationDeclarations {
+    /// Whether or not this `AnimationDeclarations` is empty.
+    pub(crate) fn is_empty(&self) -> bool {
+        self.animations.is_none() && self.transitions.is_none()
     }
 }
 
+
 /// An enum describes how a declaration should update
 /// the declaration block.
 #[derive(Clone, Copy, Debug, Eq, PartialEq)]
 enum DeclarationUpdate {
     /// The given declaration doesn't update anything.
     None,
     /// The given declaration is new, and should be append directly.
     Append,
--- a/servo/components/style/rule_collector.rs
+++ b/servo/components/style/rule_collector.rs
@@ -1,17 +1,17 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 //! Collects a series of applicable rules for a given element.
 
 use crate::applicable_declarations::{ApplicableDeclarationBlock, ApplicableDeclarationList};
 use crate::dom::{TElement, TNode, TShadowRoot};
-use crate::properties::{AnimationRules, PropertyDeclarationBlock};
+use crate::properties::{AnimationDeclarations, PropertyDeclarationBlock};
 use crate::rule_tree::{CascadeLevel, ShadowCascadeOrder};
 use crate::selector_map::SelectorMap;
 use crate::selector_parser::PseudoElement;
 use crate::shared_lock::Locked;
 use crate::stylesheets::Origin;
 use crate::stylist::{AuthorStylesEnabled, Rule, RuleInclusion, Stylist};
 use crate::Atom;
 use selectors::matching::{ElementSelectorFlags, MatchingContext, MatchingMode};
@@ -65,17 +65,17 @@ where
     E: TElement,
 {
     element: E,
     rule_hash_target: E,
     stylist: &'a Stylist,
     pseudo_element: Option<&'a PseudoElement>,
     style_attribute: Option<ArcBorrow<'a, Locked<PropertyDeclarationBlock>>>,
     smil_override: Option<ArcBorrow<'a, Locked<PropertyDeclarationBlock>>>,
-    animation_rules: AnimationRules,
+    animation_declarations: AnimationDeclarations,
     rule_inclusion: RuleInclusion,
     rules: &'a mut ApplicableDeclarationList,
     context: &'a mut MatchingContext<'b, E::Impl>,
     flags_setter: &'a mut F,
     matches_user_and_author_rules: bool,
     matches_document_author_rules: bool,
     in_sort_scope: bool,
 }
@@ -87,17 +87,17 @@ where
 {
     /// Trivially construct a new collector.
     pub fn new(
         stylist: &'a Stylist,
         element: E,
         pseudo_element: Option<&'a PseudoElement>,
         style_attribute: Option<ArcBorrow<'a, Locked<PropertyDeclarationBlock>>>,
         smil_override: Option<ArcBorrow<'a, Locked<PropertyDeclarationBlock>>>,
-        animation_rules: AnimationRules,
+        animation_declarations: AnimationDeclarations,
         rule_inclusion: RuleInclusion,
         rules: &'a mut ApplicableDeclarationList,
         context: &'a mut MatchingContext<'b, E::Impl>,
         flags_setter: &'a mut F,
     ) -> Self {
         // When we're matching with matching_mode =
         // `ForStatelessPseudoeElement`, the "target" for the rule hash is the
         // element itself, since it's what's generating the pseudo-element.
@@ -118,17 +118,17 @@ where
 
         Self {
             element,
             rule_hash_target,
             stylist,
             pseudo_element,
             style_attribute,
             smil_override,
-            animation_rules,
+            animation_declarations,
             rule_inclusion,
             context,
             flags_setter,
             rules,
             matches_user_and_author_rules,
             matches_document_author_rules: matches_user_and_author_rules,
             in_sort_scope: false,
         }
@@ -445,27 +445,27 @@ where
                     so.clone_arc(),
                     CascadeLevel::SMILOverride,
                 ));
         }
 
         // The animations sheet (CSS animations, script-generated
         // animations, and CSS transitions that are no longer tied to CSS
         // markup).
-        if let Some(anim) = self.animation_rules.0.take() {
+        if let Some(anim) = self.animation_declarations.animations.take() {
             self.rules
                 .push(ApplicableDeclarationBlock::from_declarations(
                     anim,
                     CascadeLevel::Animations,
                 ));
         }
 
         // The transitions sheet (CSS transitions that are tied to CSS
         // markup).
-        if let Some(anim) = self.animation_rules.1.take() {
+        if let Some(anim) = self.animation_declarations.transitions.take() {
             self.rules
                 .push(ApplicableDeclarationBlock::from_declarations(
                     anim,
                     CascadeLevel::Transitions,
                 ));
         }
     }
 
--- a/servo/components/style/style_resolver.rs
+++ b/servo/components/style/style_resolver.rs
@@ -5,17 +5,17 @@
 //! Style resolution for a given element or pseudo-element.
 
 use crate::applicable_declarations::ApplicableDeclarationList;
 use crate::context::{CascadeInputs, ElementCascadeInputs, StyleContext};
 use crate::data::{EagerPseudoStyles, ElementStyles};
 use crate::dom::TElement;
 use crate::matching::MatchMethods;
 use crate::properties::longhands::display::computed_value::T as Display;
-use crate::properties::{AnimationRules, ComputedValues};
+use crate::properties::ComputedValues;
 use crate::rule_tree::StrongRuleNode;
 use crate::selector_parser::{PseudoElement, SelectorImpl};
 use crate::stylist::RuleInclusion;
 use log::Level::Trace;
 use selectors::matching::{ElementSelectorFlags, MatchingContext};
 use selectors::matching::{MatchingMode, VisitedHandlingMode};
 use servo_arc::Arc;
 
@@ -468,17 +468,17 @@ where
             };
 
             // Compute the primary rule node.
             stylist.push_applicable_declarations(
                 self.element,
                 implemented_pseudo.as_ref(),
                 self.element.style_attribute(),
                 self.element.smil_override(),
-                self.element.animation_rules(self.context.shared),
+                self.element.animation_declarations(self.context.shared),
                 self.rule_inclusion,
                 &mut applicable_declarations,
                 &mut matching_context,
                 &mut set_selector_flags,
             );
         }
 
         // FIXME(emilio): This is a hack for animations, and should go away.
@@ -547,17 +547,17 @@ where
 
         // NB: We handle animation rules for ::before and ::after when
         // traversing them.
         stylist.push_applicable_declarations(
             self.element,
             Some(pseudo_element),
             None,
             None,
-            AnimationRules(None, None),
+            /* animation_declarations = */ Default::default(),
             self.rule_inclusion,
             &mut applicable_declarations,
             &mut matching_context,
             &mut set_selector_flags,
         );
 
         if applicable_declarations.is_empty() {
             return None;
--- a/servo/components/style/stylist.rs
+++ b/servo/components/style/stylist.rs
@@ -10,17 +10,17 @@ use crate::dom::{TElement, TShadowRoot};
 use crate::element_state::{DocumentState, ElementState};
 use crate::font_metrics::FontMetricsProvider;
 #[cfg(feature = "gecko")]
 use crate::gecko_bindings::structs::{ServoStyleSetSizes, StyleRuleInclusion};
 use crate::invalidation::element::invalidation_map::InvalidationMap;
 use crate::invalidation::media_queries::{EffectiveMediaQueryResults, ToMediaListKey};
 use crate::media_queries::Device;
 use crate::properties::{self, CascadeMode, ComputedValues};
-use crate::properties::{AnimationRules, PropertyDeclarationBlock};
+use crate::properties::{AnimationDeclarations, PropertyDeclarationBlock};
 use crate::rule_cache::{RuleCache, RuleCacheConditions};
 use crate::rule_collector::{containing_shadow_ignoring_svg_use, RuleCollector};
 use crate::rule_tree::{CascadeLevel, RuleTree, StrongRuleNode, StyleSource};
 use crate::selector_map::{PrecomputedHashMap, PrecomputedHashSet, SelectorMap, SelectorMapEntry};
 use crate::selector_parser::{PerPseudoElementMap, PseudoElement, SelectorImpl, SnapshotMap};
 use crate::shared_lock::{Locked, SharedRwLockReadGuard, StylesheetGuards};
 use crate::stylesheet_set::{DataValidity, DocumentStylesheetSet, SheetRebuildKind};
 use crate::stylesheet_set::{DocumentStylesheetFlusher, SheetCollectionFlusher};
@@ -969,17 +969,17 @@ impl Stylist {
 
         matching_context.pseudo_element_matching_fn = matching_fn;
 
         self.push_applicable_declarations(
             element,
             Some(&pseudo),
             None,
             None,
-            AnimationRules(None, None),
+            /* animation_declarations = */ Default::default(),
             rule_inclusion,
             &mut declarations,
             &mut matching_context,
             &mut set_selector_flags,
         );
 
         if declarations.is_empty() && is_probe {
             return None;
@@ -999,17 +999,17 @@ impl Stylist {
             );
             matching_context.pseudo_element_matching_fn = matching_fn;
 
             self.push_applicable_declarations(
                 element,
                 Some(&pseudo),
                 None,
                 None,
-                AnimationRules(None, None),
+                /* animation_declarations = */ Default::default(),
                 rule_inclusion,
                 &mut declarations,
                 &mut matching_context,
                 &mut set_selector_flags,
             );
             if !declarations.is_empty() {
                 let rule_node = self.rule_tree.insert_ordered_rules_with_important(
                     declarations.drain(..).map(|a| a.for_rule_tree()),
@@ -1122,32 +1122,32 @@ impl Stylist {
 
     /// Returns the applicable CSS declarations for the given element.
     pub fn push_applicable_declarations<E, F>(
         &self,
         element: E,
         pseudo_element: Option<&PseudoElement>,
         style_attribute: Option<ArcBorrow<Locked<PropertyDeclarationBlock>>>,
         smil_override: Option<ArcBorrow<Locked<PropertyDeclarationBlock>>>,
-        animation_rules: AnimationRules,
+        animation_declarations: AnimationDeclarations,
         rule_inclusion: RuleInclusion,
         applicable_declarations: &mut ApplicableDeclarationList,
         context: &mut MatchingContext<E::Impl>,
         flags_setter: &mut F,
     ) where
         E: TElement,
         F: FnMut(&E, ElementSelectorFlags),
     {
         RuleCollector::new(
             self,
             element,
             pseudo_element,
             style_attribute,
             smil_override,
-            animation_rules,
+            animation_declarations,
             rule_inclusion,
             applicable_declarations,
             context,
             flags_setter,
         )
         .collect_all();
     }