Bug 1455885: Make the SVG context paint not use a node property, but a member in SVGDocument. r=jwatt

MozReview-Commit-ID: H6SRTsDL5Rh

dom/svg/SVGDocument.h

@@ -13,6 +13,9 @@
 class nsSVGElement;
 
 namespace mozilla {
+
+class SVGContextPaint;
+
 namespace dom {
 
 class SVGForeignObjectElement;
@@ -36,14 +39,23 @@ public:
   virtual nsresult Clone(mozilla::dom::NodeInfo *aNodeInfo, nsINode **aResult,
                          bool aPreallocateChildren) const override;
 
-  virtual SVGDocument* AsSVGDocument() override {
-    return this;
+  void SetCurrentContextPaint(const SVGContextPaint* aContextPaint)
+  {
+    mCurrentContextPaint = aContextPaint;
+  }
+
+  const SVGContextPaint* GetCurrentContextPaint() const
+  {
+    return mCurrentContextPaint;
   }
 
 private:
   void EnsureNonSVGUserAgentStyleSheetsLoaded();
 
   bool mHasLoadedNonSVGUserAgentStyleSheets;
+
+  // This is maintained by AutoSetRestoreSVGContextPaint.
+  const SVGContextPaint* mCurrentContextPaint = nullptr;
 };
 
 } // namespace dom

gfx/thebes/gfxSVGGlyphs.cpp

@@ -23,6 +23,7 @@
 #include "nsIPrincipal.h"
 #include "mozilla/BasePrincipal.h"
 #include "mozilla/dom/Element.h"
+#include "mozilla/dom/SVGDocument.h"
 #include "mozilla/LoadInfo.h"
 #include "nsSVGUtils.h"
 #include "nsHostObjectProtocolHandler.h"
@@ -215,10 +216,11 @@ gfxSVGGlyphs::RenderGlyph(gfxContext *aContext, uint32_t aGlyphId,
 {
     gfxContextAutoSaveRestore aContextRestorer(aContext);
 
-    Element *glyph = mGlyphIdMap.Get(aGlyphId);
+    Element* glyph = mGlyphIdMap.Get(aGlyphId);
     MOZ_ASSERT(glyph, "No glyph element. Should check with HasSVGGlyph() first!");
 
-    AutoSetRestoreSVGContextPaint autoSetRestore(aContextPaint, glyph->OwnerDoc());
+    AutoSetRestoreSVGContextPaint autoSetRestore(
+        *aContextPaint, *glyph->OwnerDoc()->AsSVGDocument());
 
     nsSVGUtils::PaintSVGGlyph(glyph, aContext);
 }

image/SVGDocumentWrapper.cpp

@@ -7,10 +7,10 @@
 
 #include "mozilla/dom/DocumentTimeline.h"
 #include "mozilla/dom/Element.h"
+#include "mozilla/dom/SVGDocument.h"
 #include "nsICategoryManager.h"
 #include "nsIChannel.h"
 #include "nsIContentViewer.h"
-#include "nsIDocument.h"
 #include "nsIDocumentLoaderFactory.h"
 #include "nsIHttpChannel.h"
 #include "nsIObserverService.h"
@@ -418,19 +418,22 @@ SVGDocumentWrapper::UnregisterForXPCOMShutdown()
 void
 SVGDocumentWrapper::FlushLayout()
 {
-  if (nsIDocument* doc = GetDocument()) {
+  if (SVGDocument* doc = GetDocument()) {
     doc->FlushPendingNotifications(FlushType::Layout);
   }
 }
 
-nsIDocument*
+SVGDocument*
 SVGDocumentWrapper::GetDocument()
 {
   if (!mViewer) {
     return nullptr;
   }
-
-  return mViewer->GetDocument(); // May be nullptr.
+  nsIDocument* doc = mViewer->GetDocument();
+  if (!doc) {
+    return nullptr;
+  }
+  return doc->AsSVGDocument();
 }
 
 SVGSVGElement*

image/SVGDocumentWrapper.h

@@ -30,6 +30,7 @@ class nsIFrame;
 namespace mozilla {
 namespace dom {
 class SVGSVGElement;
+class SVGDocument;
 } // namespace dom
 
 namespace image {
@@ -54,7 +55,7 @@ public:
   /**
    * Returns the wrapped document, or nullptr on failure. (No AddRef.)
    */
-  nsIDocument* GetDocument();
+  mozilla::dom::SVGDocument* GetDocument();
 
   /**
    * Returns the root <svg> element for the wrapped document, or nullptr on

image/VectorImage.cpp

@@ -15,6 +15,7 @@
 #include "mozilla/MemoryReporting.h"
 #include "mozilla/dom/Event.h"
 #include "mozilla/dom/SVGSVGElement.h"
+#include "mozilla/dom/SVGDocument.h"
 #include "mozilla/gfx/2D.h"
 #include "mozilla/RefPtr.h"
 #include "mozilla/Tuple.h"
@@ -127,7 +128,7 @@ class SVGParseCompleteListener final : public nsStubDocumentObserver {
 public:
   NS_DECL_ISUPPORTS
 
-  SVGParseCompleteListener(nsIDocument* aDocument,
+  SVGParseCompleteListener(SVGDocument* aDocument,
                            VectorImage* aImage)
     : mDocument(aDocument)
     , mImage(aImage)
@@ -171,7 +172,7 @@ public:
   }
 
 private:
-  nsCOMPtr<nsIDocument> mDocument;
+  RefPtr<SVGDocument> mDocument;
   VectorImage* const mImage; // Raw pointer to owner.
 };
 
@@ -342,12 +343,15 @@ public:
     , mTime(aSVGDocumentWrapper->GetRootSVGElem(), aParams.animationTime)
   {
     MOZ_ASSERT(!aIsDrawing);
+    MOZ_ASSERT(aSVGDocumentWrapper->GetDocument());
+
     aIsDrawing = true;
 
     // Set context paint (if specified) on the document:
     if (aContextPaint) {
-      mContextPaint.emplace(aParams.svgContext->GetContextPaint(),
-                            aSVGDocumentWrapper->GetDocument());
+      MOZ_ASSERT(aParams.svgContext->GetContextPaint());
+      mContextPaint.emplace(*aParams.svgContext->GetContextPaint(),
+                            *aSVGDocumentWrapper->GetDocument());
     }
   }
 
@@ -418,7 +422,7 @@ VectorImage::SizeOfSourceWithComputedFallback(SizeOfState& aState) const
     return 0; // No document, so no memory used for the document.
   }
 
-  nsIDocument* doc = mSVGDocumentWrapper->GetDocument();
+  SVGDocument* doc = mSVGDocumentWrapper->GetDocument();
   if (!doc) {
     return 0; // No document, so no memory used for the document.
   }
@@ -1373,7 +1377,7 @@ VectorImage::OnStartRequest(nsIRequest* aRequest, nsISupports* aCtxt)
   // listener that waits for parsing to complete and cancels the
   // SVGLoadEventListener if needed. The listeners are automatically attached
   // to the document by their constructors.
-  nsIDocument* document = mSVGDocumentWrapper->GetDocument();
+  SVGDocument* document = mSVGDocumentWrapper->GetDocument();
   mLoadEventListener = new SVGLoadEventListener(document, this);
   mParseCompleteListener = new SVGParseCompleteListener(document, this);
 

layout/svg/SVGContextPaint.cpp

@@ -9,6 +9,7 @@
 #include "gfxContext.h"
 #include "gfxUtils.h"
 #include "mozilla/gfx/2D.h"
+#include "mozilla/dom/SVGDocument.h"
 #include "mozilla/Preferences.h"
 #include "nsIDocument.h"
 #include "nsSVGPaintServerFrame.h"
@@ -212,13 +213,17 @@ SVGContextPaint::GetContextPaint(nsIContent* aContent)
 {
   nsIDocument* ownerDoc = aContent->OwnerDoc();
 
-  if (!ownerDoc->IsBeingUsedAsImage()) {
+  if (!ownerDoc->IsSVGDocument()) {
     return nullptr;
   }
 
-  // XXX The SVGContextPaint that was passed to SetProperty was const. Ideally
-  // we could and should re-apply that constness to the SVGContextPaint that
-  // we get here (SVGImageContext is never changed after it is initialized).
+  auto* contextPaint = ownerDoc->AsSVGDocument()->GetCurrentContextPaint();
+  MOZ_ASSERT_IF(contextPaint, ownerDoc->IsBeingUsedAsImage());
+
+  // XXX The SVGContextPaint that SVGDocument keeps around is const. We could
+  // and should keep that constness to the SVGContextPaint that we get here
+  // (SVGImageContext is never changed after it is initialized).
+  //
   // Unfortunately lazy initialization of SVGContextPaint (which is a member of
   // SVGImageContext, and also conceptually never changes after construction)
   // prevents some of SVGContextPaint's conceptually const methods from being
@@ -226,9 +231,7 @@ SVGContextPaint::GetContextPaint(nsIContent* aContent)
   // bit of a headache so for now we punt on that, don't reapply the constness
   // to the SVGContextPaint here, and trust that no one will add code that
   // actually modifies the object.
-
-  return static_cast<SVGContextPaint*>(
-           ownerDoc->GetProperty(nsGkAtoms::svgContextPaint));
+  return const_cast<SVGContextPaint*>(contextPaint);
 }
 
 already_AddRefed<gfxPattern>
@@ -325,39 +328,20 @@ SVGContextPaintImpl::Paint::GetPattern(const DrawTarget* aDrawTarget,
 }
 
 AutoSetRestoreSVGContextPaint::AutoSetRestoreSVGContextPaint(
-                                 const SVGContextPaint* aContextPaint,
-                                 nsIDocument* aSVGDocument)
+                                 const SVGContextPaint& aContextPaint,
+                                 dom::SVGDocument& aSVGDocument)
   : mSVGDocument(aSVGDocument)
-  , mOuterContextPaint(aSVGDocument->GetProperty(nsGkAtoms::svgContextPaint))
+  , mOuterContextPaint(aSVGDocument.GetCurrentContextPaint())
 {
-  // The way that we supply context paint is to temporarily set the context
-  // paint on the owner document of the SVG that we're painting while it's
-  // being painted.
-
-  MOZ_ASSERT(aContextPaint);
-  MOZ_ASSERT(aSVGDocument->IsBeingUsedAsImage(),
+  MOZ_ASSERT(aSVGDocument.IsBeingUsedAsImage(),
              "SVGContextPaint::GetContextPaint assumes this");
 
-  if (mOuterContextPaint) {
-    mSVGDocument->UnsetProperty(nsGkAtoms::svgContextPaint);
-  }
-
-  DebugOnly<nsresult> res =
-    mSVGDocument->SetProperty(nsGkAtoms::svgContextPaint,
-                              const_cast<SVGContextPaint*>(aContextPaint));
-
-  NS_WARNING_ASSERTION(NS_SUCCEEDED(res), "Failed to set context paint");
+  mSVGDocument.SetCurrentContextPaint(&aContextPaint);
 }
 
 AutoSetRestoreSVGContextPaint::~AutoSetRestoreSVGContextPaint()
 {
-  mSVGDocument->UnsetProperty(nsGkAtoms::svgContextPaint);
-  if (mOuterContextPaint) {
-    DebugOnly<nsresult> res =
-      mSVGDocument->SetProperty(nsGkAtoms::svgContextPaint, mOuterContextPaint);
-
-    NS_WARNING_ASSERTION(NS_SUCCEEDED(res), "Failed to restore context paint");
-  }
+  mSVGDocument.SetCurrentContextPaint(mOuterContextPaint);
 }
 
 

layout/svg/SVGContextPaint.h

@@ -26,6 +26,10 @@ class nsSVGPaintServerFrame;
 
 namespace mozilla {
 
+namespace dom {
+class SVGDocument;
+}
+
 /**
  * This class is used to pass information about a context element through to
  * SVG painting code in order to resolve the 'context-fill' and related
@@ -139,14 +143,14 @@ private:
 class MOZ_RAII AutoSetRestoreSVGContextPaint
 {
 public:
-  AutoSetRestoreSVGContextPaint(const SVGContextPaint* aContextPaint,
-                                nsIDocument* aSVGDocument);
+  AutoSetRestoreSVGContextPaint(const SVGContextPaint& aContextPaint,
+                                dom::SVGDocument& aSVGDocument);
   ~AutoSetRestoreSVGContextPaint();
 private:
-  nsIDocument* mSVGDocument;
+  dom::SVGDocument& mSVGDocument;
   // The context paint that needs to be restored by our dtor after it removes
   // aContextPaint:
-  void* mOuterContextPaint;
+  const SVGContextPaint* mOuterContextPaint;
 };
 
 

xpcom/ds/nsGkAtomList.h

@@ -1692,7 +1692,6 @@ GK_ATOM(stroke_width, "stroke-width")
 GK_ATOM(strokeWidth, "strokeWidth")
 GK_ATOM(surfaceScale, "surfaceScale")
 GK_ATOM(svg, "svg")
-GK_ATOM(svgContextPaint, "svgContextPaint")
 GK_ATOM(svgSwitch, "switch")
 GK_ATOM(symbol, "symbol")
 GK_ATOM(systemLanguage, "systemLanguage")