Bug 1483964: Manually inline class and ID getters. r=xidorn
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 15 Aug 2018 01:29:40 +0200
changeset 487244 62d4efbe32d764134fcb8d63debc03c922e7d8f9
parent 487243 a7a2da45f8f7663d7fa3b3b2066d1eb05dad1d14
child 487245 4bb194e17f26293ae8fed85e65fd56081e944da2
push id9719
push userffxbld-merge
push dateFri, 24 Aug 2018 17:49:46 +0000
treeherdermozilla-beta@719ec98fba77 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersxidorn
bugs1483964
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1483964: Manually inline class and ID getters. r=xidorn Somewhat ugly but hopefully not too much. Somehow it ends up removing more lines than adding. Differential Revision: https://phabricator.services.mozilla.com/D3536
dom/base/Element.h
dom/base/nsAttrValue.h
layout/style/ServoBindings.cpp
layout/style/ServoBindings.h
layout/style/ServoBindings.toml
layout/style/ServoElementSnapshot.h
servo/components/style/gecko/snapshot.rs
servo/components/style/gecko/snapshot_helpers.rs
servo/components/style/gecko/wrapper.rs
servo/components/style/gecko_string_cache/mod.rs
--- a/dom/base/Element.h
+++ b/dom/base/Element.h
@@ -944,33 +944,22 @@ public:
 
   virtual bool IsNodeOfType(uint32_t aFlags) const override;
 
   /**
    * Get the class list of this element (this corresponds to the value of the
    * class attribute).  This may be null if there are no classes, but that's not
    * guaranteed (e.g. we could have class="").
    */
