From 83a4935b6083255cea087b83bb096c9e20ff9b0a Mon Sep 17 00:00:00 2001 From: Hiroyuki Ikezoe Date: Mon, 1 May 2017 18:11:33 +0900 Subject: [PATCH] Pass AnimationValueMap raw pointer instead of Arc to Gecko_GetAnimationRule() --- components/style/build_gecko.rs | 3 +-- components/style/gecko/arc_types.rs | 8 ++---- components/style/gecko/wrapper.rs | 11 +++----- .../helpers/animated_properties.mako.rs | 8 ++++++ ports/geckolib/glue.rs | 25 +++++++++---------- ports/geckolib/lib.rs | 1 - 6 files changed, 27 insertions(+), 29 deletions(-) diff --git a/components/style/build_gecko.rs b/components/style/build_gecko.rs index 05fcc0982ab..7b223e4eac4 100644 --- a/components/style/build_gecko.rs +++ b/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", diff --git a/components/style/gecko/arc_types.rs b/components/style/gecko/arc_types.rs index 134f3d3062f..f9818ba0eb5 100644 --- a/components/style/gecko/arc_types.rs +++ b/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 => RawServoImportRule impl_arc_ffi!(AnimationValue => RawServoAnimationValue [Servo_AnimationValue_AddRef, Servo_AnimationValue_Release]); -impl_arc_ffi!(RwLock => RawServoAnimationValueMap - [Servo_AnimationValueMap_AddRef, Servo_AnimationValueMap_Release]); - impl_arc_ffi!(Locked => RawServoMediaList [Servo_MediaList_AddRef, Servo_MediaList_Release]); diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 177aa5987b6..100830c0e0d 100644 --- a/components/style/gecko/wrapper.rs +++ b/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>> { - // FIXME(emilio): This is quite dumb, why an RwLock, it's local to this - // function? - // + use gecko_bindings::sugar::ownership::HasSimpleFFI; // 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 } diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index f2da326e048..1b2e8f456c3 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/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; +#[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, diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 63ffda73810..9011ec75815 100644 --- a/ports/geckolib/glue.rs +++ b/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::::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::::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()); } } diff --git a/ports/geckolib/lib.rs b/ports/geckolib/lib.rs index d5f291e6a64..9a28de32528 100644 --- a/ports/geckolib/lib.rs +++ b/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;