Auto merge of #20339 - emilio:atom-from-dead-beef, r=nox

style: Remove unsound Atom From implementations.

Fixes #20158

<!-- 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/20339)
<!-- Reviewable:end -->

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")
         }

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() };

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 {

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<String> 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);

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<CustomIdent> = 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<CustomIdent> = 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") %>

components/style/stylist.rs

@@ -1636,7 +1636,7 @@ impl ExtraStyleData {
         guard: &SharedRwLockReadGuard,
         rule: &Arc<Locked<CounterStyleRule>>,
     ) {
-        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());
     }