-  const nsAttrValue* GetClasses() const {
-    if (MayHaveClass()) {
-      return DoGetClasses();
+  const nsAttrValue* GetClasses() const
+  {
+    if (!MayHaveClass()) {
+      return nullptr;
     }
-    return nullptr;
-  }
 
-  /**
-   * Hook for implementing GetClasses. This should only be called if the
-   * ElementMayHaveClass flag is set.
-   *
-   * Public only because Servo needs to call it too, and it ensures the
-   * precondition before calling this.
-   */
-  const nsAttrValue* DoGetClasses() const
-  {
-    MOZ_ASSERT(MayHaveClass(), "Unexpected call");
     if (IsSVGElement()) {
       if (const nsAttrValue* value = GetSVGAnimatedClass()) {
         return value;
       }
     }
 
     return GetParsedAttr(nsGkAtoms::_class);
   }
@@ -1962,17 +1951,17 @@ protected:
    */
   virtual void GetLinkTarget(nsAString& aTarget);
 
   nsDOMTokenList* GetTokenList(nsAtom* aAtom,
                                const DOMTokenListSupportedTokenArray aSupportedTokens = nullptr);
 
 private:
   /**
-   * Slow path for DoGetClasses, this should only be called for SVG elements.
+   * Slow path for GetClasses, this should only be called for SVG elements.
    */
   const nsAttrValue* GetSVGAnimatedClass() const;
 
   /**
    * Get this element's client area rect in app units.
    * @return the frame's client area
    */
   MOZ_CAN_RUN_SCRIPT nsRect GetClientAreaRect();
--- a/dom/base/nsAttrValue.h
+++ b/dom/base/nsAttrValue.h
@@ -36,17 +36,17 @@ class nsStyledElement;
 struct MiscContainer;
 
 namespace mozilla {
 class DeclarationBlock;
 } // namespace mozilla
 
 #define NS_ATTRVALUE_MAX_STRINGLENGTH_ATOM 12
 
-#define NS_ATTRVALUE_BASETYPE_MASK (uintptr_t(3))
+const uintptr_t NS_ATTRVALUE_BASETYPE_MASK = 3;
 #define NS_ATTRVALUE_POINTERVALUE_MASK (~NS_ATTRVALUE_BASETYPE_MASK)
 
 #define NS_ATTRVALUE_INTEGERTYPE_BITS 4
 #define NS_ATTRVALUE_INTEGERTYPE_MASK (uintptr_t((1 << NS_ATTRVALUE_INTEGERTYPE_BITS) - 1))
 #define NS_ATTRVALUE_INTEGERTYPE_MULTIPLIER (1 << NS_ATTRVALUE_INTEGERTYPE_BITS)
 #define NS_ATTRVALUE_INTEGERTYPE_MAXVALUE ((1 << (31 - NS_ATTRVALUE_INTEGERTYPE_BITS)) - 1)
 #define NS_ATTRVALUE_INTEGERTYPE_MINVALUE (-NS_ATTRVALUE_INTEGERTYPE_MAXVALUE - 1)
 
--- a/layout/style/ServoBindings.cpp
+++ b/layout/style/ServoBindings.cpp
@@ -913,24 +913,16 @@ Gecko_IsBrowserFrame(RawGeckoElementBorr
 {
   nsIMozBrowserFrame* browserFrame =
     const_cast<Element*>(aElement)->GetAsMozBrowserFrame();
   return browserFrame && browserFrame->GetReallyIsBrowser();
 }
 
 template <typename Implementor>
 static nsAtom*
-AtomAttrValue(Implementor* aElement, nsAtom* aName)
-{
-  const nsAttrValue* attr = aElement->GetParsedAttr(aName);
-  return attr ? attr->GetAtomValue() : nullptr;
-}
-
-template <typename Implementor>
-static nsAtom*
 LangValue(Implementor* aElement)
 {
   // TODO(emilio): Deduplicate a bit with nsIContent::GetLang().
   const nsAttrValue* attr =
     aElement->GetParsedAttr(nsGkAtoms::lang, kNameSpaceID_XML);
   if (!attr && aElement->SupportsLangAttr()) {
     attr = aElement->GetParsedAttr(nsGkAtoms::lang);
   }
@@ -1069,104 +1061,17 @@ AttrHasSuffix(Implementor* aElement, nsA
     nsAutoString str;
     aValue->ToString(str);
     WITH_COMPARATOR(aIgnoreCase, c,
                     StringEndsWith(str, nsDependentAtomString(aStr), c))
   };
   return DoMatch(aElement, aNS, aName, match);
 }
 
-/**
- * Returns whether an element contains a class in its class list or not.
- */
-template <typename Implementor>
-static bool
-HasClass(Implementor* aElement, nsAtom* aClass, bool aIgnoreCase)
-{
-  const nsAttrValue* attr = aElement->DoGetClasses();
-  if (!attr) {
-    return false;
-  }
-
-  return attr->Contains(aClass, aIgnoreCase ? eIgnoreCase : eCaseMatters);
-}
-
-/**
- * Gets the class or class list (if any) of the implementor. The calling
- * convention here is rather hairy, and is optimized for getting Servo the
- * information it needs for hot calls.
- *
- * The return value indicates the number of classes. If zero, neither outparam
- * is valid. If one, the class_ outparam is filled with the atom of the class.
- * If two or more, the classList outparam is set to point to an array of atoms
- * representing the class list.
- *
- * The array is borrowed and the atoms are not addrefed. These values can be
- * invalidated by any DOM mutation. Use them in a tight scope.
- */
-template <typename Implementor>
-static uint32_t
-ClassOrClassList(Implementor* aElement, nsAtom** aClass, nsAtom*** aClassList)
-{
-  const nsAttrValue* attr = aElement->DoGetClasses();
-  if (!attr) {
-    return 0;
-  }
-
-  // For class values with only whitespace, Gecko just stores a string. For the
-  // purposes of the style system, there is no class in this case.
-  nsAttrValue::ValueType type = attr->Type();
-  if (MOZ_UNLIKELY(type == nsAttrValue::eString)) {
-    MOZ_ASSERT(nsContentUtils::TrimWhitespace<nsContentUtils::IsHTMLWhitespace>(
-                 attr->GetStringValue()).IsEmpty());
-    return 0;
-  }
-
-  // Single tokens are generally stored as an atom. Check that case.
-  if (type == nsAttrValue::eAtom) {
-    *aClass = attr->GetAtomValue();
-    return 1;
-  }
-
-  // At this point we should have an atom array. It is likely, but not
-  // guaranteed, that we have two or more elements in the array.
-  MOZ_ASSERT(type == nsAttrValue::eAtomArray);
-  nsTArray<RefPtr<nsAtom>>* atomArray = attr->GetAtomArrayValue();
-  uint32_t length = atomArray->Length();
-
-  // Special case: zero elements.
-  if (length == 0) {
-    return 0;
-  }
-
-  // Special case: one element.
-  if (length == 1) {
-    *aClass = atomArray->ElementAt(0);
-    return 1;
-  }
-
-  // General case: Two or more elements.
-  //
-  // Note: We could also expose this array as an array of nsCOMPtrs, since
-  // bindgen knows what those look like, and eliminate the reinterpret_cast.
-  // But it's not obvious that that would be preferable.
-  static_assert(sizeof(RefPtr<nsAtom>) == sizeof(nsAtom*), "Bad simplification");
-  static_assert(alignof(RefPtr<nsAtom>) == alignof(nsAtom*), "Bad simplification");
-
-  RefPtr<nsAtom>* elements = atomArray->Elements();
-  nsAtom** rawElements = reinterpret_cast<nsAtom**>(elements);
-  *aClassList = rawElements;
-  return atomArray->Length();
-}
-
 #define SERVO_IMPL_ELEMENT_ATTR_MATCHING_FUNCTIONS(prefix_, implementor_)        \
-  nsAtom* prefix_##AtomAttrValue(implementor_ aElement, nsAtom* aName)           \
-  {                                                                              \
-    return AtomAttrValue(aElement, aName);                                       \
-  }                                                                              \
   nsAtom* prefix_##LangValue(implementor_ aElement)                              \
   {                                                                              \
     return LangValue(aElement);                                                  \
   }                                                                              \
   bool prefix_##HasAttr(implementor_ aElement, nsAtom* aNS, nsAtom* aName)       \
   {                                                                              \
     return HasAttr(aElement, aNS, aName);                                        \
   }                                                                              \
@@ -1194,26 +1099,17 @@ ClassOrClassList(Implementor* aElement, 
                               nsAtom* aName, nsAtom* aStr, bool aIgnoreCase)     \
   {                                                                              \
     return AttrHasPrefix(aElement, aNS, aName, aStr, aIgnoreCase);               \
   }                                                                              \
   bool prefix_##AttrHasSuffix(implementor_ aElement, nsAtom* aNS,                \
                               nsAtom* aName, nsAtom* aStr, bool aIgnoreCase)     \
   {                                                                              \
     return AttrHasSuffix(aElement, aNS, aName, aStr, aIgnoreCase);               \
-  }                                                                              \
-  uint32_t prefix_##ClassOrClassList(implementor_ aElement, nsAtom** aClass,     \
-                                     nsAtom*** aClassList)                       \
-  {                                                                              \
-    return ClassOrClassList(aElement, aClass, aClassList);                       \
-  }                                                                              \
-  bool prefix_##HasClass(implementor_ aElement, nsAtom* aClass, bool aIgnoreCase)\
-  {                                                                              \
-    return HasClass(aElement, aClass, aIgnoreCase);                              \
-  }                                                                              \
+  }
 
 
 SERVO_IMPL_ELEMENT_ATTR_MATCHING_FUNCTIONS(Gecko_, RawGeckoElementBorrowed)
 SERVO_IMPL_ELEMENT_ATTR_MATCHING_FUNCTIONS(Gecko_Snapshot, const ServoElementSnapshot*)
 
 #undef SERVO_IMPL_ELEMENT_ATTR_MATCHING_FUNCTIONS
 
 nsAtom*
