Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

style: Remove unsound Atom From implementations. #20339

Merged
merged 1 commit into from Mar 19, 2018
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

style: Remove unsound Atom From implementations.

Fixes #20158
  • Loading branch information
emilio committed Mar 19, 2018
commit 55e2cd5dc315c4e5b06a86eb1d7dbf634e82f275
@@ -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")
}
@@ -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() };
@@ -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 {
@@ -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);
@@ -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") %>
@@ -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());
}

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.