Bug 1358966 - Factor out a process that creates AnimationValue iterator from PropertyDeclarationBlock. r?manishearth draft
authorHiroyuki Ikezoe <hikezoe@mozilla.com>
Wed, 07 Jun 2017 08:48:05 +0900
changeset 589848 66d1d14102e0c5b3713b94ac2b6a953ff8e77d42
parent 589847 6dba15f240887b7b9addf88477d08070cdca1ed3
child 589849 86dda2f4ed634353885220c0045720db0a470944
push id62546
push userhikezoe@mozilla.com
push dateTue, 06 Jun 2017 23:48:51 +0000
reviewersmanishearth
bugs1358966
milestone55.0a1
Bug 1358966 - Factor out a process that creates AnimationValue iterator from PropertyDeclarationBlock. r?manishearth MozReview-Commit-ID: 2V1CGX3p6em
servo/components/style/properties/declaration_block.rs
servo/ports/geckolib/glue.rs
--- a/servo/components/style/properties/declaration_block.rs
+++ b/servo/components/style/properties/declaration_block.rs
@@ -6,22 +6,24 @@
 
 #![deny(missing_docs)]
 
 use context::QuirksMode;
 use cssparser::{DeclarationListParser, parse_important};
 use cssparser::{Parser, AtRuleParser, DeclarationParser, Delimiter};
 use error_reporting::ParseErrorReporter;
 use parser::{PARSING_MODE_DEFAULT, ParsingMode, ParserContext, log_css_error};
+use properties::animated_properties::AnimationValue;
 use shared_lock::Locked;
 use std::fmt;
 use std::slice::Iter;
 use style_traits::ToCss;
 use stylesheets::{CssRuleType, Origin, UrlExtraData};
 use stylesheets::{MallocSizeOf, MallocSizeOfFn};
+use values::computed::Context;
 use super::*;
 #[cfg(feature = "gecko")] use properties::animated_properties::AnimationValueMap;
 
 /// The animation rules.
 ///
 /// The first one is for Animation cascade level, and the second one is for
 /// Transition cascade level.
 pub struct AnimationRules<'a>(pub Option<&'a Arc<Locked<PropertyDeclarationBlock>>>,
@@ -97,16 +99,64 @@ impl<'a> Iterator for PropertyDeclaratio
         fn get_declaration(dec: &(PropertyDeclaration, Importance))
             -> &PropertyDeclaration {
             &dec.0
         }
         self.iter.next().map(get_declaration as fn(_) -> _)
     }
 }
 