@@ -2964,8 +2860,27 @@ Gecko_IsInServoTraversal()
   return ServoStyleSet::IsInServoTraversal();
 }
 
 bool
 Gecko_IsMainThread()
 {
   return NS_IsMainThread();
 }
+
+const nsAttrValue*
+Gecko_GetSVGAnimatedClass(RawGeckoElementBorrowed aElement)
+{
+  MOZ_ASSERT(aElement->IsSVGElement());
+  return static_cast<const nsSVGElement*>(aElement)->GetAnimatedClassName();
+}
+
+bool
+Gecko_AssertClassAttrValueIsSane(const nsAttrValue* aValue)
+{
+  MOZ_ASSERT(aValue->Type() == nsAttrValue::eAtom ||
+             aValue->Type() == nsAttrValue::eString ||
+             aValue->Type() == nsAttrValue::eAtomArray);
+  MOZ_ASSERT_IF(aValue->Type() == nsAttrValue::eString,
+                nsContentUtils::TrimWhitespace<nsContentUtils::IsHTMLWhitespace>(
+                  aValue->GetStringValue()).IsEmpty());
+  return true;
+}
--- a/layout/style/ServoBindings.h
+++ b/layout/style/ServoBindings.h
@@ -195,36 +195,35 @@ bool Gecko_MatchLang(RawGeckoElementBorr
                      const char16_t* value);
 nsAtom* Gecko_GetXMLLangValue(RawGeckoElementBorrowed element);
 nsIDocument::DocumentTheme Gecko_GetDocumentLWTheme(const nsIDocument* aDocument);
 bool Gecko_IsTableBorderNonzero(RawGeckoElementBorrowed element);
 bool Gecko_IsBrowserFrame(RawGeckoElementBorrowed element);
 
 // Attributes.
 #define SERVO_DECLARE_ELEMENT_ATTR_MATCHING_FUNCTIONS(prefix_, implementor_)  \
