From ebb78498d2a74bcbdf7d1501a05c4af0723c2a50 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Sat, 31 Dec 2016 01:10:45 -0800 Subject: [PATCH] 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 | 59 ++++++++++++++++++++++++++-------------- dom/svg/nsSVGElement.h | 8 ++++-- 2 files changed, 44 insertions(+), 23 deletions(-) diff --git a/dom/svg/nsSVGElement.cpp b/dom/svg/nsSVGElement.cpp index 932a95abaf0a..78e1ef2fc217 100644 --- a/dom/svg/nsSVGElement.cpp +++ b/dom/svg/nsSVGElement.cpp @@ -53,6 +53,8 @@ #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" @@ -94,6 +96,10 @@ nsSVGElement::nsSVGElement(already_AddRefed& aNodeInfo) { } +nsSVGElement::~nsSVGElement() +{ +} + JSObject* nsSVGElement::WrapNode(JSContext *aCx, JS::Handle aGivenProto) { @@ -293,12 +299,11 @@ nsSVGElement::AfterSetAttr(int32_t aNamespaceID, nsIAtom* aName, "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) { @@ -659,9 +664,11 @@ nsSVGElement::UnsetAttrInternal(int32_t aNamespaceID, nsIAtom* aName, // 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(); @@ -903,11 +910,12 @@ nsSVGElement::WalkContentStyleRules(nsRuleWalker* aRuleWalker) #ifdef DEBUG // printf("nsSVGElement(%p)::WalkContentStyleRules()\n", this); #endif - if (!mContentStyleRule) - UpdateContentStyleRule(); + if (!mContentDeclarationBlock) { + UpdateContentDeclarationBlock(); + } - if (mContentStyleRule) { - css::Declaration* declaration = mContentStyleRule->GetDeclaration(); + if (mContentDeclarationBlock) { + css::Declaration* declaration = mContentDeclarationBlock->AsGecko(); declaration->SetImmutable(); aRuleWalker->Forward(declaration); } @@ -1153,9 +1161,13 @@ public: 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 GetDeclarationBlock(); + + // Like GetDeclarationBlock(), but returns a StyleRule. It's about to go + // away once we convert the animation bits to GetDeclarationBlock(). already_AddRefed CreateStyleRule(); private: @@ -1168,7 +1180,7 @@ private: nsCOMPtr mBaseURI; // Declaration for storing parsed values (lazily initialized) - css::Declaration* mDecl; + RefPtr mDecl; // For reporting use counters nsSVGElement* mElement; @@ -1179,15 +1191,15 @@ MappedAttrParser::MappedAttrParser(css::Loader* aLoader, already_AddRefed 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 @@ -1241,6 +1253,12 @@ MappedAttrParser::ParseMappedAttrValue(nsIAtom* aMappedAttrName, } } +already_AddRefed +MappedAttrParser::GetDeclarationBlock() +{ + return mDecl.forget(); +} + already_AddRefed MappedAttrParser::CreateStyleRule() { @@ -1259,9 +1277,10 @@ MappedAttrParser::CreateStyleRule() // 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) { @@ -1310,7 +1329,7 @@ nsSVGElement::UpdateContentStyleRule() mAttrsAndChildren.AttrAt(i)->ToString(value); mappedAttrParser.ParseMappedAttrValue(attrName->Atom(), value); } - mContentStyleRule = mappedAttrParser.CreateStyleRule(); + mContentDeclarationBlock = mappedAttrParser.GetDeclarationBlock(); } static void diff --git a/dom/svg/nsSVGElement.h b/dom/svg/nsSVGElement.h index d51740cec174..303ce2de64f5 100644 --- a/dom/svg/nsSVGElement.h +++ b/dom/svg/nsSVGElement.h @@ -39,6 +39,8 @@ class nsSVGString; class nsSVGViewBox; namespace mozilla { +class DeclarationBlock; + namespace dom { class SVGSVGElement; @@ -78,7 +80,7 @@ protected: friend nsresult NS_NewSVGElement(mozilla::dom::Element **aResult, already_AddRefed&& aNodeInfo); nsresult Init(); - virtual ~nsSVGElement(){} + virtual ~nsSVGElement(); public: @@ -343,7 +345,7 @@ protected: nsIAtom* aAttribute, const nsAString& aValue); - void UpdateContentStyleRule(); + void UpdateContentDeclarationBlock(); void UpdateAnimatedContentStyleRule(); mozilla::css::StyleRule* GetAnimatedContentStyleRule(); @@ -635,7 +637,7 @@ private: nsSVGClass mClassAttribute; nsAutoPtr mClassAnimAttr; - RefPtr mContentStyleRule; + RefPtr mContentDeclarationBlock; }; /**