Bug 1422528: Inline and make stylo take the rare path for GetClasses directly. r?bz draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Sat, 02 Dec 2017 21:45:33 +0100
changeset 706611 70da93e9a1f21fe40cfefc89b6186e68202f9817
parent 706606 009ebc83fce1e531fb78a90f9e39d52dcf2a64ca
child 706613 724f42a649ce5e3d80d322bc14d707b2e3f054ec
push id91854
push userbmo:emilio@crisal.io
push dateSat, 02 Dec 2017 20:47:20 +0000
reviewersbz
bugs1422528
milestone59.0a1
Bug 1422528: Inline and make stylo take the rare path for GetClasses directly. r?bz Servo already checks MayHaveClass. This should improve Gecko performance, too. MozReview-Commit-ID: KpVOVsKh6pe
dom/base/Element.cpp
dom/base/Element.h
layout/style/ServoBindings.cpp
layout/style/ServoElementSnapshot.h
--- a/dom/base/Element.cpp
+++ b/dom/base/Element.cpp
@@ -200,28 +200,20 @@ nsIContent::DoGetID() const
 {
   MOZ_ASSERT(HasID(), "Unexpected call");
   MOZ_ASSERT(IsElement(), "Only elements can have IDs");
 
   return AsElement()->GetParsedAttr(nsGkAtoms::id)->GetAtomValue();
 }
 
 const nsAttrValue*
-Element::DoGetClasses() const
+Element::GetSVGAnimatedClass() const
 {
-  MOZ_ASSERT(MayHaveClass(), "Unexpected call");
-  if (IsSVGElement()) {
-    const nsAttrValue* animClass =
-      static_cast<const nsSVGElement*>(this)->GetAnimatedClassName();
-    if (animClass) {
-      return animClass;
-    }
-  }
-
-  return GetParsedAttr(nsGkAtoms::_class);
+  MOZ_ASSERT(MayHaveClass() && IsSVGElement(), "Unexpected call");
+  return static_cast<const nsSVGElement*>(this)->GetAnimatedClassName();
 }
 
 NS_IMETHODIMP
 Element::QueryInterface(REFNSIID aIID, void** aInstancePtr)
 {
   NS_ASSERTION(aInstancePtr,
                "QueryInterface requires a non-NULL destination!");
   nsresult rv = FragmentOrElement::QueryInterface(aIID, aInstancePtr);
--- a/dom/base/Element.h
+++ b/dom/base/Element.h
@@ -794,16 +794,35 @@ public:
    */
   const nsAttrValue* GetClasses() const {
     if (MayHaveClass()) {
       return DoGetClasses();
     }
     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);
+  }
+
 #ifdef DEBUG
   virtual void List(FILE* out = stdout, int32_t aIndent = 0) const override
   {
     List(out, aIndent, EmptyCString());
   }
   virtual void DumpContent(FILE* out, int32_t aIndent, bool aDumpAll) const override;
   void List(FILE* out, int32_t aIndent, const nsCString& aPrefix) const;
   void ListAttributes(FILE* out) const;
@@ -1785,20 +1804,19 @@ protected:
    */
   virtual void GetLinkTarget(nsAString& aTarget);
 
   nsDOMTokenList* GetTokenList(nsAtom* aAtom,
                                const DOMTokenListSupportedTokenArray aSupportedTokens = nullptr);
 
 private:
   /**
-   * Hook for implementing GetClasses.  This is guaranteed to only be
-   * called if the NODE_MAY_HAVE_CLASS flag is set.
+   * Slow path for DoGetClasses, this should only be called for SVG elements.
    */
-  const nsAttrValue* DoGetClasses() const;
+  const nsAttrValue* GetSVGAnimatedClass() const;
 
   /**
    * Get this element's client area rect in app units.
    * @return the frame's client area
    */
   nsRect GetClientAreaRect();
 
   nsIScrollableFrame* GetScrollFrame(nsIFrame **aStyledFrame = nullptr,
--- a/layout/style/ServoBindings.cpp
+++ b/layout/style/ServoBindings.cpp
@@ -1089,17 +1089,17 @@ AttrHasSuffix(Implementor* aElement, nsA
  *
  * 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->GetClasses();
+  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)) {
--- a/layout/style/ServoElementSnapshot.h
+++ b/layout/style/ServoElementSnapshot.h
@@ -146,17 +146,17 @@ public:
       if (mAttrs[i].mName.Equals(aLocalName, aNamespaceID)) {
         return &mAttrs[i].mValue;
       }
     }
 
     return nullptr;
   }
 
-  const nsAttrValue* GetClasses() const
+  const nsAttrValue* DoGetClasses() const
   {
     MOZ_ASSERT(HasAttrs());
     return &mClass;
   }
 
   bool IsInChromeDocument() const { return mIsInChromeDocument; }
   bool SupportsLangAttr() const { return mSupportsLangAttr; }