From 9370172552d2ad95b3c257e9a0e03612de43c453 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 11 May 2023 08:25:36 +0000 Subject: [PATCH] style: Remove HasArcFFI for AnimationValue See previous patches for context. Differential Revision: https://phabricator.services.mozilla.com/D177622 --- components/style/gecko/arc_types.rs | 20 ++-- components/style/gecko/wrapper.rs | 10 +- .../style/gecko_bindings/sugar/refptr.rs | 97 ++++++++----------- 3 files changed, 56 insertions(+), 71 deletions(-) diff --git a/components/style/gecko/arc_types.rs b/components/style/gecko/arc_types.rs index c9f5717bc19..8a213df9324 100644 --- a/components/style/gecko/arc_types.rs +++ b/components/style/gecko/arc_types.rs @@ -10,12 +10,12 @@ use crate::gecko::url::CssUrlData; use crate::gecko_bindings::structs::{ - RawServoAnimationValue, RawServoContainerRule, RawServoCounterStyleRule, - RawServoDeclarationBlock, RawServoFontFaceRule, RawServoFontFeatureValuesRule, - RawServoFontPaletteValuesRule, RawServoImportRule, RawServoKeyframe, RawServoKeyframesRule, - RawServoLayerBlockRule, RawServoLayerStatementRule, RawServoMediaList, RawServoMediaRule, - RawServoMozDocumentRule, RawServoNamespaceRule, RawServoPageRule, RawServoStyleRule, - RawServoSupportsRule, ServoCssRules, + RawServoContainerRule, RawServoCounterStyleRule, RawServoDeclarationBlock, + RawServoFontFaceRule, RawServoFontFeatureValuesRule, RawServoFontPaletteValuesRule, + RawServoImportRule, RawServoKeyframe, RawServoKeyframesRule, RawServoLayerBlockRule, + RawServoLayerStatementRule, RawServoMediaList, RawServoMediaRule, RawServoMozDocumentRule, + RawServoNamespaceRule, RawServoPageRule, RawServoStyleRule, RawServoSupportsRule, + ServoCssRules, }; use crate::gecko_bindings::sugar::ownership::HasArcFFI; use crate::media_queries::MediaList; @@ -60,9 +60,6 @@ impl_arc_ffi!(Locked => RawServoStyleRule impl_arc_ffi!(Locked => RawServoImportRule [Servo_ImportRule_AddRef, Servo_ImportRule_Release]); -impl_arc_ffi!(AnimationValue => RawServoAnimationValue - [Servo_AnimationValue_AddRef, Servo_AnimationValue_Release]); - impl_arc_ffi!(Locked => RawServoKeyframe [Servo_Keyframe_AddRef, Servo_Keyframe_Release]); @@ -137,3 +134,8 @@ impl_simple_arc_ffi!( Servo_ComputedStyle_AddRef, Servo_ComputedStyle_Release ); +impl_simple_arc_ffi!( + AnimationValue, + Servo_AnimationValue_AddRef, + Servo_AnimationValue_Release +); diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index f6ae9f69391..e085910bec0 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -825,14 +825,12 @@ impl<'le> GeckoElement<'le> { let mut map = FxHashMap::with_capacity_and_hasher(collection_length, Default::default()); for i in 0..collection_length { - let raw_end_value = unsafe { Gecko_ElementTransitions_EndValueAt(self.0, i).as_ref() }; - - let end_value = AnimationValue::arc_from_borrowed(&raw_end_value) - .expect("AnimationValue not found in ElementTransitions"); - + let end_value = unsafe { + Arc::from_raw_addrefed(Gecko_ElementTransitions_EndValueAt(self.0, i)) + }; let property = end_value.id(); debug_assert!(!property.is_logical()); - map.insert(property, end_value.clone_arc()); + map.insert(property, end_value); } map } diff --git a/components/style/gecko_bindings/sugar/refptr.rs b/components/style/gecko_bindings/sugar/refptr.rs index e2536018622..51e2b095e66 100644 --- a/components/style/gecko_bindings/sugar/refptr.rs +++ b/components/style/gecko_bindings/sugar/refptr.rs @@ -9,7 +9,7 @@ use crate::gecko_bindings::{bindings, structs}; use crate::Atom; use servo_arc::Arc; use std::marker::PhantomData; -use std::ops::{Deref, DerefMut}; +use std::ops::Deref; use std::{fmt, mem, ptr}; use std::fmt::Write; @@ -40,18 +40,6 @@ impl fmt::Debug for RefPtr { } } -/// A RefPtr that we know is uniquely owned. -/// -/// This is basically Box, with the additional guarantee that the box can be -/// safely interpreted as a RefPtr (with refcount 1) -/// -/// This is useful when you wish to create a refptr and mutate it temporarily, -/// while it is still uniquely owned. -pub struct UniqueRefPtr(RefPtr); - -// There is no safe conversion from &T to RefPtr (like Gecko has) -// because this lets you break UniqueRefPtr's guarantee - impl RefPtr { /// Create a new RefPtr from an already addrefed pointer obtained from FFI. /// @@ -59,7 +47,7 @@ impl RefPtr { pub unsafe fn from_addrefed(ptr: *mut T) -> Self { debug_assert!(!ptr.is_null()); RefPtr { - ptr: ptr, + ptr, _marker: PhantomData, } } @@ -125,23 +113,6 @@ impl RefPtr { } } -impl UniqueRefPtr { - /// Create a unique refptr from an already addrefed pointer obtained from - /// FFI. - /// - /// The refcount must be one. - /// - /// The pointer must be valid and non null - pub unsafe fn from_addrefed(ptr: *mut T) -> Self { - UniqueRefPtr(RefPtr::from_addrefed(ptr)) - } - - /// Convert to a RefPtr so that it can be used. - pub fn get(self) -> RefPtr { - self.0 - } -} - impl Deref for RefPtr { type Target = T; fn deref(&self) -> &T { @@ -150,19 +121,6 @@ impl Deref for RefPtr { } } -impl Deref for UniqueRefPtr { - type Target = T; - fn deref(&self) -> &T { - unsafe { &*self.0.ptr } - } -} - -impl DerefMut for UniqueRefPtr { - fn deref_mut(&mut self) -> &mut T { - unsafe { &mut *self.0.ptr } - } -} - impl structs::RefPtr { /// Produces a Rust-side RefPtr from an FFI RefPtr, bumping the refcount. /// @@ -226,24 +184,51 @@ impl structs::RefPtr { } impl structs::RefPtr { - /// Sets the contents to an `Arc`, releasing the old value in `self` if - /// necessary. - pub fn set_arc(&mut self, other: Arc) + /// Returns a new, null refptr. + pub fn null() -> Self { + Self { + mRawPtr: ptr::null_mut(), + _phantom_0: PhantomData, + } + } + + /// Create a new RefPtr from an arc. + pub fn from_arc(s: Arc) -> Self { + Self { + mRawPtr: Arc::into_raw(s) as *mut _, + _phantom_0: PhantomData, + } + } + + /// Create a new RefPtr from an arc. + pub fn from_arc_ffi(s: Arc) -> Self + where + U: HasArcFFI, + { + Self { + mRawPtr: unsafe { mem::transmute(Arc::into_raw_offset(s)) }, + _phantom_0: PhantomData, + } + } + + /// Sets the contents to an Arc. + pub fn set_arc(&mut self, other: Arc) { + unsafe { + if !self.mRawPtr.is_null() { + let _ = Arc::from_raw(self.mRawPtr); + } + self.mRawPtr = Arc::into_raw(other) as *mut _; + } + } + + /// Sets the contents to an Arc. + pub fn set_arc_ffi(&mut self, other: Arc) where U: HasArcFFI, { unsafe { U::release_opt(self.mRawPtr.as_ref()); } - self.set_arc_leaky(other); - } - - /// Sets the contents to an Arc - /// will leak existing contents - pub fn set_arc_leaky(&mut self, other: Arc) - where - U: HasArcFFI, - { *self = unsafe { mem::transmute(Arc::into_raw_offset(other)) }; } }