Bug 1365472 - Use animated class names when doing selector matching in Servo; r?heycam draft
authorBrian Birtles <birtles@gmail.com>
Tue, 27 Jun 2017 10:55:03 -0700
changeset 603370 c91a9dbb99a1ed5da94bc7c562cf049b38aa1f05
parent 603312 a3b192dc8344679ce208af42b6246c3c0d42cab3
child 635925 e3c3d3b1ccf7cdc49624f4b43286caca20827604
push id66783
push userbbirtles@mozilla.com
push dateMon, 03 Jul 2017 23:51:19 +0000
reviewersheycam
bugs1365472, 735820
milestone56.0a1
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;