-  nsAtom* prefix_##AtomAttrValue(implementor_ element, nsAtom* attribute);    \
   nsAtom* prefix_##LangValue(implementor_ element);                           \
   bool prefix_##HasAttr(implementor_ element, nsAtom* ns, nsAtom* name);      \
   bool prefix_##AttrEquals(implementor_ element, nsAtom* ns, nsAtom* name,    \
                            nsAtom* str, bool ignoreCase);                     \
   bool prefix_##AttrDashEquals(implementor_ element, nsAtom* ns,              \
                                nsAtom* name, nsAtom* str, bool ignore_case);  \
   bool prefix_##AttrIncludes(implementor_ element, nsAtom* ns,                \
                              nsAtom* name, nsAtom* str, bool ignore_case);    \
   bool prefix_##AttrHasSubstring(implementor_ element, nsAtom* ns,            \
                                  nsAtom* name, nsAtom* str,                   \
                                  bool ignore_case);                           \
   bool prefix_##AttrHasPrefix(implementor_ element, nsAtom* ns,               \
                               nsAtom* name, nsAtom* str, bool ignore_case);   \
   bool prefix_##AttrHasSuffix(implementor_ element, nsAtom* ns,               \
-                              nsAtom* name, nsAtom* str, bool ignore_case);   \
-  uint32_t prefix_##ClassOrClassList(implementor_ element, nsAtom** class_,   \
-                                     nsAtom*** classList);                    \
-  bool prefix_##HasClass(implementor_ element, nsAtom* class_,                \
-                         bool ignore_case);
+                              nsAtom* name, nsAtom* str, bool ignore_case);
+
+bool Gecko_AssertClassAttrValueIsSane(const nsAttrValue*);
+const nsAttrValue* Gecko_GetSVGAnimatedClass(RawGeckoElementBorrowed);
+
 
 SERVO_DECLARE_ELEMENT_ATTR_MATCHING_FUNCTIONS(Gecko_, RawGeckoElementBorrowed)
 SERVO_DECLARE_ELEMENT_ATTR_MATCHING_FUNCTIONS(Gecko_Snapshot,
                                               const ServoElementSnapshot*)
 
 #undef SERVO_DECLARE_ELEMENT_ATTR_MATCHING_FUNCTIONS
 
 // Style attributes.
@@ -648,17 +647,16 @@ mozilla::StyleSheet* Gecko_StyleSheet_Cl
     const mozilla::StyleSheet* aNewParentSheet);
 void Gecko_StyleSheet_AddRef(const mozilla::StyleSheet* aSheet);
 void Gecko_StyleSheet_Release(const mozilla::StyleSheet* aSheet);
 
 nsCSSKeyword Gecko_LookupCSSKeyword(const uint8_t* string, uint32_t len);
 const char* Gecko_CSSKeywordString(nsCSSKeyword keyword, uint32_t* len);
 
 bool Gecko_IsDocumentBody(RawGeckoElementBorrowed element);
-
 // We use an int32_t here instead of a LookAndFeel::ColorID
 // because forward-declaring a nested enum/struct is impossible
 nscolor Gecko_GetLookAndFeelSystemColor(int32_t color_id,
                                         RawGeckoPresContextBorrowed pres_context);
 
 void Gecko_AddPropertyToSet(nsCSSPropertyIDSetBorrowedMut, nsCSSPropertyID);
 
 // Style-struct management.
