Bug 1326388 part 1. Switch nsSVGElement from storing a StyleRule to storing a DeclarationBlock to represent its mapped attributes. r=dbaron
authorBoris Zbarsky <bzbarsky@mit.edu>
Sat, 31 Dec 2016 01:10:45 -0800
changeset 327744 13b297c37408524ae3891d550bfd28e03bfa3c76
parent 327743 2caf84698f926b00b3f092694b76924e656ee867
child 327745 9f6e6c283faa4d56ed3115a55de81bf2ab66c50c
push id31146
push userphilringnalda@gmail.com
push dateSat, 31 Dec 2016 19:07:34 +0000
treeherdermozilla-central@cb9f43eb5525 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron
bugs1326388
milestone53.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 1326388 part 1. Switch nsSVGElement from storing a StyleRule to storing a DeclarationBlock to represent its mapped attributes. r=dbaron The current setup complicates things by causing the existence of StyleRule instances that have a null GetDOMRule(), which inDOMUtils::GetCSSStyleRulesrelies on to not return the relevant rules. Since we want to get rid of GetDOMRule(), better to not have a StyleRule there in the first place.
dom/svg/nsSVGElement.cpp
dom/svg/nsSVGElement.h
--- a/dom/svg/nsSVGElement.cpp
+++ b/dom/svg/nsSVGElement.cpp
@@ -48,16 +48,18 @@
 #include "nsIFrame.h"
 #include "nsQueryObject.h"
 #include <stdarg.h>
 #include "nsSMILMappedAttribute.h"
 #include "SVGMotionSMILAttr.h"
 #include "nsAttrValueOrString.h"
 #include "nsSMILAnimationController.h"
 #include "mozilla/dom/SVGElementBinding.h"
+#include "mozilla/DeclarationBlock.h"
+#include "mozilla/DeclarationBlockInlines.h"
 #include "mozilla/Unused.h"
 #include "mozilla/RestyleManagerHandle.h"
 #include "mozilla/RestyleManagerHandleInlines.h"
 
 using namespace mozilla;
 using namespace mozilla::dom;
 
 // This is needed to ensure correct handling of calls to the
@@ -89,16 +91,20 @@ nsSVGEnumMapping nsSVGElement::sSVGUnitT
   {nullptr, 0}
 };
 
 nsSVGElement::nsSVGElement(already_AddRefed<mozilla::dom::NodeInfo>& aNodeInfo)
   : nsSVGElementBase(aNodeInfo)
 {
 }
 
+nsSVGElement::~nsSVGElement()
+{
+}
+
 JSObject*
 nsSVGElement::WrapNode(JSContext *aCx, JS::Handle<JSObject*> aGivenProto)
 {
   return SVGElementBinding::Wrap(aCx, this, aGivenProto);
 }
 
 //----------------------------------------------------------------------
 
