From 498a163cdfbdcb260fb0149cb5103fe1cd6f3e34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 15 Apr 2019 20:11:45 +0000 Subject: [PATCH] style: The counters code should use atoms rather than strings. Servo already atomizes the counter names, it makes no sense to copy the string rather than bumping the refcount. Differential Revision: https://phabricator.services.mozilla.com/D27061 --- components/style/properties/gecko.mako.rs | 36 ++++++++++++-------- components/style/values/generics/counters.rs | 14 +++++--- 2 files changed, 31 insertions(+), 19 deletions(-) diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 81e1ee4307d..95afdd56a79 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -4455,7 +4455,7 @@ clip-path fn set_counter_function( data: &mut nsStyleContentData, content_type: StyleContentType, - name: &CustomIdent, + name: CustomIdent, sep: &str, style: CounterStyleOrNone, ) { @@ -4464,7 +4464,9 @@ clip-path let counter_func = unsafe { bindings::Gecko_SetCounterFunction(data, content_type).as_mut().unwrap() }; - counter_func.mIdent.assign(name.0.as_slice()); + counter_func.mIdent.set_move(unsafe { + RefPtr::from_addrefed(name.0.into_addrefed()) + }); if content_type == StyleContentType::Counters { counter_func.mSeparator.assign_str(sep); } @@ -4493,14 +4495,14 @@ clip-path Gecko_ClearAndResizeStyleContents(&mut self.gecko, items.len() as u32); } - for (i, item) in items.into_iter().enumerate() { + for (i, item) in items.into_vec().into_iter().enumerate() { // NB: Gecko compares the mString value if type is not image // or URI independently of whatever gets there. In the quote // cases, they set it to null, so do the same here. unsafe { *self.gecko.mContents[i].mContent.mString.as_mut() = ptr::null_mut(); } - match *item { + match item { ContentItem::String(ref value) => { self.gecko.mContents[i].mType = StyleContentType::String; unsafe { @@ -4536,22 +4538,22 @@ clip-path => self.gecko.mContents[i].mType = StyleContentType::NoOpenQuote, ContentItem::NoCloseQuote => self.gecko.mContents[i].mType = StyleContentType::NoCloseQuote, - ContentItem::Counter(ref name, ref style) => { + ContentItem::Counter(name, style) => { set_counter_function( &mut self.gecko.mContents[i], StyleContentType::Counter, - &name, + name, "", - style.clone(), + style, ); } - ContentItem::Counters(ref name, ref sep, ref style) => { + ContentItem::Counters(name, sep, style) => { set_counter_function( &mut self.gecko.mContents[i], StyleContentType::Counters, - &name, + name, &sep, - style.clone(), + style, ); } ContentItem::Url(ref url) => { @@ -4627,7 +4629,9 @@ clip-path StyleContentType::Counter | StyleContentType::Counters => { let gecko_function = unsafe { &**gecko_content.mContent.mCounters.as_ref() }; - let ident = CustomIdent(Atom::from(&*gecko_function.mIdent)); + let ident = CustomIdent(unsafe { + Atom::from_raw(gecko_function.mIdent.mRawPtr) + }); let style = CounterStyleOrNone::from_gecko_value(&gecko_function.mCounterStyle); let style = match style { @@ -4664,8 +4668,10 @@ clip-path ) { unsafe { bindings::Gecko_ClearAndResizeCounter${counter_property}s(&mut self.gecko, v.len() as u32); - for (i, ref pair) in v.iter().enumerate() { - self.gecko.m${counter_property}s[i].mCounter.assign(pair.name.0.as_slice()); + for (i, pair) in v.0.into_vec().into_iter().enumerate() { + self.gecko.m${counter_property}s[i].mCounter.set_move( + RefPtr::from_addrefed(pair.name.0.into_addrefed()) + ); self.gecko.m${counter_property}s[i].mValue = pair.value; } } @@ -4690,7 +4696,9 @@ clip-path longhands::counter_${counter_property.lower()}::computed_value::T::new( self.gecko.m${counter_property}s.iter().map(|ref gecko_counter| { CounterPair { - name: CustomIdent(Atom::from(gecko_counter.mCounter.to_string())), + name: CustomIdent(unsafe { + Atom::from_raw(gecko_counter.mCounter.mRawPtr) + }), value: gecko_counter.mValue, } }).collect() diff --git a/components/style/values/generics/counters.rs b/components/style/values/generics/counters.rs index 17ac687a670..fbb6927b9f1 100644 --- a/components/style/values/generics/counters.rs +++ b/components/style/values/generics/counters.rs @@ -45,7 +45,7 @@ pub struct CounterPair { ToResolvedValue, ToShmem, )] -pub struct CounterIncrement(Counters); +pub struct CounterIncrement(pub Counters); impl CounterIncrement { /// Returns a new value for `counter-increment`. @@ -77,7 +77,7 @@ impl Deref for CounterIncrement { ToResolvedValue, ToShmem, )] -pub struct CounterSetOrReset(Counters); +pub struct CounterSetOrReset(pub Counters); impl CounterSetOrReset { /// Returns a new value for `counter-set` / `counter-reset`. @@ -102,6 +102,7 @@ impl Deref for CounterSetOrReset { #[derive( Clone, Debug, + Default, MallocSizeOf, PartialEq, SpecifiedValueInfo, @@ -112,10 +113,13 @@ impl Deref for CounterSetOrReset { )] pub struct Counters(#[css(iterable, if_empty = "none")] Box<[CounterPair]>); -impl Default for Counters { +impl Counters { + /// Move out the Box into a vector. This could just return the Box<>, but + /// Vec<> is a bit more convenient because Box<[T]> doesn't implement + /// IntoIter: https://github.com/rust-lang/rust/issues/59878 #[inline] - fn default() -> Self { - Counters(vec![].into_boxed_slice()) + pub fn into_vec(self) -> Vec> { + self.0.into_vec() } }