diff --git a/components/style/gecko/conversions.rs b/components/style/gecko/conversions.rs index b4171abb2a8..43a4b8dc91a 100644 --- a/components/style/gecko/conversions.rs +++ b/components/style/gecko/conversions.rs @@ -413,7 +413,7 @@ impl nsStyleImage { nsStyleImageType::eStyleImageType_Element => { use gecko_string_cache::Atom; let atom = Gecko_GetImageElement(self); - Some(GenericImage::Element(Atom::from(atom))) + Some(GenericImage::Element(Atom::from_raw(atom))) }, _ => panic!("Unexpected image type") } diff --git a/components/style/gecko/media_queries.rs b/components/style/gecko/media_queries.rs index d96d4d68755..481ad3c29c7 100644 --- a/components/style/gecko/media_queries.rs +++ b/components/style/gecko/media_queries.rs @@ -176,7 +176,7 @@ impl Device { context.mMedium }; - MediaType(CustomIdent(Atom::from(medium_to_use))) + MediaType(CustomIdent(unsafe { Atom::from_raw(medium_to_use) })) } /// Returns the current viewport size in app units. @@ -262,7 +262,7 @@ impl ToCss for Expression { } // NB: CssStringWriter not needed, feature names are under control. - write!(dest, "{}", Atom::from(unsafe { *self.feature.mName }))?; + write!(dest, "{}", unsafe { Atom::from_static(*self.feature.mName) })?; if let Some(ref val) = self.value { dest.write_str(": ")?; @@ -394,9 +394,9 @@ impl MediaExpressionValue { } nsMediaFeature_ValueType::eIdent => { debug_assert_eq!(css_value.mUnit, nsCSSUnit::eCSSUnit_AtomIdent); - Some(MediaExpressionValue::Ident(Atom::from(unsafe { - *css_value.mValue.mAtom.as_ref() - }))) + Some(MediaExpressionValue::Ident(unsafe { + Atom::from_raw(*css_value.mValue.mAtom.as_ref()) + })) } nsMediaFeature_ValueType::eIntRatio => { let array = unsafe { css_value.array_unchecked() }; diff --git a/components/style/gecko/values.rs b/components/style/gecko/values.rs index 322fcdf2d1b..abc28c8f5ec 100644 --- a/components/style/gecko/values.rs +++ b/components/style/gecko/values.rs @@ -507,7 +507,7 @@ impl CounterStyleOrNone { let name = unsafe { bindings::Gecko_CounterStyle_GetName(gecko_value) }; if !name.is_null() { - let name = Atom::from(name); + let name = unsafe { Atom::from_raw(name) }; if name == atom!("none") { Either::First(CounterStyleOrNone::None) } else { diff --git a/components/style/gecko_string_cache/mod.rs b/components/style/gecko_string_cache/mod.rs index 3499b13e310..e3e9a9c691e 100644 --- a/components/style/gecko_string_cache/mod.rs +++ b/components/style/gecko_string_cache/mod.rs @@ -99,7 +99,9 @@ impl WeakAtom { /// Clone this atom, bumping the refcount if the atom is not static. #[inline] pub fn clone(&self) -> Atom { - Atom::from(self.as_ptr()) + unsafe { + Atom::from_raw(self.as_ptr()) + } } /// Get the atom hash. @@ -267,15 +269,25 @@ impl Atom { /// /// Right now it's only used by the atom macro, and ideally it should keep /// that way, now we have sugar for is_static, creating atoms using - /// Atom::from should involve almost no overhead. + /// Atom::from_raw should involve almost no overhead. #[inline] - unsafe fn from_static(ptr: *mut nsStaticAtom) -> Self { + pub unsafe fn from_static(ptr: *mut nsStaticAtom) -> Self { let atom = Atom(ptr as *mut WeakAtom); debug_assert!(atom.is_static(), "Called from_static for a non-static atom!"); atom } + /// Creates an atom from an atom pointer. + #[inline(always)] + pub unsafe fn from_raw(ptr: *mut nsAtom) -> Self { + let atom = Atom(ptr as *mut WeakAtom); + if !atom.is_static() { + Gecko_AddRefAtom(ptr); + } + atom + } + /// Creates an atom from a dynamic atom pointer that has already had AddRef /// called on it. #[inline] @@ -308,7 +320,9 @@ impl Hash for WeakAtom { impl Clone for Atom { #[inline(always)] fn clone(&self) -> Atom { - Atom::from(self.as_ptr()) + unsafe { + Atom::from_raw(self.as_ptr()) + } } } @@ -388,28 +402,4 @@ impl From for Atom { } } -impl From<*mut nsAtom> for Atom { - #[inline] - fn from(ptr: *mut nsAtom) -> Atom { - assert!(!ptr.is_null()); - unsafe { - let ret = Atom(WeakAtom::new(ptr)); - if !ret.is_static() { - Gecko_AddRefAtom(ptr); - } - ret - } - } -} - -impl From<*mut nsStaticAtom> for Atom { - #[inline] - fn from(ptr: *mut nsStaticAtom) -> Atom { - assert!(!ptr.is_null()); - unsafe { - Atom::from_static(ptr) - } - } -} - malloc_size_of_is_0!(Atom); diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 5ec5e6d414f..ac13b04690b 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -131,7 +131,7 @@ impl ComputedValues { return None; } - let atom = Atom::from(atom); + let atom = unsafe { Atom::from_raw(atom) }; PseudoElement::from_atom(&atom) } @@ -3248,12 +3248,15 @@ fn static_assert() { use gecko_bindings::structs::nsCSSPropertyID::eCSSPropertyExtra_no_properties; use gecko_bindings::structs::nsCSSPropertyID::eCSSPropertyExtra_variable; use gecko_bindings::structs::nsCSSPropertyID::eCSSProperty_UNKNOWN; + use Atom; let property = self.gecko.mTransitions[index].mProperty; if property == eCSSProperty_UNKNOWN || property == eCSSPropertyExtra_variable { let atom = self.gecko.mTransitions[index].mUnknownProperty.mRawPtr; debug_assert!(!atom.is_null()); - TransitionProperty::Unsupported(CustomIdent(atom.into())) + TransitionProperty::Unsupported(CustomIdent(unsafe{ + Atom::from_raw(atom) + })) } else if property == eCSSPropertyExtra_no_properties { // Actually, we don't expect TransitionProperty::Unsupported also represents "none", // but if the caller wants to convert it, it is fine. Please use it carefully. @@ -3343,13 +3346,13 @@ fn static_assert() { pub fn animation_name_at(&self, index: usize) -> longhands::animation_name::computed_value::SingleComputedValue { use properties::longhands::animation_name::single_value::SpecifiedValue as AnimationName; + use Atom; let atom = self.gecko.mAnimations[index].mName.mRawPtr; if atom == atom!("").as_ptr() { - AnimationName(None) - } else { - AnimationName(Some(KeyframesName::from_atom(atom.into()))) + return AnimationName(None) } + AnimationName(Some(KeyframesName::from_atom(unsafe { Atom::from_raw(atom) }))) } pub fn copy_animation_name_from(&mut self, other: &Self) { self.gecko.mAnimationNameCount = other.gecko.mAnimationNameCount; @@ -3549,16 +3552,19 @@ fn static_assert() { use properties::longhands::will_change::computed_value::T; use gecko_bindings::structs::nsAtom; use values::CustomIdent; + use Atom; if self.gecko.mWillChange.len() == 0 { - T::Auto - } else { - let custom_idents: Vec = self.gecko.mWillChange.iter().map(|gecko_atom| { - CustomIdent((gecko_atom.mRawPtr as *mut nsAtom).into()) - }).collect(); - - T::AnimateableFeatures(custom_idents.into_boxed_slice()) + return T::Auto } + + let custom_idents: Vec = self.gecko.mWillChange.iter().map(|gecko_atom| { + unsafe { + CustomIdent(Atom::from_raw(gecko_atom.mRawPtr as *mut nsAtom)) + } + }).collect(); + + T::AnimateableFeatures(custom_idents.into_boxed_slice()) } <% impl_shape_source("shape_outside", "mShapeOutside") %> diff --git a/components/style/stylist.rs b/components/style/stylist.rs index b5cb97b6719..4666869af04 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -1636,7 +1636,7 @@ impl ExtraStyleData { guard: &SharedRwLockReadGuard, rule: &Arc>, ) { - let name = rule.read_with(guard).mName.mRawPtr.into(); + let name = unsafe { Atom::from_raw(rule.read_with(guard).mName.mRawPtr) }; self.counter_styles.insert(name, rule.clone()); } diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 8e0ee71738b..4168c089fe2 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -2020,9 +2020,9 @@ pub extern "C" fn Servo_KeyframesRule_GetName(rule: RawServoKeyframesRuleBorrowe } #[no_mangle] -pub extern "C" fn Servo_KeyframesRule_SetName(rule: RawServoKeyframesRuleBorrowed, name: *mut nsAtom) { +pub unsafe extern "C" fn Servo_KeyframesRule_SetName(rule: RawServoKeyframesRuleBorrowed, name: *mut nsAtom) { write_locked_arc(rule, |rule: &mut KeyframesRule| { - rule.name = KeyframesName::Ident(CustomIdent(unsafe { Atom::from_addrefed(name) })); + rule.name = KeyframesName::Ident(CustomIdent(Atom::from_addrefed(name))); }) } @@ -2167,7 +2167,7 @@ pub extern "C" fn Servo_FontFeatureValuesRule_GetValueText( } #[no_mangle] -pub extern "C" fn Servo_ComputedValues_GetForAnonymousBox( +pub unsafe extern "C" fn Servo_ComputedValues_GetForAnonymousBox( parent_style_or_null: ServoStyleContextBorrowedOrNull, pseudo_tag: *mut nsAtom, raw_data: RawServoStyleSetBorrowed, @@ -2176,7 +2176,7 @@ pub extern "C" fn Servo_ComputedValues_GetForAnonymousBox( let guard = global_style_data.shared_lock.read(); let guards = StylesheetGuards::same(&guard); let data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut(); - let atom = Atom::from(pseudo_tag); + let atom = Atom::from_raw(pseudo_tag); let pseudo = PseudoElement::from_anon_box_atom(&atom) .expect("Not an anon box pseudo?"); @@ -2478,7 +2478,7 @@ fn get_pseudo_style( } #[no_mangle] -pub extern "C" fn Servo_ComputedValues_Inherit( +pub unsafe extern "C" fn Servo_ComputedValues_Inherit( raw_data: RawServoStyleSetBorrowed, pseudo_tag: *mut nsAtom, parent_style_context: ServoStyleContextBorrowedOrNull, @@ -2487,7 +2487,7 @@ pub extern "C" fn Servo_ComputedValues_Inherit( let data = PerDocumentStyleData::from_ffi(raw_data).borrow(); let for_text = target == structs::InheritTarget::Text; - let atom = Atom::from(pseudo_tag); + let atom = Atom::from_raw(pseudo_tag); let pseudo = PseudoElement::from_anon_box_atom(&atom) .expect("Not an anon-box? Gah!"); @@ -3210,7 +3210,7 @@ pub extern "C" fn Servo_DeclarationBlock_PropertyIsSet( } #[no_mangle] -pub extern "C" fn Servo_DeclarationBlock_SetIdentStringValue( +pub unsafe extern "C" fn Servo_DeclarationBlock_SetIdentStringValue( declarations: RawServoDeclarationBlockBorrowed, property: nsCSSPropertyID, value: *mut nsAtom, @@ -3220,7 +3220,7 @@ pub extern "C" fn Servo_DeclarationBlock_SetIdentStringValue( let long = get_longhand_from_id!(property); let prop = match_wrap_declared! { long, - XLang => Lang(Atom::from(value)), + XLang => Lang(Atom::from_raw(value)), }; write_locked_arc(declarations, |decls: &mut PropertyDeclarationBlock| { decls.push(prop, Importance::Normal, DeclarationSource::CssOm); @@ -4138,7 +4138,7 @@ fn fill_in_missing_keyframe_values( } #[no_mangle] -pub extern "C" fn Servo_StyleSet_GetKeyframesForName( +pub unsafe extern "C" fn Servo_StyleSet_GetKeyframesForName( raw_data: RawServoStyleSetBorrowed, name: *mut nsAtom, inherited_timing_function: nsTimingFunctionBorrowed, @@ -4148,7 +4148,7 @@ pub extern "C" fn Servo_StyleSet_GetKeyframesForName( "keyframes should be initially empty"); let data = PerDocumentStyleData::from_ffi(raw_data).borrow(); - let name = Atom::from(name); + let name = Atom::from_raw(name); let animation = match data.stylist.get_animation(&name) { Some(animation) => animation, @@ -4183,11 +4183,11 @@ pub extern "C" fn Servo_StyleSet_GetKeyframesForName( // Look for an existing keyframe with the same offset and timing // function or else add a new keyframe at the beginning of the keyframe // array. - let keyframe = unsafe { - Gecko_GetOrCreateKeyframeAtStart(keyframes, - step.start_percentage.0 as f32, - &timing_function) - }; + let keyframe = Gecko_GetOrCreateKeyframeAtStart( + keyframes, + step.start_percentage.0 as f32, + &timing_function, + ); match step.value { KeyframesStepValue::ComputedValues => { @@ -4197,12 +4197,10 @@ pub extern "C" fn Servo_StyleSet_GetKeyframesForName( // animation should be set to the underlying computed value for // that keyframe. for property in animation.properties_changed.iter() { - unsafe { - Gecko_AppendPropertyValuePair( - &mut (*keyframe).mPropertyValues, - property.to_nscsspropertyid(), - ); - } + Gecko_AppendPropertyValuePair( + &mut (*keyframe).mPropertyValues, + property.to_nscsspropertyid(), + ); } if current_offset == 0.0 { has_complete_initial_keyframe = true; @@ -4249,23 +4247,19 @@ pub extern "C" fn Servo_StyleSet_GetKeyframesForName( continue; } - let pair = unsafe { - Gecko_AppendPropertyValuePair( - &mut (*keyframe).mPropertyValues, - id.to_nscsspropertyid(), - ) - }; + let pair = Gecko_AppendPropertyValuePair( + &mut (*keyframe).mPropertyValues, + id.to_nscsspropertyid(), + ); - unsafe { - (*pair).mServoDeclarationBlock.set_arc_leaky( - Arc::new(global_style_data.shared_lock.wrap( - PropertyDeclarationBlock::with_one( - declaration.clone(), - Importance::Normal, - ) - )) - ); - } + (*pair).mServoDeclarationBlock.set_arc_leaky( + Arc::new(global_style_data.shared_lock.wrap( + PropertyDeclarationBlock::with_one( + declaration.clone(), + Importance::Normal, + ) + )) + ); if current_offset == 0.0 { properties_set_at_start.insert(id); @@ -4276,19 +4270,14 @@ pub extern "C" fn Servo_StyleSet_GetKeyframesForName( } if custom_properties.any_normal() { - let pair = unsafe { - Gecko_AppendPropertyValuePair( - &mut (*keyframe).mPropertyValues, - nsCSSPropertyID::eCSSPropertyExtra_variable, - ) - }; - - unsafe { - (*pair).mServoDeclarationBlock.set_arc_leaky(Arc::new( - global_style_data.shared_lock.wrap(custom_properties) - )); - } + let pair = Gecko_AppendPropertyValuePair( + &mut (*keyframe).mPropertyValues, + nsCSSPropertyID::eCSSPropertyExtra_variable, + ); + (*pair).mServoDeclarationBlock.set_arc_leaky(Arc::new( + global_style_data.shared_lock.wrap(custom_properties) + )); } }, } @@ -4498,7 +4487,7 @@ pub extern "C" fn Servo_StyleSet_HasDocumentStateDependency( } #[no_mangle] -pub extern "C" fn Servo_GetCustomPropertyValue( +pub unsafe extern "C" fn Servo_GetCustomPropertyValue( computed_values: ServoStyleContextBorrowed, name: *const nsAString, value: *mut nsAString, @@ -4508,13 +4497,13 @@ pub extern "C" fn Servo_GetCustomPropertyValue( None => return false, }; - let name = unsafe { Atom::from(&*name) }; + let name = Atom::from(&*name); let computed_value = match custom_properties.get(&name) { Some(v) => v, None => return false, }; - computed_value.to_css(&mut CssWriter::new(unsafe { value.as_mut().unwrap() })).unwrap(); + computed_value.to_css(&mut CssWriter::new(&mut *value)).unwrap(); true }