Bug 1365472 - Use animated class names when doing selector matching in Servo; r=heycam
authorBrian Birtles <birtles@gmail.com>
Tue, 27 Jun 2017 10:55:03 -0700
changeset 367173 9d0c157be7670fa138de10829d30239db7abaabd
parent 367172 9c66c38a8c93705df0c05e36f9ec5ca8942b7706
child 367174 f53f2b4b6e2caa7e65699bfc8341f16665679b9c
push id32124
push usercbook@mozilla.com
push dateTue, 04 Jul 2017 08:47:08 +0000
treeherdermozilla-central@da639765d999 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1365472, 735820
milestone56.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 1365472 - Use animated class names when doing selector matching in Servo; r=heycam Using SVG SMIL it is possible to animate the class attribute of an element using markup such as the following: <style> .red { fill: red; } </style> <svg> <circle cx="50" cy="50" r="30" fill="blue"> <set attributeName="class" to="red" begin="1s"/> </circle> </svg> In Gecko, Element::GetClasses handles this case by looking for an animated class string when the element in question is an SVG element. This patch causes our Servo bindings to use GetClasses when querying attribute values for selector matching. Note that animating the class attribute is *not* expected to affect attribute selectors such as `circle[class="red"]`. It does in Chrome, but that is due to a Blink bug where animating attributes using SMIL affects the result of getAttribute: https://bugs.chromium.org/p/chromium/issues/detail?id=735820 This patch adjusts the behavior for both the GeckoElement case and the ServoElementSnapshot case. MozReview-Commit-ID: DAFWHSH1aYB
dom/svg/nsSVGElement.cpp
layout/base/ServoRestyleManager.cpp
layout/base/ServoRestyleManager.h
layout/reftests/svg/smil/reftest.list
layout/style/ServoBindings.cpp
layout/style/ServoElementSnapshot.cpp
layout/style/ServoElementSnapshot.h
--- a/dom/svg/nsSVGElement.cpp
+++ b/dom/svg/nsSVGElement.cpp
@@ -123,24 +123,34 @@ nsSVGElement::GetStyle(nsIDOMCSSStyleDec
 }
 
 //----------------------------------------------------------------------
 // nsSVGElement methods
 
 void
 nsSVGElement::DidAnimateClass()
 {
+  // For Servo, snapshot the element before we change it.
+  nsIPresShell* shell = OwnerDoc()->GetShell();
+  if (shell) {
+    nsPresContext* presContext = shell->GetPresContext();
+    if (presContext && presContext->RestyleManager()->IsServo()) {
+      presContext->RestyleManager()
+                 ->AsServo()
+                 ->ClassAttributeWillBeChangedBySMIL(this);
+    }
+  }
+
   nsAutoString src;
   mClassAttribute.GetAnimValue(src, this);
   if (!mClassAnimAttr) {
     mClassAnimAttr = new nsAttrValue();
   }
   mClassAnimAttr->ParseAtomArray(src);
 
-  nsIPresShell* shell = OwnerDoc()->GetShell();
   if (shell) {
     shell->RestyleForAnimation(this, eRestyle_Self);
   }
 }
 
 nsresult
 nsSVGElement::Init()
 {
--- a/layout/base/ServoRestyleManager.cpp
+++ b/layout/base/ServoRestyleManager.cpp
@@ -976,16 +976,31 @@ AttributeInfluencesOtherPseudoClassState
 }
 
 void
 ServoRestyleManager::AttributeWillChange(Element* aElement,
                                          int32_t aNameSpaceID,
                                          nsIAtom* aAttribute, int32_t aModType,
                                          const nsAttrValue* aNewValue)
 {
+  TakeSnapshotForAttributeChange(aElement, aNameSpaceID, aAttribute);
+}
+
+void
+ServoRestyleManager::ClassAttributeWillBeChangedBySMIL(Element* aElement)
+{
+  TakeSnapshotForAttributeChange(aElement, kNameSpaceID_None,
+                                 nsGkAtoms::_class);
+}
+
+void
+ServoRestyleManager::TakeSnapshotForAttributeChange(Element* aElement,
+                                                    int32_t aNameSpaceID,
+                                                    nsIAtom* aAttribute)
+{
   MOZ_ASSERT(!mInStyleRefresh);
 
   if (!aElement->HasServoData()) {
     return;
   }
 
   bool influencesOtherPseudoClassState =
     AttributeInfluencesOtherPseudoClassState(aElement, aAttribute);
--- a/layout/base/ServoRestyleManager.h
+++ b/layout/base/ServoRestyleManager.h
@@ -137,16 +137,17 @@ public:
   void RestyleForAppend(nsIContent* aContainer,
                         nsIContent* aFirstNewContent);
   void ContentStateChanged(nsIContent* aContent, EventStates aStateMask);
   void AttributeWillChange(dom::Element* aElement,
                            int32_t aNameSpaceID,
                            nsIAtom* aAttribute,
                            int32_t aModType,
                            const nsAttrValue* aNewValue);
+  void ClassAttributeWillBeChangedBySMIL(dom::Element* aElement);
 
   void AttributeChanged(dom::Element* aElement, int32_t aNameSpaceID,
                         nsIAtom* aAttribute, int32_t aModType,
                         const nsAttrValue* aOldValue);
 
   nsresult ReparentStyleContext(nsIFrame* aFrame);
 
   /**
@@ -212,16 +213,19 @@ private:
                "ServoRestyleManager should only be used with a Servo-flavored "
                "style backend");
     return PresContext()->StyleSet()->AsServo();
   }
 
   const SnapshotTable& Snapshots() const { return mSnapshots; }
   void ClearSnapshots();
   ServoElementSnapshot& SnapshotFor(mozilla::dom::Element* aElement);
+  void TakeSnapshotForAttributeChange(mozilla::dom::Element* aElement,
+                                      int32_t aNameSpaceID,
+                                      nsIAtom* aAttribute);
 
   void DoProcessPendingRestyles(TraversalRestyleBehavior aRestyleBehavior);
 
   // We use a separate data structure from nsStyleChangeList because we need a
   // frame to create nsStyleChangeList entries, and the primary frame may not be
   // attached yet.
   struct ReentrantChange {
     nsCOMPtr<nsIContent> mContent;
--- a/layout/reftests/svg/smil/reftest.list
+++ b/layout/reftests/svg/smil/reftest.list
@@ -151,20 +151,20 @@ skip-if(styloVsGecko) == anim-view-01.sv
 # animate some string attributes:
 == anim-filter-href-01.svg lime.svg
 == anim-gradient-href-01.svg lime.svg
 == anim-image-href-01.svg lime.svg
 == anim-pattern-href-01.svg lime.svg
 == anim-use-href-01.svg lime.svg
 
 # animate the class attribute
-fails-if(styloVsGecko||stylo) == anim-class-01.svg lime.svg
-fails-if(styloVsGecko||stylo) == anim-class-02.svg lime.svg
-fails-if(styloVsGecko||stylo) == anim-class-03.svg lime.svg
-fails-if(styloVsGecko||stylo) == anim-class-04.svg anim-class-04-ref.svg
+== anim-class-01.svg lime.svg
+== anim-class-02.svg lime.svg
+== anim-class-03.svg lime.svg
+== anim-class-04.svg anim-class-04-ref.svg
 
 # animate with some paint server values
 fails-if(styloVsGecko||stylo) == anim-paintserver-1.svg anim-paintserver-1-ref.svg
 
 # animate attributes on text content children
 == anim-text-attr-01.svg anim-text-attr-01-ref.svg
 
 # animate where the base value is non-interpolatable but will be replaced anyway
--- a/layout/style/ServoBindings.cpp
+++ b/layout/style/ServoBindings.cpp
@@ -1028,17 +1028,17 @@ AttrHasSuffix(Implementor* aElement, nsI
  *
  * 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, nsIAtom** aClass, nsIAtom*** aClassList)
 {
-  const nsAttrValue* attr = aElement->GetParsedAttr(nsGkAtoms::_class);
+  const nsAttrValue* attr = aElement->GetClasses();
   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.
   if (attr->Type() == nsAttrValue::eString) {
     MOZ_ASSERT(nsContentUtils::TrimWhitespace<nsContentUtils::IsHTMLWhitespace>(
--- a/layout/style/ServoElementSnapshot.cpp
+++ b/layout/style/ServoElementSnapshot.cpp
@@ -62,17 +62,18 @@ ServoElementSnapshot::AddAttrs(Element* 
     const nsAttrValue* attrValue =
       aElement->GetParsedAttr(attrName->LocalName(), attrName->NamespaceID());
     mAttrs.AppendElement(ServoAttrSnapshot(*attrName, *attrValue));
   }
   mContains |= Flags::Attributes;
   if (aElement->HasID()) {
     mContains |= Flags::Id;
   }
-  if (aElement->MayHaveClass()) {
+  if (const nsAttrValue* classValue = aElement->GetClasses()) {
+    mClass = *classValue;
     mContains |= Flags::MaybeClass;
   }
 }
 
 void
 ServoElementSnapshot::AddOtherPseudoClassState(Element* aElement)
 {
   MOZ_ASSERT(aElement);
--- a/layout/style/ServoElementSnapshot.h
+++ b/layout/style/ServoElementSnapshot.h
@@ -8,16 +8,17 @@
 #define mozilla_ServoElementSnapshot_h
 
 #include "mozilla/EventStates.h"
 #include "mozilla/TypedEnumBits.h"
 #include "mozilla/dom/BorrowedAttrInfo.h"
 #include "nsAttrName.h"
 #include "nsAttrValue.h"
 #include "nsChangeHint.h"
+#include "nsGkAtoms.h"
 #include "nsIAtom.h"
 
 namespace mozilla {
 
 namespace dom {
 class Element;
 } // namespace dom
 
@@ -145,16 +146,22 @@ public:
       if (mAttrs[i].mName.Equals(aLocalName, aNamespaceID)) {
         return &mAttrs[i].mValue;
       }
     }
 
     return nullptr;
   }
 
+  const nsAttrValue* GetClasses() 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());
@@ -168,16 +175,17 @@ public:
   }
 
 private:
   // TODO: Profile, a 1 or 2 element AutoTArray could be worth it, given we know
   // we're dealing with attribute changes when we take snapshots of attributes,
   // though it can be wasted space if we deal with a lot of state-only
   // snapshots.
   nsTArray<ServoAttrSnapshot> mAttrs;
+  nsAttrValue mClass;
   ServoStateType mState;
   Flags mContains;
   bool mIsHTMLElementInHTMLDocument : 1;
   bool mIsInChromeDocument : 1;
   bool mSupportsLangAttr : 1;
   bool mIsTableBorderNonzero : 1;
   bool mIsMozBrowserFrame : 1;
   bool mClassAttributeChanged : 1;