+/// Iterator for AnimationValue to be generated from PropertyDeclarationBlock.
+pub struct AnimationValueIterator<'a, 'cx, 'cx_a:'cx> {
+    iter: Iter<'a, (PropertyDeclaration, Importance)>,
+    context: &'cx mut Context<'cx_a>,
+    default_values: &'a Arc<ComputedValues>,
+}
+
+impl<'a, 'cx, 'cx_a:'cx> AnimationValueIterator<'a, 'cx, 'cx_a> {
+    fn new(declarations: &'a PropertyDeclarationBlock,
+           context: &'cx mut Context<'cx_a>,
+           default_values: &'a Arc<ComputedValues>) -> AnimationValueIterator<'a, 'cx, 'cx_a> {
+        AnimationValueIterator {
+            iter: declarations.declarations().iter(),
+            context: context,
+            default_values: default_values,
+        }
+    }
+}
+
+impl<'a, 'cx, 'cx_a:'cx> Iterator for AnimationValueIterator<'a, 'cx, 'cx_a> {
+    type Item = (TransitionProperty, AnimationValue);
+    #[inline]
+    fn next(&mut self) -> Option<Self::Item> {
+        use properties::Importance;
+
+        loop {
+            let next = self.iter.next();
+            match next {
+                Some(&(ref decl, importance)) => {
+                    if importance == Importance::Normal {
+                        let property = TransitionProperty::from_declaration(decl);
+                        let animation = AnimationValue::from_declaration(decl, &mut self.context,
+                                                                         self.default_values);
+                        debug_assert!(property.is_none() == animation.is_none(),
+                                      "The failure condition of TransitionProperty::from_declaration \
+                                       and AnimationValue::from_declaration should be the same");
+                        // Skip the property if either ::from_declaration fails.
+                        if property.is_some() && animation.is_some() {
+                            return Some((property.unwrap(), animation.unwrap()));
+                        }
+                    }
+                },
+                None => return None,
+            }
+        }
+    }
+}
+
 impl fmt::Debug for PropertyDeclarationBlock {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         self.declarations.fmt(f)
     }
 }
 
 impl PropertyDeclarationBlock {
     /// Returns the number of declarations in the block.
@@ -143,16 +193,24 @@ impl PropertyDeclarationBlock {
 
     /// Iterate over only PropertyDeclaration.
     pub fn declarations_iter(&self) -> PropertyDeclarationIterator {
         PropertyDeclarationIterator {
             iter: self.declarations.iter(),
         }
     }
 
+    /// Return an iterator of (TransitionProperty, AnimationValue).
+    pub fn to_animation_value_iter<'a, 'cx, 'cx_a:'cx>(&'a self,
+                                                       context: &'cx mut Context<'cx_a>,
+                                                       default_values: &'a Arc<ComputedValues>)
+                                                       -> AnimationValueIterator<'a, 'cx, 'cx_a> {
+        AnimationValueIterator::new(self, context, default_values)
+    }
+
     /// Returns whether this block contains any declaration with `!important`.
     ///
     /// This is based on the `important_count` counter,
     /// which should be maintained whenever `declarations` is changed.
     // FIXME: make fields private and maintain it here in methods?
     pub fn any_important(&self) -> bool {
         self.important_count > 0
     }
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -2533,17 +2533,16 @@ fn create_context<'a>(per_doc_data: &'a 
 pub extern "C" fn Servo_GetComputedKeyframeValues(keyframes: RawGeckoKeyframeListBorrowed,
                                                   element: RawGeckoElementBorrowed,
                                                   style: ServoComputedValuesBorrowed,
                                                   raw_data: RawServoStyleSetBorrowed,
                                                   computed_keyframes: RawGeckoComputedKeyframeValuesListBorrowedMut)
 {
     use std::mem;
     use style::properties::LonghandIdSet;
-    use style::properties::declaration_block::Importance;
 
     let data = PerDocumentStyleData::from_ffi(raw_data).borrow();
     let metrics = get_metrics_provider_for_product();
     let style = ComputedValues::as_arc(&style);
 
     let element = GeckoElement(element);
     let parent_element = element.inheritance_parent();
     let parent_data = parent_element.as_ref().and_then(|e| e.borrow_data());
@@ -2568,38 +2567,17 @@ pub extern "C" fn Servo_GetComputedKeyfr
             if simulate_compute_values_failure(property) {
                 continue;
             }
 
             let declarations = unsafe { &*property.mServoDeclarationBlock.mRawPtr.clone() };
             let declarations = Locked::<PropertyDeclarationBlock>::as_arc(&declarations);
             let guard = declarations.read_with(&guard);
 
-            let anim_iter = guard.declarations()
-                            .iter()
-                            .filter_map(|&(ref decl, imp)| {
-                                if imp == Importance::Normal {
-                                    let property = TransitionProperty::from_declaration(decl);
-                                    let animation = AnimationValue::from_declaration(decl, &mut context,
-                                                                                     default_values);
-                                    debug_assert!(property.is_none() == animation.is_none(),
-                                                  "The failure condition of TransitionProperty::from_declaration \
-                                                   and AnimationValue::from_declaration should be the same");
-                                    // Skip the property if either ::from_declaration fails.
-                                    if property.is_none() || animation.is_none() {
-                                        None
-                                    } else {
-                                        Some((property.unwrap(), animation.unwrap()))
-                                    }
-                                } else {
-                                    None
-                                }
-                            });
-
-            for anim in anim_iter {
+            for anim in guard.to_animation_value_iter(&mut context, &default_values) {
                 if !seen.has_transition_property_bit(&anim.0) {
                     // This is safe since we immediately write to the uninitialized values.
                     unsafe { animation_values.set_len((property_index + 1) as u32) };
                     seen.set_transition_property_bit(&anim.0);
                     animation_values[property_index].mProperty = (&anim.0).into();
                     // We only make sure we have enough space for this variable,
                     // but didn't construct a default value for StyleAnimationValue,
                     // so we should zero it to avoid getting undefined behaviors.