Auto merge of #16671 - hiikezoe:animation-values-map, r=emilio

Pass AnimationValueMap raw pointer

This is a PR for https://bugzilla.mozilla.org/show_bug.cgi?id=1360776
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes do not require tests because it's for stylo.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16671)
<!-- Reviewable:end -->

components/style/build_gecko.rs

@@ -466,7 +466,6 @@ mod bindings {
             "mozilla::DefaultDelete",
             "mozilla::Side",
             "mozilla::binding_danger::AssertAndSuppressCleanupPolicy",
-            "RawServoAnimationValueMapBorrowed",
             "mozilla::LengthParsingMode",
             "mozilla::InheritTarget",
         ];
@@ -636,7 +635,6 @@ mod bindings {
             "RawGeckoNode",
             "RawGeckoAnimationValueList",
             "RawServoAnimationValue",
-            "RawServoAnimationValueMap",
             "RawServoDeclarationBlock",
             "RawServoStyleRule",
             "RawGeckoPresContext",
@@ -743,6 +741,7 @@ mod bindings {
             ServoOwnedType { name: "RawServoStyleSet", opaque: true },
             ServoOwnedType { name: "StyleChildrenIterator", opaque: true },
             ServoOwnedType { name: "ServoElementSnapshot", opaque: false },
+            ServoOwnedType { name: "RawServoAnimationValueMap", opaque: true },
         ];
         let servo_immutable_borrow_types = [
             "RawGeckoNode",

components/style/gecko/arc_types.rs

@@ -11,13 +11,12 @@
 use gecko_bindings::bindings::{RawServoMediaList, RawServoMediaRule, RawServoNamespaceRule, RawServoPageRule};
 use gecko_bindings::bindings::{RawServoStyleSheet, RawServoImportRule, RawServoSupportsRule};
 use gecko_bindings::bindings::{ServoComputedValues, ServoCssRules};
-use gecko_bindings::structs::{RawServoAnimationValue, RawServoAnimationValueMap};
 use gecko_bindings::structs::{RawServoDeclarationBlock, RawServoStyleRule};
+use gecko_bindings::structs::RawServoAnimationValue;
 use gecko_bindings::sugar::ownership::{HasArcFFI, HasFFI};
 use media_queries::MediaList;
-use parking_lot::RwLock;
 use properties::{ComputedValues, PropertyDeclarationBlock};
-use properties::animated_properties::{AnimationValue, AnimationValueMap};
+use properties::animated_properties::AnimationValue;
 use shared_lock::Locked;
 use stylesheets::{CssRules, Stylesheet, StyleRule, ImportRule, MediaRule};
 use stylesheets::{NamespaceRule, PageRule, SupportsRule};
@@ -62,9 +61,6 @@ impl_arc_ffi!(Locked<ImportRule> => RawServoImportRule
 impl_arc_ffi!(AnimationValue => RawServoAnimationValue
               [Servo_AnimationValue_AddRef, Servo_AnimationValue_Release]);
 
-impl_arc_ffi!(RwLock<AnimationValueMap> => RawServoAnimationValueMap
-              [Servo_AnimationValueMap_AddRef, Servo_AnimationValueMap_Release]);
-
 impl_arc_ffi!(Locked<MediaList> => RawServoMediaList
               [Servo_MediaList_AddRef, Servo_MediaList_Release]);
 

components/style/gecko/wrapper.rs

@@ -55,7 +55,6 @@ use gecko_bindings::structs::NODE_IS_NATIVE_ANONYMOUS;
 use gecko_bindings::sugar::ownership::HasArcFFI;
 use logical_geometry::WritingMode;
 use media_queries::Device;
-use parking_lot::RwLock;
 use properties::{ComputedValues, parse_style_attribute};
 use properties::{Importance, PropertyDeclaration, PropertyDeclarationBlock};
 use properties::animated_properties::{AnimationValue, AnimationValueMap, TransitionProperty};
@@ -423,17 +422,15 @@ fn selector_flags_to_node_flags(flags: ElementSelectorFlags) -> u32 {
 fn get_animation_rule(element: &GeckoElement,
                       cascade_level: CascadeLevel)
                       -> Option<Arc<Locked<PropertyDeclarationBlock>>> {
-    // FIXME(emilio): This is quite dumb, why an RwLock, it's local to this
+    use gecko_bindings::sugar::ownership::HasSimpleFFI;
-    // function?
-    //
     // Also, we should try to reuse the PDB, to avoid creating extra rule nodes.
-    let animation_values = Arc::new(RwLock::new(AnimationValueMap::new()));
+    let mut animation_values = AnimationValueMap::new();
     if unsafe { Gecko_GetAnimationRule(element.0,
                                        cascade_level,
-                                       HasArcFFI::arc_as_borrowed(&animation_values)) } {
+                                       AnimationValueMap::as_ffi_mut(&mut animation_values)) } {
         let shared_lock = &GLOBAL_STYLE_DATA.shared_lock;
         Some(Arc::new(shared_lock.wrap(
-            PropertyDeclarationBlock::from_animation_value_map(&animation_values.read()))))
+            PropertyDeclarationBlock::from_animation_value_map(&animation_values))))
     } else {
         None
     }

components/style/gecko_bindings/bindings.rs

@@ -15,7 +15,6 @@ use gecko_bindings::structs::RawGeckoFontFaceRuleList;
 use gecko_bindings::structs::RawGeckoNode;
 use gecko_bindings::structs::RawGeckoAnimationValueList;
 use gecko_bindings::structs::RawServoAnimationValue;
-use gecko_bindings::structs::RawServoAnimationValueMap;
 use gecko_bindings::structs::RawServoDeclarationBlock;
 use gecko_bindings::structs::RawServoStyleRule;
 use gecko_bindings::structs::RawGeckoPresContext;
@@ -222,9 +221,6 @@ pub struct RawServoImportRule(RawServoImportRuleVoid);
 pub type RawServoAnimationValueStrong = ::gecko_bindings::sugar::ownership::Strong<RawServoAnimationValue>;
 pub type RawServoAnimationValueBorrowed<'a> = &'a RawServoAnimationValue;
 pub type RawServoAnimationValueBorrowedOrNull<'a> = Option<&'a RawServoAnimationValue>;
-pub type RawServoAnimationValueMapStrong = ::gecko_bindings::sugar::ownership::Strong<RawServoAnimationValueMap>;
-pub type RawServoAnimationValueMapBorrowed<'a> = &'a RawServoAnimationValueMap;
-pub type RawServoAnimationValueMapBorrowedOrNull<'a> = Option<&'a RawServoAnimationValueMap>;
 pub type RawServoMediaListStrong = ::gecko_bindings::sugar::ownership::Strong<RawServoMediaList>;
 pub type RawServoMediaListBorrowed<'a> = &'a RawServoMediaList;
 pub type RawServoMediaListBorrowedOrNull<'a> = Option<&'a RawServoMediaList>;
@@ -272,6 +268,14 @@ pub type ServoElementSnapshotBorrowed<'a> = &'a ServoElementSnapshot;
 pub type ServoElementSnapshotBorrowedOrNull<'a> = Option<&'a ServoElementSnapshot>;
 pub type ServoElementSnapshotBorrowedMut<'a> = &'a mut ServoElementSnapshot;
 pub type ServoElementSnapshotBorrowedMutOrNull<'a> = Option<&'a mut ServoElementSnapshot>;
+pub type RawServoAnimationValueMapOwned = ::gecko_bindings::sugar::ownership::Owned<RawServoAnimationValueMap>;
+pub type RawServoAnimationValueMapOwnedOrNull = ::gecko_bindings::sugar::ownership::OwnedOrNull<RawServoAnimationValueMap>;
+pub type RawServoAnimationValueMapBorrowed<'a> = &'a RawServoAnimationValueMap;
+pub type RawServoAnimationValueMapBorrowedOrNull<'a> = Option<&'a RawServoAnimationValueMap>;
+pub type RawServoAnimationValueMapBorrowedMut<'a> = &'a mut RawServoAnimationValueMap;
+pub type RawServoAnimationValueMapBorrowedMutOrNull<'a> = Option<&'a mut RawServoAnimationValueMap>;
+enum RawServoAnimationValueMapVoid { }
+pub struct RawServoAnimationValueMap(RawServoAnimationValueMapVoid);
 pub type RawGeckoNodeBorrowed<'a> = &'a RawGeckoNode;
 pub type RawGeckoNodeBorrowedOrNull<'a> = Option<&'a RawGeckoNode>;
 pub type RawGeckoElementBorrowed<'a> = &'a RawGeckoElement;
@@ -373,14 +377,6 @@ extern "C" {
 extern "C" {
     pub fn Servo_AnimationValue_Release(ptr: RawServoAnimationValueBorrowed);
 }
-extern "C" {
-    pub fn Servo_AnimationValueMap_AddRef(ptr:
-                                              RawServoAnimationValueMapBorrowed);
-}
-extern "C" {
-    pub fn Servo_AnimationValueMap_Release(ptr:
-                                               RawServoAnimationValueMapBorrowed);
-}
 extern "C" {
     pub fn Servo_MediaList_AddRef(ptr: RawServoMediaListBorrowed);
 }
@@ -631,7 +627,7 @@ extern "C" {
                                   aCascadeLevel:
                                       EffectCompositor_CascadeLevel,
                                   aAnimationValues:
-                                      RawServoAnimationValueMapBorrowed)
+                                      RawServoAnimationValueMapBorrowedMut)
      -> bool;
 }
 extern "C" {
@@ -1790,7 +1786,7 @@ extern "C" {
 }
 extern "C" {
     pub fn Servo_AnimationValueMap_Push(arg1:
-                                            RawServoAnimationValueMapBorrowed,
+                                            RawServoAnimationValueMapBorrowedMut,
                                         property: nsCSSPropertyID,
                                         value:
                                             RawServoAnimationValueBorrowed);
@@ -1964,7 +1960,7 @@ extern "C" {
 }
 extern "C" {
     pub fn Servo_AnimationCompose(animation_values:
-                                      RawServoAnimationValueMapBorrowed,
+                                      RawServoAnimationValueMapBorrowedMut,
                                   base_values: *mut ::std::os::raw::c_void,
                                   property: nsCSSPropertyID,
                                   animation_segment:

components/style/properties/helpers/animated_properties.mako.rs

@@ -9,7 +9,9 @@
 use app_units::Au;
 use cssparser::{Color as CSSParserColor, Parser, RGBA, serialize_identifier};
 use euclid::{Point2D, Size2D};
+#[cfg(feature = "gecko")] use gecko_bindings::bindings::RawServoAnimationValueMap;
 #[cfg(feature = "gecko")] use gecko_bindings::structs::nsCSSPropertyID;
+#[cfg(feature = "gecko")] use gecko_bindings::sugar::ownership::{HasFFI, HasSimpleFFI};
 #[cfg(feature = "gecko")] use gecko_string_cache::Atom;
 use properties::{CSSWideKeyword, PropertyDeclaration};
 use properties::longhands;
@@ -388,6 +390,12 @@ impl AnimatedProperty {
 /// composed for each TransitionProperty.
 #[cfg(feature = "gecko")]
 pub type AnimationValueMap = HashMap<TransitionProperty, AnimationValue>;
+#[cfg(feature = "gecko")]
+unsafe impl HasFFI for AnimationValueMap {
+    type FFIType = RawServoAnimationValueMap;
+}
+#[cfg(feature = "gecko")]
+unsafe impl HasSimpleFFI for AnimationValueMap {}
 
 /// An enum to represent a single computed value belonging to an animated
 /// property in order to be interpolated with another one. When interpolating,

components/style/stylist.rs

@@ -752,7 +752,7 @@ impl Stylist {
             if let Some(anim) = animation_rules.0 {
                 Push::push(
                     applicable_declarations,
-                    ApplicableDeclarationBlock::from_declarations(anim.clone(),
+                    ApplicableDeclarationBlock::from_declarations(anim,
                                                                   CascadeLevel::Animations));
             }
             debug!("animation: {:?}", relations);
@@ -808,7 +808,7 @@ impl Stylist {
         if let Some(anim) = animation_rules.1 {
             Push::push(
                 applicable_declarations,
-                ApplicableDeclarationBlock::from_declarations(anim.clone(), CascadeLevel::Transitions));
+                ApplicableDeclarationBlock::from_declarations(anim, CascadeLevel::Transitions));
         }
         debug!("transition: {:?}", relations);
 

ports/geckolib/glue.rs

@@ -6,7 +6,6 @@ use atomic_refcell::AtomicRefMut;
 use cssparser::Parser;
 use cssparser::ToCss as ParserToCss;
 use env_logger::LogBuilder;
-use parking_lot::RwLock;
 use selectors::Element;
 use std::borrow::Cow;
 use std::env;
@@ -47,7 +46,7 @@ use style::gecko_bindings::bindings::RawGeckoElementBorrowed;
 use style::gecko_bindings::bindings::RawGeckoFontFaceRuleListBorrowedMut;
 use style::gecko_bindings::bindings::RawGeckoServoStyleRuleListBorrowedMut;
 use style::gecko_bindings::bindings::RawServoAnimationValueBorrowed;
-use style::gecko_bindings::bindings::RawServoAnimationValueMapBorrowed;
+use style::gecko_bindings::bindings::RawServoAnimationValueMapBorrowedMut;
 use style::gecko_bindings::bindings::RawServoAnimationValueStrong;
 use style::gecko_bindings::bindings::RawServoImportRuleBorrowed;
 use style::gecko_bindings::bindings::RawServoStyleRuleBorrowed;
@@ -279,19 +278,19 @@ pub extern "C" fn Servo_AnimationValues_ComputeDistance(from: RawServoAnimationV
 }
 
 #[no_mangle]
-pub extern "C" fn Servo_AnimationValueMap_Push(value_map: RawServoAnimationValueMapBorrowed,
+pub extern "C" fn Servo_AnimationValueMap_Push(value_map: RawServoAnimationValueMapBorrowedMut,
                                                property: nsCSSPropertyID,
                                                value: RawServoAnimationValueBorrowed)
 {
     use style::properties::animated_properties::AnimationValueMap;
 
-    let value_map = RwLock::<AnimationValueMap>::as_arc(&value_map);
+    let value_map = AnimationValueMap::from_ffi_mut(value_map);
     let value = AnimationValue::as_arc(&value).as_ref();
-    value_map.write().insert(property.into(), value.clone());
+    value_map.insert(property.into(), value.clone());
 }
 
 #[no_mangle]
-pub extern "C" fn Servo_AnimationCompose(raw_value_map: RawServoAnimationValueMapBorrowed,
+pub extern "C" fn Servo_AnimationCompose(raw_value_map: RawServoAnimationValueMapBorrowedMut,
                                          base_values: *mut ::std::os::raw::c_void,
                                          css_property: nsCSSPropertyID,
                                          segment: RawGeckoAnimationPropertySegmentBorrowed,
@@ -303,7 +302,7 @@ pub extern "C" fn Servo_AnimationCompose(raw_value_map: RawServoAnimationValueMa
     use style::properties::animated_properties::AnimationValueMap;
 
     let property: TransitionProperty = css_property.into();
-    let value_map = RwLock::<AnimationValueMap>::as_arc(&raw_value_map);
+    let value_map = AnimationValueMap::from_ffi_mut(raw_value_map);
 
     // If either of the segment endpoints are null, get the underlying value to
     // use from the current value in the values map (set by a lower-priority
@@ -311,7 +310,7 @@ pub extern "C" fn Servo_AnimationCompose(raw_value_map: RawServoAnimationValueMa
     // for this property.
     let underlying_value = if segment.mFromValue.mServo.mRawPtr.is_null() ||
                               segment.mToValue.mServo.mRawPtr.is_null() {
-        let previous_composed_value = value_map.read().get(&property).cloned();
+        let previous_composed_value = value_map.get(&property).cloned();
         previous_composed_value.or_else(|| {
             let raw_base_style = unsafe { Gecko_AnimationGetBaseStyle(base_values, css_property) };
             AnimationValue::arc_from_borrowed(&raw_base_style).map(|v| v.as_ref()).cloned()
@@ -347,9 +346,9 @@ pub extern "C" fn Servo_AnimationCompose(raw_value_map: RawServoAnimationValueMa
     let progress = unsafe { Gecko_GetProgressFromComputedTiming(computed_timing) };
     if segment.mToKey == segment.mFromKey {
         if progress < 0. {
-            value_map.write().insert(property, from_value.clone());
+            value_map.insert(property, from_value.clone());
         } else {
-            value_map.write().insert(property, to_value.clone());
+            value_map.insert(property, to_value.clone());
         }
         return;
     }
@@ -358,11 +357,11 @@ pub extern "C" fn Servo_AnimationCompose(raw_value_map: RawServoAnimationValueMa
         Gecko_GetPositionInSegment(segment, progress, computed_timing.mBeforeFlag)
     };
     if let Ok(value) = from_value.interpolate(to_value, position) {
-        value_map.write().insert(property, value);
+        value_map.insert(property, value);
     } else if progress < 0.5 {
-        value_map.write().insert(property, from_value.clone());
+        value_map.insert(property, from_value.clone());
     } else {
-        value_map.write().insert(property, to_value.clone());
+        value_map.insert(property, to_value.clone());
     }
 }
 

ports/geckolib/lib.rs

@@ -9,7 +9,6 @@ extern crate cssparser;
 extern crate env_logger;
 extern crate libc;
 #[macro_use] extern crate log;
-extern crate parking_lot;
 extern crate selectors;
 #[macro_use] extern crate style;
 extern crate style_traits;