--- a/layout/style/ServoBindings.toml
+++ b/layout/style/ServoBindings.toml
@@ -192,16 +192,17 @@ rusty-enums = [
     "mozilla::LookAndFeel_FontID",
     "nsStyleTransformMatrix::MatrixTransformOperator",
     "mozilla::StyleGeometryBox",
     "mozilla::SystemColor",
 ]
 whitelist-vars = [
     "NS_AUTHOR_SPECIFIED_.*",
     "NS_THEME_.*",
+    "NS_ATTRVALUE_.*",
     "NODE_.*",
     "ELEMENT_.*",
     "NS_FONT_.*",
     "NS_STYLE_.*",
     "NS_MATHML_.*",
     "NS_RADIUS_.*",
     "BORDER_COLOR_.*",
     "BORDER_STYLE_.*",
@@ -471,16 +472,17 @@ structs-types = [
     "mozilla::FontSlantStyle",
     "mozilla::FontWeight",
     "mozilla::MallocSizeOf",
     "mozilla::OriginFlags",
     "mozilla::UniquePtr",
     "mozilla::StyleDisplayMode",
     "ServoRawOffsetArc",
     "DeclarationBlockMutationClosure",
+    "nsAttrValue",
     "nsIContent",
     "nsINode",
     "nsIDocument",
     "nsIDocument_DocumentTheme",
     "nsSimpleContentList",
     "MediumFeaturesChangedResult",
     "RawGeckoAnimationPropertySegment",
     "RawGeckoComputedTiming",
--- a/layout/style/ServoElementSnapshot.h
+++ b/layout/style/ServoElementSnapshot.h
@@ -129,22 +129,16 @@ public:
       if (mAttrs[i].mName.Equals(aLocalName, aNamespaceID)) {
         return &mAttrs[i].mValue;
       }
     }
 
     return nullptr;
   }
 
