Bug 1393189 part 5 - Rewrite CounterStyleOrNone::from_gecko_value to use fewer binding functions. r?heycam draft
authorXidorn Quan <me@upsuper.org>
Tue, 29 Aug 2017 17:11:13 +1000
changeset 656374 5a5281dd3a9157f787faf93daa46b8c4d57fd548
parent 656373 246e4827b3d7f0f7249267d3928eac21989da65c
child 656375 89854592373b96392da0e7151e2ff161b6f5f927
push id77188
push userxquan@mozilla.com
push dateThu, 31 Aug 2017 04:04:58 +0000
reviewersheycam
bugs1393189
milestone57.0a1
Bug 1393189 part 5 - Rewrite CounterStyleOrNone::from_gecko_value to use fewer binding functions. r?heycam MozReview-Commit-ID: 3EsExs0DzQr
layout/style/CounterStyleManager.h
layout/style/ServoBindings.cpp
layout/style/ServoBindings.h
layout/style/ServoBindings.toml
servo/components/style/gecko/values.rs
servo/components/style/properties/gecko.mako.rs
--- a/layout/style/CounterStyleManager.h
+++ b/layout/style/CounterStyleManager.h
@@ -233,16 +233,28 @@ public:
   bool operator==(const CounterStylePtr& aOther) const
     { return mRaw == aOther.mRaw; }
   bool operator!=(const CounterStylePtr& aOther) const
     { return mRaw != aOther.mRaw; }
 
   bool IsResolved() const { return !IsUnresolved(); }
   inline void Resolve(CounterStyleManager* aManager);
 
+  nsIAtom* AsAtom() const
+  {
+    MOZ_ASSERT(IsUnresolved());
+    return reinterpret_cast<nsIAtom*>(mRaw & ~eMask);
+  }
+  AnonymousCounterStyle* AsAnonymous() const
+  {
+    MOZ_ASSERT(IsAnonymous());
+    return static_cast<AnonymousCounterStyle*>(
+      reinterpret_cast<CounterStyle*>(mRaw & ~eMask));
+  }
+
 private:
   CounterStyle* Get() const
   {
     MOZ_ASSERT(IsResolved());
     return reinterpret_cast<CounterStyle*>(mRaw & ~eMask);
   }
   template<typename T>
   void AssertPointerAligned(T* aPointer)
@@ -261,27 +273,16 @@ private:
     eAnonymousCounterStyle = 1,
     eUnresolvedAtom = 2,
     eMask = 3,
   };
 
   Type GetType() const { return static_cast<Type>(mRaw & eMask); }
   bool IsUnresolved() const { return GetType() == eUnresolvedAtom; }
   bool IsAnonymous() const { return GetType() == eAnonymousCounterStyle; }