@@ -288,22 +294,21 @@ nsSVGElement::AfterSetAttr(int32_t aName
   // member data of SVG elements and if an nsAttrValue outlives the SVG element
   // whose data it points to (by virtue of being stored in
   // mAttrsAndChildren->mMappedAttributes, meaning it's shared between
   // elements), the pointer will dangle. See bug 724680.
   MOZ_ASSERT(!mAttrsAndChildren.HasMappedAttrs(),
              "Unexpected use of nsMappedAttributes within SVG");
 
   // If this is an svg presentation attribute we need to map it into
-  // the content stylerule.
+  // the content declaration block.
   // XXX For some reason incremental mapping doesn't work, so for now
-  // just delete the style rule and lazily reconstruct it in
-  // GetContentStyleRule()
+  // just delete the style rule and lazily reconstruct it as needed).
   if (aNamespaceID == kNameSpaceID_None && IsAttributeMapped(aName)) {
-    mContentStyleRule = nullptr;
+    mContentDeclarationBlock = nullptr;
   }
 
   if (IsEventAttributeName(aName) && aValue) {
     MOZ_ASSERT(aValue->Type() == nsAttrValue::eString,
                "Expected string value for script body");
     nsresult rv = SetEventHandler(GetEventNameForAttr(aName),
                                   aValue->GetStringValue());
     NS_ENSURE_SUCCESS(rv, rv);
@@ -654,19 +659,21 @@ nsSVGElement::ParseAttribute(int32_t aNa
 void
 nsSVGElement::UnsetAttrInternal(int32_t aNamespaceID, nsIAtom* aName,
                                 bool aNotify)
 {
   // XXXbz there's a bunch of redundancy here with AfterSetAttr.
   // Maybe consolidate?
 
   if (aNamespaceID == kNameSpaceID_None) {
-    // If this is an svg presentation attribute, remove rule to force an update
-    if (IsAttributeMapped(aName))
-      mContentStyleRule = nullptr;
+    // If this is an svg presentation attribute, remove declaration block to
+    // force an update
+    if (IsAttributeMapped(aName)) {
+      mContentDeclarationBlock = nullptr;
+    }
 
     if (IsEventAttributeName(aName)) {
       EventListenerManager* manager = GetExistingListenerManager();
       if (manager) {
         nsIAtom* eventName = GetEventNameForAttr(aName);
         manager->RemoveEventHandler(eventName, EmptyString());
       }
       return;
@@ -898,21 +905,22 @@ nsSVGElement::IsNodeOfType(uint32_t aFla
 }
 
 NS_IMETHODIMP
 nsSVGElement::WalkContentStyleRules(nsRuleWalker* aRuleWalker)
 {
 #ifdef DEBUG
 //  printf("nsSVGElement(%p)::WalkContentStyleRules()\n", this);
 #endif
-  if (!mContentStyleRule)
-    UpdateContentStyleRule();
-
-  if (mContentStyleRule) {
-    css::Declaration* declaration = mContentStyleRule->GetDeclaration();
+  if (!mContentDeclarationBlock) {
+    UpdateContentDeclarationBlock();
+  }
+
+  if (mContentDeclarationBlock) {
+    css::Declaration* declaration = mContentDeclarationBlock->AsGecko();
     declaration->SetImmutable();
     aRuleWalker->Forward(declaration);
   }
 
   return NS_OK;
 }
 
 void
@@ -1148,51 +1156,55 @@ public:
                    already_AddRefed<nsIURI> aBaseURI,
                    nsSVGElement* aElement);
   ~MappedAttrParser();
 
   // Parses a mapped attribute value.
   void ParseMappedAttrValue(nsIAtom* aMappedAttrName,
                             const nsAString& aMappedAttrValue);
 
-  // If we've parsed any values for mapped attributes, this method returns
-  // a new already_AddRefed css::StyleRule that incorporates the parsed
+  // If we've parsed any values for mapped attributes, this method returns the
+  // already_AddRefed css::Declaration that incorporates the parsed
   // values. Otherwise, this method returns null.
+  already_AddRefed<css::Declaration> GetDeclarationBlock();
+
+  // Like GetDeclarationBlock(), but returns a StyleRule.  It's about to go
+  // away once we convert the animation bits to GetDeclarationBlock().
   already_AddRefed<css::StyleRule> CreateStyleRule();
 
 private:
   // MEMBER DATA
   // -----------
   nsCSSParser       mParser;
 
   // Arguments for nsCSSParser::ParseProperty
   nsIURI*           mDocURI;
   nsCOMPtr<nsIURI>  mBaseURI;
 
   // Declaration for storing parsed values (lazily initialized)
-  css::Declaration* mDecl;
+  RefPtr<css::Declaration> mDecl;
 
   // For reporting use counters
   nsSVGElement*     mElement;
 };
 
 MappedAttrParser::MappedAttrParser(css::Loader* aLoader,
                                    nsIURI* aDocURI,
                                    already_AddRefed<nsIURI> aBaseURI,
                                    nsSVGElement* aElement)
   : mParser(aLoader), mDocURI(aDocURI), mBaseURI(aBaseURI),
-    mDecl(nullptr), mElement(aElement)
+    mElement(aElement)
 {
 }
 
 MappedAttrParser::~MappedAttrParser()
 {
   MOZ_ASSERT(!mDecl,
-             "If mDecl was initialized, it should have been converted "
-             "into a style rule (and had its pointer cleared)");
+             "If mDecl was initialized, it should have been returned via "
+             "GetDeclarationBlock (and had its pointer cleared)");
 }
 
 void
 MappedAttrParser::ParseMappedAttrValue(nsIAtom* aMappedAttrName,
                                        const nsAString& aMappedAttrValue)
 {
   if (!mDecl) {
     mDecl = new css::Declaration();
@@ -1236,16 +1248,22 @@ MappedAttrParser::ParseMappedAttrValue(n
     mDecl->ExpandTo(&block);
     nsCSSValue cssValue(PromiseFlatString(aMappedAttrValue), eCSSUnit_Ident);
     block.AddLonghandProperty(propertyID, cssValue);
     mDecl->ValueAppended(propertyID);
     mDecl->CompressFrom(&block);
   }
 }
 
+already_AddRefed<css::Declaration>
+MappedAttrParser::GetDeclarationBlock()
+{
+  return mDecl.forget();
+}
+
 already_AddRefed<css::StyleRule>
 MappedAttrParser::CreateStyleRule()
 {
   if (!mDecl) {
     return nullptr; // No mapped attributes were parsed
   }
 
   RefPtr<css::StyleRule> rule = new css::StyleRule(nullptr, mDecl, 0, 0);
@@ -1254,19 +1272,20 @@ MappedAttrParser::CreateStyleRule()
 }
 
 } // namespace
 
 //----------------------------------------------------------------------
 // Implementation Helpers:
 
 void
-nsSVGElement::UpdateContentStyleRule()
+nsSVGElement::UpdateContentDeclarationBlock()
 {
-  NS_ASSERTION(!mContentStyleRule, "we already have a content style rule");
+  NS_ASSERTION(!mContentDeclarationBlock,
+               "we already have a content declaration block");
 
   uint32_t attrCount = mAttrsAndChildren.AttrCount();
   if (!attrCount) {
     // nothing to do
     return;
   }
 
   nsIDocument* doc = OwnerDoc();
@@ -1305,17 +1324,17 @@ nsSVGElement::UpdateContentStyleRule()
         continue;
       }
     }
 
     nsAutoString value;
     mAttrsAndChildren.AttrAt(i)->ToString(value);
     mappedAttrParser.ParseMappedAttrValue(attrName->Atom(), value);
   }
-  mContentStyleRule = mappedAttrParser.CreateStyleRule();
+  mContentDeclarationBlock = mappedAttrParser.GetDeclarationBlock();
 }
 
 static void
 ParseMappedAttrAnimValueCallback(void*    aObject,
                                  nsIAtom* aPropertyName,
                                  void*    aPropertyValue,
                                  void*    aData)
 {
--- a/dom/svg/nsSVGElement.h
+++ b/dom/svg/nsSVGElement.h
@@ -34,16 +34,18 @@ class nsSVGInteger;
 class nsSVGIntegerPair;
 class nsSVGLength2;
 class nsSVGNumber2;
 class nsSVGNumberPair;
 class nsSVGString;
 class nsSVGViewBox;
 
 namespace mozilla {
+class DeclarationBlock;
+
 namespace dom {
 class SVGSVGElement;
 
 static const unsigned short SVG_UNIT_TYPE_UNKNOWN           = 0;
 static const unsigned short SVG_UNIT_TYPE_USERSPACEONUSE    = 1;
 static const unsigned short SVG_UNIT_TYPE_OBJECTBOUNDINGBOX = 2;
 
 } // namespace dom
@@ -73,17 +75,17 @@ typedef nsStyledElement nsSVGElementBase
 class nsSVGElement : public nsSVGElementBase    // nsIContent
                    , public nsIDOMSVGElement
 {
 protected:
   explicit nsSVGElement(already_AddRefed<mozilla::dom::NodeInfo>& aNodeInfo);
   friend nsresult NS_NewSVGElement(mozilla::dom::Element **aResult,
                                    already_AddRefed<mozilla::dom::NodeInfo>&& aNodeInfo);
   nsresult Init();
-  virtual ~nsSVGElement(){}
+  virtual ~nsSVGElement();
 
 public:
 
   virtual nsresult Clone(mozilla::dom::NodeInfo *aNodeInfo, nsINode **aResult) const MOZ_MUST_OVERRIDE override;
 
   typedef mozilla::SVGNumberList SVGNumberList;
   typedef mozilla::SVGAnimatedNumberList SVGAnimatedNumberList;
   typedef mozilla::SVGUserUnitList SVGUserUnitList;
@@ -338,17 +340,17 @@ protected:
   virtual nsresult AfterSetAttr(int32_t aNamespaceID, nsIAtom* aName,
                                 const nsAttrValue* aValue, bool aNotify) override;
   virtual bool ParseAttribute(int32_t aNamespaceID, nsIAtom* aAttribute,
                                 const nsAString& aValue, nsAttrValue& aResult) override;
   static nsresult ReportAttributeParseFailure(nsIDocument* aDocument,
                                               nsIAtom* aAttribute,
                                               const nsAString& aValue);
 
-  void UpdateContentStyleRule();
+  void UpdateContentDeclarationBlock();
   void UpdateAnimatedContentStyleRule();
   mozilla::css::StyleRule* GetAnimatedContentStyleRule();
 
   nsAttrValue WillChangeValue(nsIAtom* aName);
   // aNewValue is set to the old value. This value may be invalid if
   // !StoresOwnData.
   void DidChangeValue(nsIAtom* aName, const nsAttrValue& aEmptyOrOldValue,
                       nsAttrValue& aNewValue);
@@ -630,17 +632,17 @@ protected:
   static nsSVGEnumMapping sSVGUnitTypesMap[];
 
 private:
   void UnsetAttrInternal(int32_t aNameSpaceID, nsIAtom* aAttribute,
                          bool aNotify);
 
   nsSVGClass mClassAttribute;
   nsAutoPtr<nsAttrValue> mClassAnimAttr;
-  RefPtr<mozilla::css::StyleRule> mContentStyleRule;
+  RefPtr<mozilla::DeclarationBlock> mContentDeclarationBlock;
 };
 
 /**
  * A macro to implement the NS_NewSVGXXXElement() functions.
  */
 #define NS_IMPL_NS_NEW_SVG_ELEMENT(_elementName)                             \
 nsresult                                                                     \
 NS_NewSVG##_elementName##Element(nsIContent **aResult,                       \