-  const nsAttrValue* DoGetClasses() const
-  {
-    MOZ_ASSERT(HasAttrs());
-    return &mClass;
-  }
-
   bool IsInChromeDocument() const { return mIsInChromeDocument; }
   bool SupportsLangAttr() const { return mSupportsLangAttr; }
 
   bool HasAny(Flags aFlags) const { return bool(mContains & aFlags); }
 
   bool IsTableBorderNonzero() const
   {
     MOZ_ASSERT(HasOtherPseudoClassState());
--- a/servo/components/style/gecko/snapshot.rs
+++ b/servo/components/style/gecko/snapshot.rs
@@ -46,20 +46,16 @@ impl SnapshotMap {
 }
 
 impl GeckoElementSnapshot {
     #[inline]
     fn has_any(&self, flags: Flags) -> bool {
         (self.mContains as u8 & flags as u8) != 0
     }
 
-    fn as_ptr(&self) -> *const Self {
-        self
-    }
-
     /// Returns true if the snapshot has stored state for pseudo-classes
     /// that depend on things other than `ElementState`.
     #[inline]
     pub fn has_other_pseudo_class_state(&self) -> bool {
         self.has_any(Flags::OtherPseudoClassState)
     }
 
     /// Returns true if the snapshot recorded an id change.
@@ -179,54 +175,38 @@ impl ElementSnapshot for GeckoElementSna
     }
 
     #[inline]
     fn id_attr(&self) -> Option<&WeakAtom> {
         if !self.has_any(Flags::Id) {
             return None;
         }
 
-        let ptr = unsafe { bindings::Gecko_SnapshotAtomAttrValue(self, atom!("id").as_ptr()) };
-
-        // FIXME(emilio): This should assert, since this flag is exact.
-        if ptr.is_null() {
-            None
-        } else {
-            Some(unsafe { WeakAtom::new(ptr) })
-        }
+        snapshot_helpers::get_id(&*self.mAttrs)
     }
 
     #[inline]
     fn has_class(&self, name: &Atom, case_sensitivity: CaseSensitivity) -> bool {
         if !self.has_any(Flags::MaybeClass) {
             return false;
         }
 
-        snapshot_helpers::has_class(
-            self.as_ptr(),
-            name,
-            case_sensitivity,
-            bindings::Gecko_SnapshotHasClass,
-        )
+        snapshot_helpers::has_class(name, case_sensitivity, &self.mClass)
     }
 
     #[inline]
     fn each_class<F>(&self, callback: F)
     where
         F: FnMut(&Atom),
     {
         if !self.has_any(Flags::MaybeClass) {
             return;
         }
 
-        snapshot_helpers::each_class(
-            self.as_ptr(),
-            callback,
-            bindings::Gecko_SnapshotClassOrClassList,
-        )
+        snapshot_helpers::each_class(&self.mClass, callback)
     }
 
     #[inline]
     fn lang_attr(&self) -> Option<Atom> {
         let ptr = unsafe { bindings::Gecko_SnapshotLangValue(self) };
         if ptr.is_null() {
             None
         } else {
--- a/servo/components/style/gecko/snapshot_helpers.rs
+++ b/servo/components/style/gecko/snapshot_helpers.rs
@@ -1,61 +1,117 @@
 /* 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/. */
 
 //! Element an snapshot common logic.
 
-use gecko_bindings::structs::nsAtom;
+use CaseSensitivityExt;
+use gecko_bindings::bindings;
+use gecko_bindings::structs::{self, nsAtom};
 use selectors::attr::CaseSensitivity;
-use std::{ptr, slice};
-use string_cache::Atom;
+use string_cache::{Atom, WeakAtom};
 
 /// A function that, given an element of type `T`, allows you to get a single
 /// class or a class list.
-pub type ClassOrClassList<T> =
-    unsafe extern "C" fn(T, *mut *mut nsAtom, *mut *mut *mut nsAtom) -> u32;
+enum Class<'a> {
+    None,
+    One(*const nsAtom),
+    More(&'a [structs::RefPtr<nsAtom>]),
+}
+
+#[inline(always)]
+fn base_type(attr: &structs::nsAttrValue) -> structs::nsAttrValue_ValueBaseType {
+    (attr.mBits & structs::NS_ATTRVALUE_BASETYPE_MASK) as structs::nsAttrValue_ValueBaseType
+}
+
+#[inline(always)]
+unsafe fn ptr<T>(attr: &structs::nsAttrValue) -> *const T {
+    (attr.mBits & !structs::NS_ATTRVALUE_BASETYPE_MASK) as *const T
+}
+
+#[inline(always)]
+unsafe fn get_class_from_attr(attr: &structs::nsAttrValue) -> Class {
+    debug_assert!(bindings::Gecko_AssertClassAttrValueIsSane(attr));
+    let base_type = base_type(attr);
+    if base_type == structs::nsAttrValue_ValueBaseType_eStringBase {
+        return Class::None;
+    }
+    if base_type == structs::nsAttrValue_ValueBaseType_eAtomBase {
+        return Class::One(ptr::<nsAtom>(attr));
+    }
+    debug_assert_eq!(base_type, structs::nsAttrValue_ValueBaseType_eOtherBase);
 
-/// A function to return whether an element of type `T` has a given class.
-///
-/// The `bool` argument represents whether it should compare case-insensitively
-/// or not.
-pub type HasClass<T> = unsafe extern "C" fn(T, *mut nsAtom, bool) -> bool;
+    let container = ptr::<structs::MiscContainer>(attr);
+    debug_assert_eq!((*container).mType, structs::nsAttrValue_ValueType_eAtomArray);
+    let array =
+        (*container).__bindgen_anon_1.mValue.as_ref().__bindgen_anon_1.mAtomArray.as_ref();
+    Class::More(&***array)
+}
+
+#[inline(always)]
+unsafe fn get_id_from_attr(attr: &structs::nsAttrValue) -> &WeakAtom {
+    debug_assert_eq!(base_type(attr), structs::nsAttrValue_ValueBaseType_eAtomBase);
+    WeakAtom::new(ptr::<nsAtom>(attr))
+}
 
-/// Given an item `T`, a class name, and a getter function, return whether that
-/// element has the class that `name` represents.
+/// Find an attribute value with a given name and no namespace.
 #[inline(always)]
-pub fn has_class<T>(
-    item: T,
+pub fn find_attr<'a>(
+    attrs: &'a [structs::AttrArray_InternalAttr],
+    name: &Atom,
+) -> Option<&'a structs::nsAttrValue> {
+    attrs.iter()
+        .find(|attr| attr.mName.mBits == name.as_ptr() as usize)
+        .map(|attr| &attr.mValue)
+}
+
+/// Finds the id attribute from a list of attributes.
+#[inline(always)]
+pub fn get_id(attrs: &[structs::AttrArray_InternalAttr]) -> Option<&WeakAtom> {
+    Some(unsafe { get_id_from_attr(find_attr(attrs, &atom!("id"))?) })
+}
+
+/// Given a class name, a case sensitivity, and an array of attributes, returns
+/// whether the class has the attribute that name represents.
+#[inline(always)]
+pub fn has_class(
     name: &Atom,
     case_sensitivity: CaseSensitivity,
-    getter: HasClass<T>,
+    attr: &structs::nsAttrValue,
 ) -> bool {
-    let ignore_case = match case_sensitivity {
-        CaseSensitivity::CaseSensitive => false,
-        CaseSensitivity::AsciiCaseInsensitive => true,
-    };
-
-    unsafe { getter(item, name.as_ptr(), ignore_case) }
+    match unsafe { get_class_from_attr(attr) } {
+        Class::None => false,
+        Class::One(atom) => unsafe {
+            case_sensitivity.eq_atom(name, WeakAtom::new(atom))
+        },
+        Class::More(atoms) => {
+            match case_sensitivity {
+                CaseSensitivity::CaseSensitive => {
+                    atoms.iter().any(|atom| atom.mRawPtr == name.as_ptr())
+                }
+                CaseSensitivity::AsciiCaseInsensitive => unsafe {
+                    atoms.iter().any(|atom| WeakAtom::new(atom.mRawPtr).eq_ignore_ascii_case(name))
+                }
+            }
+        }
+    }
 }
 
 /// Given an item, a callback, and a getter, execute `callback` for each class
 /// this `item` has.
-pub fn each_class<F, T>(item: T, mut callback: F, getter: ClassOrClassList<T>)
+#[inline(always)]
+pub fn each_class<F>(attr: &structs::nsAttrValue, mut callback: F)
 where
     F: FnMut(&Atom),
 {
     unsafe {
-        let mut class: *mut nsAtom = ptr::null_mut();
-        let mut list: *mut *mut nsAtom = ptr::null_mut();
-        let length = getter(item, &mut class, &mut list);
-        match length {
-            0 => {},
-            1 => Atom::with(class, callback),
-            n => {
-                let classes = slice::from_raw_parts(list, n as usize);
-                for c in classes {
-                    Atom::with(*c, &mut callback)
+        match get_class_from_attr(attr) {
+            Class::None => {},
+            Class::One(atom) => Atom::with(atom, callback),
+            Class::More(atoms) => {
+                for atom in atoms {
+                    Atom::with(atom.mRawPtr, &mut callback)
                 }
-            },
+            }
         }
     }
 }
--- a/servo/components/style/gecko/wrapper.rs
+++ b/servo/components/style/gecko/wrapper.rs
@@ -27,17 +27,16 @@ use font_metrics::{FontMetrics, FontMetr
 use gecko::data::GeckoStyleSheet;
 use gecko::global_style_data::GLOBAL_STYLE_DATA;
 use gecko::selector_parser::{NonTSPseudoClass, PseudoElement, SelectorImpl};
 use gecko::snapshot_helpers;
 use gecko_bindings::bindings;
 use gecko_bindings::bindings::{Gecko_ElementState, Gecko_GetDocumentLWTheme};
 use gecko_bindings::bindings::{Gecko_GetLastChild, Gecko_GetPreviousSibling, Gecko_GetNextStyleChild};
 use gecko_bindings::bindings::{Gecko_SetNodeFlags, Gecko_UnsetNodeFlags};
-use gecko_bindings::bindings::Gecko_ClassOrClassList;
 use gecko_bindings::bindings::Gecko_ElementHasAnimations;
 use gecko_bindings::bindings::Gecko_ElementHasCSSAnimations;
 use gecko_bindings::bindings::Gecko_ElementHasCSSTransitions;
 use gecko_bindings::bindings::Gecko_GetActiveLinkAttrDeclarationBlock;
 use gecko_bindings::bindings::Gecko_GetAnimationEffectCount;
 use gecko_bindings::bindings::Gecko_GetAnimationRule;
 use gecko_bindings::bindings::Gecko_GetExtraContentStyleDeclarations;
 use gecko_bindings::bindings::Gecko_GetHTMLPresentationAttrDeclarationBlock;
@@ -576,16 +575,44 @@ impl<'le> fmt::Debug for GeckoElement<'l
             bindings::Gecko_Element_DebugListAttributes(self.0, &mut attrs);
         }
         write!(f, "{}", attrs)?;
         write!(f, "> ({:#x})", self.as_node().opaque().0)
     }
 }
 
 impl<'le> GeckoElement<'le> {
+    #[inline(always)]
+    fn attrs(&self) -> &[structs::AttrArray_InternalAttr] {
+        unsafe {
+            let attrs = match self.0._base.mAttrs.mImpl.mPtr.as_ref() {
+                Some(attrs) => attrs,
+                None => return &[],
+            };
+
+            attrs.mBuffer.as_slice(attrs.mAttrCount as usize)
+        }
+    }
+
+    #[inline(always)]
+    fn get_class_attr(&self) -> Option<&structs::nsAttrValue> {
+        if !self.may_have_class() {
+            return None;
+        }
+
+        if self.is_svg_element() {
+            let svg_class = unsafe { bindings::Gecko_GetSVGAnimatedClass(self.0).as_ref() };
+            if let Some(c) = svg_class {
+                return Some(c)
+            }
+        }
+
+        snapshot_helpers::find_attr(self.attrs(), &atom!("class"))
+    }
+
     #[inline]
     fn closest_anon_subtree_root_parent(&self) -> Option<Self> {
         debug_assert!(self.is_in_native_anonymous_subtree());
         let mut current = *self;
 
         loop {
             if current.is_root_of_native_anonymous_subtree() {
                 return current.traversal_parent();
@@ -1276,36 +1303,29 @@ impl<'le> TElement for GeckoElement<'le>
 
     // FIXME(emilio): we should probably just return a reference to the Atom.
     #[inline]
     fn id(&self) -> Option<&WeakAtom> {
         if !self.has_id() {
             return None;
         }
 
-        let ptr = unsafe { bindings::Gecko_AtomAttrValue(self.0, atom!("id").as_ptr()) };
-
-        // FIXME(emilio): Pretty sure the has_id flag is exact and we could
-        // assert here.
-        if ptr.is_null() {
-            None
-        } else {
-            Some(unsafe { WeakAtom::new(ptr) })
-        }
+        snapshot_helpers::get_id(self.attrs())
     }
 
     fn each_class<F>(&self, callback: F)
     where
         F: FnMut(&Atom),
     {
-        if !self.may_have_class() {
-            return;
-        }
+        let attr = match self.get_class_attr() {
+            Some(c) => c,
+            None => return,
+        };
 
-        snapshot_helpers::each_class(self.0, callback, Gecko_ClassOrClassList)
+        snapshot_helpers::each_class(attr, callback)
     }
 
     #[inline]
     fn has_snapshot(&self) -> bool {
         self.flags() & (ELEMENT_HAS_SNAPSHOT as u32) != 0
     }
 
     #[inline]
@@ -2260,34 +2280,32 @@ impl<'le> ::selectors::Element for Gecko
     }
 
     #[inline]
     fn has_id(&self, id: &Atom, case_sensitivity: CaseSensitivity) -> bool {
         if !self.has_id() {
             return false;
         }
 
-        unsafe {
-            let ptr = bindings::Gecko_AtomAttrValue(self.0, atom!("id").as_ptr());
+        let element_id = match snapshot_helpers::get_id(self.attrs()) {
+            Some(id) => id,
+            None => return false,
+        };
 
-            if ptr.is_null() {
-                false
-            } else {
-                case_sensitivity.eq_atom(WeakAtom::new(ptr), id)
-            }
-        }
+        case_sensitivity.eq_atom(element_id, id)
     }
 
     #[inline(always)]
     fn has_class(&self, name: &Atom, case_sensitivity: CaseSensitivity) -> bool {
-        if !self.may_have_class() {
-            return false;
-        }
+        let attr = match self.get_class_attr() {
+            Some(c) => c,
+            None => return false,
+        };
 
-        snapshot_helpers::has_class(self.0, name, case_sensitivity, bindings::Gecko_HasClass)
+        snapshot_helpers::has_class(name, case_sensitivity, attr)
     }
 
     #[inline]
     fn is_html_element_in_html_document(&self) -> bool {
         self.is_html_element() && self.as_node().owner_doc().is_html_document()
     }
 
     #[inline]
--- a/servo/components/style/gecko_string_cache/mod.rs
+++ b/servo/components/style/gecko_string_cache/mod.rs
@@ -258,17 +258,17 @@ impl fmt::Display for WeakAtom {
             w.write_char(c.unwrap_or(char::REPLACEMENT_CHARACTER))?
         }
         Ok(())
     }
 }
 
 impl Atom {
     /// Execute a callback with the atom represented by `ptr`.
-    pub unsafe fn with<F, R>(ptr: *mut nsAtom, callback: F) -> R
+    pub unsafe fn with<F, R>(ptr: *const nsAtom, callback: F) -> R
     where
         F: FnOnce(&Atom) -> R,
     {
         let atom = Atom(WeakAtom::new(ptr));
         let ret = callback(&atom);
         mem::forget(atom);
         ret
     }