-  nsIAtom* AsAtom()
-  {
-    MOZ_ASSERT(IsUnresolved());
-    return reinterpret_cast<nsIAtom*>(mRaw & ~eMask);
-  }
-  AnonymousCounterStyle* AsAnonymous()
-  {
-    MOZ_ASSERT(IsAnonymous());
-    return static_cast<AnonymousCounterStyle*>(
-      reinterpret_cast<CounterStyle*>(mRaw & ~eMask));
-  }
 
   void Reset()
   {
     switch (GetType()) {
       case eCounterStyle:
         break;
       case eAnonymousCounterStyle:
         AsAnonymous()->Release();
--- a/layout/style/ServoBindings.cpp
+++ b/layout/style/ServoBindings.cpp
@@ -1448,63 +1448,29 @@ Gecko_SetCounterStyleToString(CounterSty
 }
 
 void
 Gecko_CopyCounterStyle(CounterStylePtr* aDst, const CounterStylePtr* aSrc)
 {
   *aDst = *aSrc;
 }
 
-bool
-Gecko_CounterStyle_IsNone(const CounterStylePtr* aPtr) {
-  MOZ_ASSERT(aPtr);
-  return (*aPtr)->IsNone();
-}
-
-bool
-Gecko_CounterStyle_IsName(const CounterStylePtr* aPtr) {
-  return !Gecko_CounterStyle_IsNone(aPtr) && !(*aPtr)->AsAnonymous();
-}
-
-void
-Gecko_CounterStyle_GetName(const CounterStylePtr* aPtr,
-                           nsAString* aResult) {
-  MOZ_ASSERT(Gecko_CounterStyle_IsName(aPtr));
-  nsIAtom* name = (*aPtr)->GetStyleName();
-  *aResult = nsDependentAtomString(name);
+nsIAtom*
+Gecko_CounterStyle_GetName(const CounterStylePtr* aPtr)
+{
+  if (!aPtr->IsResolved()) {
+    return aPtr->AsAtom();
+  }
+  return (*aPtr)->GetStyleName();
 }
 
-const nsTArray<nsString>&
-Gecko_CounterStyle_GetSymbols(const CounterStylePtr* aPtr) {
-  MOZ_ASSERT((*aPtr)->AsAnonymous());
-  AnonymousCounterStyle* anonymous = (*aPtr)->AsAnonymous();
-  return anonymous->GetSymbols();
-}
-
-uint8_t
-Gecko_CounterStyle_GetSystem(const CounterStylePtr* aPtr) {
-  MOZ_ASSERT((*aPtr)->AsAnonymous());
-  AnonymousCounterStyle* anonymous = (*aPtr)->AsAnonymous();
-  return anonymous->GetSystem();
-}
-
-bool
-Gecko_CounterStyle_IsSingleString(const CounterStylePtr* aPtr) {
-  MOZ_ASSERT(aPtr);
-  AnonymousCounterStyle* anonymous = (*aPtr)->AsAnonymous();
-  return anonymous ? anonymous->IsSingleString() : false;
-}
-
-void
-Gecko_CounterStyle_GetSingleString(const CounterStylePtr* aPtr,
-                                   nsAString* aResult) {
-  MOZ_ASSERT(Gecko_CounterStyle_IsSingleString(aPtr));
-  const nsTArray<nsString>& symbols = Gecko_CounterStyle_GetSymbols(aPtr);
-  MOZ_ASSERT(symbols.Length() == 1);
-  aResult->Assign(symbols[0]);
+const AnonymousCounterStyle*
+Gecko_CounterStyle_GetAnonymous(const CounterStylePtr* aPtr)
+{
+  return aPtr->AsAnonymous();
 }
 
 already_AddRefed<css::URLValue>
 ServoBundledURI::IntoCssUrl()
 {
   if (!mURLString) {
     return nullptr;
   }
--- a/layout/style/ServoBindings.h
+++ b/layout/style/ServoBindings.h
@@ -327,25 +327,19 @@ void Gecko_SetCounterStyleToName(mozilla
 void Gecko_SetCounterStyleToSymbols(mozilla::CounterStylePtr* ptr,
                                     uint8_t symbols_type,
                                     nsACString const* const* symbols,
                                     uint32_t symbols_count);
 void Gecko_SetCounterStyleToString(mozilla::CounterStylePtr* ptr,
                                    const nsACString* symbol);
 void Gecko_CopyCounterStyle(mozilla::CounterStylePtr* dst,
                             const mozilla::CounterStylePtr* src);
-bool Gecko_CounterStyle_IsNone(const mozilla::CounterStylePtr* ptr);
-bool Gecko_CounterStyle_IsName(const mozilla::CounterStylePtr* ptr);
-void Gecko_CounterStyle_GetName(const mozilla::CounterStylePtr* ptr,
-                                nsAString* result);
-const nsTArray<nsString>& Gecko_CounterStyle_GetSymbols(const mozilla::CounterStylePtr* ptr);
-uint8_t Gecko_CounterStyle_GetSystem(const mozilla::CounterStylePtr* ptr);
-bool Gecko_CounterStyle_IsSingleString(const mozilla::CounterStylePtr* ptr);
-void Gecko_CounterStyle_GetSingleString(const mozilla::CounterStylePtr* ptr,
-                                        nsAString* result);
+nsIAtom* Gecko_CounterStyle_GetName(const mozilla::CounterStylePtr* ptr);
+const mozilla::AnonymousCounterStyle*
+Gecko_CounterStyle_GetAnonymous(const mozilla::CounterStylePtr* ptr);
 
 // background-image style.
 void Gecko_SetNullImageValue(nsStyleImage* image);
 void Gecko_SetGradientImageValue(nsStyleImage* image, nsStyleGradient* gradient);
 NS_DECL_THREADSAFE_FFI_REFCOUNTING(mozilla::css::ImageValue, ImageValue);
 mozilla::css::ImageValue* Gecko_ImageValue_Create(ServoBundledURI aURI);
 void Gecko_SetLayerImageImageValue(nsStyleImage* image,
                                    mozilla::css::ImageValue* aImageValue);
--- a/layout/style/ServoBindings.toml
+++ b/layout/style/ServoBindings.toml
@@ -118,16 +118,17 @@ whitelist-vars = [
     "SERVO_CSS_PSEUDO_ELEMENT_FLAGS_.*",
     "kNameSpaceID_.*",
     "kGenericFont_.*",
     "kPresContext_.*",
 ]
 whitelist-types = [
     "RawGecko.*",
     "mozilla::AnimationPropertySegment",
+    "mozilla::AnonymousCounterStyle",
     "mozilla::ComputedTiming",
     "mozilla::ComputedTimingFunction",
     "mozilla::ComputedTimingFunction::BeforeFlag",
     "mozilla::SeenPtrs",
     "mozilla::ServoElementSnapshot.*",
     "mozilla::ServoStyleContext",
     "mozilla::ServoStyleSheetInner",
     "mozilla::CSSPseudoClassType",
@@ -365,16 +366,17 @@ raw-lines = [
 ]
 whitelist-functions = ["Servo_.*", "Gecko_.*"]
 structs-types = [
     "mozilla::css::GridTemplateAreasValue",
     "mozilla::css::ErrorReporter",
     "mozilla::css::ImageValue",
     "mozilla::css::URLValue",
     "mozilla::css::URLValueData",
+    "mozilla::AnonymousCounterStyle",
     "mozilla::MallocSizeOf",
     "mozilla::OriginFlags",
     "mozilla::Side",
     "mozilla::UniquePtr",
     "nsIContent",
     "nsIDocument",
     "nsIDocument_DocumentTheme",
     "RawGeckoAnimationPropertySegment",
--- a/servo/components/style/gecko/values.rs
+++ b/servo/components/style/gecko/values.rs
@@ -1,18 +1,19 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #![allow(unsafe_code)]
 
 //! Different kind of helpers to interact with Gecko values.
 
+use Atom;
 use app_units::Au;
-use counter_style::Symbol;
+use counter_style::{Symbol, Symbols};
 use cssparser::RGBA;
 use gecko_bindings::structs::{CounterStylePtr, nsStyleCoord};
 use gecko_bindings::structs::{StyleGridTrackBreadth, StyleShapeRadius};
 use gecko_bindings::sugar::ns_style_coord::{CoordData, CoordDataMut, CoordDataValue};
 use media_queries::Device;
 use nsstring::{nsACString, nsCString};
 use std::cmp::max;
 use values::{Auto, Either, ExtremumLength, None_, Normal};
@@ -471,38 +472,40 @@ impl CounterStyleOrNone {
                     .map(|symbol| symbol as &nsACString as *const _)
                     .collect();
                 unsafe { set_symbols(gecko_value, symbols_type.to_gecko_keyword(),
                                      symbols.as_ptr(), symbols.len() as u32) };
             }
         }
     }
 
-    /// Convert Gecko CounterStylePtr to CounterStyleOrNone.
-    pub fn from_gecko_value(gecko_value: &CounterStylePtr) -> Self {
-        use counter_style::{Symbol, Symbols};
-        use gecko_bindings::bindings::Gecko_CounterStyle_GetName;
-        use gecko_bindings::bindings::Gecko_CounterStyle_GetSymbols;
-        use gecko_bindings::bindings::Gecko_CounterStyle_GetSystem;
-        use gecko_bindings::bindings::Gecko_CounterStyle_IsName;
-        use gecko_bindings::bindings::Gecko_CounterStyle_IsNone;
+    /// Convert Gecko CounterStylePtr to CounterStyleOrNone or String.
+    pub fn from_gecko_value(gecko_value: &CounterStylePtr) -> Either<Self, String> {
+        use gecko_bindings::bindings;
         use values::CustomIdent;
         use values::generics::SymbolsType;
 
-        if unsafe { Gecko_CounterStyle_IsNone(gecko_value) } {
-            CounterStyleOrNone::None
-        } else if unsafe { Gecko_CounterStyle_IsName(gecko_value) } {
-            ns_auto_string!(name);
-            unsafe { Gecko_CounterStyle_GetName(gecko_value, &mut *name) };
-            CounterStyleOrNone::Name(CustomIdent((&*name).into()))
+        let name = unsafe { bindings::Gecko_CounterStyle_GetName(gecko_value) };
+        if !name.is_null() {
+            let name = Atom::from(name);
+            if name == atom!("none") {
+                Either::First(CounterStyleOrNone::None)
+            } else {
+                Either::First(CounterStyleOrNone::Name(CustomIdent(name)))
+            }
         } else {
-            let system = unsafe { Gecko_CounterStyle_GetSystem(gecko_value) };
-            let symbol_type = SymbolsType::from_gecko_keyword(system as u32);
-            let symbols = unsafe {
-                let ref gecko_symbols = *Gecko_CounterStyle_GetSymbols(gecko_value);
-                gecko_symbols.iter().map(|gecko_symbol| {
+            let anonymous = unsafe {
+                bindings::Gecko_CounterStyle_GetAnonymous(gecko_value).as_ref()
+            }.unwrap();
+            let symbols = &anonymous.mSymbols;
+            if anonymous.mSingleString {
+                debug_assert_eq!(symbols.len(), 1);
+                Either::Second(symbols[0].to_string())
+            } else {
+                let symbol_type = SymbolsType::from_gecko_keyword(anonymous.mSystem as u32);
+                let symbols = symbols.iter().map(|gecko_symbol| {
                     Symbol::String(gecko_symbol.to_string())
-                }).collect()
-            };
-            CounterStyleOrNone::Symbols(symbol_type, Symbols(symbols))
+                }).collect();
+                Either::First(CounterStyleOrNone::Symbols(symbol_type, Symbols(symbols)))
+            }
         }
     }
 }
--- a/servo/components/style/properties/gecko.mako.rs
+++ b/servo/components/style/properties/gecko.mako.rs
@@ -4154,29 +4154,24 @@ fn static_assert() {
         }
     }
 
     pub fn reset_list_style_type(&mut self, other: &Self) {
         self.copy_list_style_type_from(other)
     }
 
     pub fn clone_list_style_type(&self) -> longhands::list_style_type::computed_value::T {
-        use gecko_bindings::bindings::Gecko_CounterStyle_IsSingleString;
-        use gecko_bindings::bindings::Gecko_CounterStyle_GetSingleString;
         use self::longhands::list_style_type::computed_value::T;
+        use values::Either;
         use values::generics::CounterStyleOrNone;
 
-        if unsafe { Gecko_CounterStyle_IsSingleString(&self.gecko.mCounterStyle) } {
-            ns_auto_string!(single_string);
-            unsafe {
-                Gecko_CounterStyle_GetSingleString(&self.gecko.mCounterStyle, &mut *single_string)
-            };
-            T::String(single_string.to_string())
-        } else {
-            T::CounterStyle(CounterStyleOrNone::from_gecko_value(&self.gecko.mCounterStyle))
+        let result = CounterStyleOrNone::from_gecko_value(&self.gecko.mCounterStyle);
+        match result {
+            Either::First(counter_style) => T::CounterStyle(counter_style),
+            Either::Second(string) => T::String(string),
         }
     }
 
     pub fn set_quotes(&mut self, other: longhands::quotes::computed_value::T) {
         use gecko_bindings::bindings::Gecko_NewStyleQuoteValues;
         use gecko_bindings::sugar::refptr::UniqueRefPtr;
 
         let mut refptr = unsafe {
@@ -5612,16 +5607,17 @@ clip-path
     pub fn reset_content(&mut self, other: &Self) {
         self.copy_content_from(other)
     }
 
     pub fn clone_content(&self) -> longhands::content::computed_value::T {
         use gecko::conversions::string_from_chars_pointer;
         use gecko_bindings::structs::nsStyleContentType::*;
         use properties::longhands::content::computed_value::{T, ContentItem};
+        use values::Either;
         use values::generics::CounterStyleOrNone;
         use values::specified::url::SpecifiedUrl;
         use values::specified::Attr;
 
         if self.gecko.mContents.is_empty() {
             return T::Normal;
         }
 
@@ -5659,16 +5655,21 @@ clip-path
                         ContentItem::Attr(Attr { namespace, attribute })
                     },
                     eStyleContentType_Counter | eStyleContentType_Counters => {
                         let gecko_function =
                             unsafe { &**gecko_content.mContent.mCounters.as_ref() };
                         let ident = gecko_function.mIdent.to_string();
                         let style =
                             CounterStyleOrNone::from_gecko_value(&gecko_function.mCounterStyle);
+                        let style = match style {
+                            Either::First(counter_style) => counter_style,
+                            Either::Second(_) =>
+                                unreachable!("counter function shouldn't have single string type"),
+                        };
                         if gecko_content.mType == eStyleContentType_Counter {
                             ContentItem::Counter(ident, style)
                         } else {
                             let separator = gecko_function.mSeparator.to_string();
                             ContentItem::Counters(ident, separator, style)
                         }
                     },
                     eStyleContentType_Image => {