Bug 1455885: Make the SVG context paint not use a node property, but a member in SVGDocument. r?jwatt draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Sat, 21 Apr 2018 18:48:29 +0200
changeset 787406 3ad6dd36573a889f7540a2ad51095b52f4c139be
parent 787405 1e146f9b6c531b30e3513b5882f7e9dbc52f03e4
push id107721
push userbmo:emilio@crisal.io
push dateTue, 24 Apr 2018 20:07:40 +0000
reviewersjwatt
bugs1455885
milestone61.0a1
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
gfx/thebes/gfxSVGGlyphs.cpp
image/SVGDocumentWrapper.cpp
image/SVGDocumentWrapper.h
image/VectorImage.cpp
layout/svg/SVGContextPaint.cpp
layout/svg/SVGContextPaint.h
xpcom/ds/nsGkAtomList.h
--- a/dom/svg/SVGDocument.h
+++ b/dom/svg/SVGDocument.h
@@ -8,16 +8,19 @@
 #define mozilla_dom_SVGDocument_h
 
 #include "mozilla/Attributes.h"
 #include "mozilla/dom/XMLDocument.h"
 
 class nsSVGElement;
 
 namespace mozilla {
+
+class SVGContextPaint;
+
 namespace dom {
 
 class SVGForeignObjectElement;
 
 class SVGDocument final : public XMLDocument
 {
   friend class SVGForeignObjectElement; // To call EnsureNonSVGUserAgentStyleSheetsLoaded
 
@@ -31,24 +34,33 @@ public:
 
   virtual nsresult InsertChildBefore(nsIContent* aKid, nsIContent* aBeforeThis,
                                      bool aNotify) override;
   virtual nsresult InsertChildAt_Deprecated(nsIContent* aKid, uint32_t aIndex,
                                             bool aNotify) override;
   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
 } // namespace mozilla
 
 inline mozilla::dom::SVGDocument*
 nsIDocument::AsSVGDocument()
 {
--- a/gfx/thebes/gfxSVGGlyphs.cpp
+++ b/gfx/thebes/gfxSVGGlyphs.cpp
@@ -18,16 +18,17 @@
 #include "nsNetUtil.h"
 #include "NullPrincipal.h"
 #include "nsIInputStream.h"
 #include "nsStringStream.h"
 #include "nsStreamUtils.h"
 #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"
 #include "nsContentUtils.h"
 #include "gfxFont.h"
 #include "nsSMILAnimationController.h"
 #include "gfxContext.h"
 #include "harfbuzz/hb.h"
@@ -210,20 +211,21 @@ gfxSVGGlyphsDocument::FindGlyphElements(
  * @return true iff rendering succeeded
  */
 void
 gfxSVGGlyphs::RenderGlyph(gfxContext *aContext, uint32_t aGlyphId,
                           SVGContextPaint* aContextPaint)
 {
     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);
 }
 
 bool
 gfxSVGGlyphs::GetGlyphExtents(uint32_t aGlyphId, const gfxMatrix& aSVGToAppSpace,
                               gfxRect *aResult)
 {
--- a/image/SVGDocumentWrapper.cpp
+++ b/image/SVGDocumentWrapper.cpp
@@ -2,20 +2,20 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "SVGDocumentWrapper.h"
 
 #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"
 #include "nsIParser.h"
 #include "nsIPresShell.h"
 #include "nsIRequest.h"
 #include "nsIStreamListener.h"
 #include "nsIXMLContentSink.h"
@@ -413,29 +413,32 @@ SVGDocumentWrapper::UnregisterForXPCOMSh
   } else {
     mRegisteredForXPCOMShutdown = false;
   }
 }
 
 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*
 SVGDocumentWrapper::GetRootSVGElem()
 {
   if (!mViewer) {
     return nullptr; // Can happen during destruction
   }
--- a/image/SVGDocumentWrapper.h
+++ b/image/SVGDocumentWrapper.h
@@ -25,16 +25,17 @@ class nsIFrame;
 #define OBSERVER_SVC_CID "@mozilla.org/observer-service;1"
 
 // undef the GetCurrentTime macro defined in WinBase.h from the MS Platform SDK
 #undef GetCurrentTime
 
 namespace mozilla {
 namespace dom {
 class SVGSVGElement;
+class SVGDocument;
 } // namespace dom
 
 namespace image {
 
 class SVGDocumentWrapper final : public nsIStreamListener,
                                  public nsIObserver,
                                  nsSupportsWeakReference
 {
@@ -49,17 +50,17 @@ public:
   enum Dimension {
     eWidth,
     eHeight
   };
 
   /**
    * 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
    * failure.
    */
   mozilla::dom::SVGSVGElement* GetRootSVGElem();
 
   /**
--- a/image/VectorImage.cpp
+++ b/image/VectorImage.cpp
@@ -10,16 +10,17 @@
 #include "gfxDrawable.h"
 #include "gfxPlatform.h"
 #include "gfxUtils.h"
 #include "imgFrame.h"
 #include "mozilla/AutoRestore.h"
 #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"
 #include "nsIPresShell.h"
 #include "nsIStreamListener.h"
 #include "nsMimeTypes.h"
 #include "nsPresContext.h"
 #include "nsRect.h"
@@ -122,17 +123,17 @@ protected:
 };
 
 NS_IMPL_ISUPPORTS(SVGRootRenderingObserver, nsIMutationObserver)
 
 class SVGParseCompleteListener final : public nsStubDocumentObserver {
 public:
   NS_DECL_ISUPPORTS
 
-  SVGParseCompleteListener(nsIDocument* aDocument,
+  SVGParseCompleteListener(SVGDocument* aDocument,
                            VectorImage* aImage)
     : mDocument(aDocument)
     , mImage(aImage)
   {
     MOZ_ASSERT(mDocument, "Need an SVG document");
     MOZ_ASSERT(mImage, "Need an image");
 
     mDocument->AddObserver(this);
@@ -166,17 +167,17 @@ public:
     MOZ_ASSERT(mDocument, "Duplicate call to Cancel");
     if (mDocument) {
       mDocument->RemoveObserver(this);
       mDocument = nullptr;
     }
   }
 
 private:
-  nsCOMPtr<nsIDocument> mDocument;
+  RefPtr<SVGDocument> mDocument;
   VectorImage* const mImage; // Raw pointer to owner.
 };
 
 NS_IMPL_ISUPPORTS(SVGParseCompleteListener, nsIDocumentObserver)
 
 class SVGLoadEventListener final : public nsIDOMEventListener {
 public:
   NS_DECL_ISUPPORTS
@@ -337,22 +338,25 @@ public:
     : mIsDrawing(aIsDrawing)
     // Apply any 'preserveAspectRatio' override (if specified) to the root
     // element:
     , mPAR(aParams.svgContext, aSVGDocumentWrapper->GetRootSVGElem())
     // Set the animation time:
     , 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());
     }
   }
 
 private:
   AutoRestore<bool> mIsDrawing;
   AutoPreserveAspectRatioOverride mPAR;
   AutoSVGTimeSetRestore mTime;
   Maybe<AutoSetRestoreSVGContextPaint> mContextPaint;
@@ -413,17 +417,17 @@ VectorImage::Init(const char* aMimeType,
 
 size_t
 VectorImage::SizeOfSourceWithComputedFallback(SizeOfState& aState) const
 {
   if (!mSVGDocumentWrapper) {
     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.
   }
 
   nsWindowSizes windowSizes(aState);
   doc->DocAddSizeOfIncludingThis(windowSizes);
 
   if (windowSizes.getTotalSize() == 0) {
@@ -1368,17 +1372,17 @@ VectorImage::OnStartRequest(nsIRequest* 
   }
 
   // Create a listener to wait until the SVG document is fully loaded, which
   // will signal that this image is ready to render. Certain error conditions
   // will prevent us from ever getting this notification, so we also create a
   // 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);
 
   return NS_OK;
 }
 
 //******************************************************************************
 NS_IMETHODIMP
--- a/layout/svg/SVGContextPaint.cpp
+++ b/layout/svg/SVGContextPaint.cpp
@@ -4,16 +4,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "SVGContextPaint.h"
 
 #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"
 #include "SVGObserverUtils.h"
 #include "nsSVGPaintServerFrame.h"
 
 using namespace mozilla::gfx;
 using namespace mozilla::image;
@@ -207,33 +208,35 @@ SVGContextPaint::InitStrokeGeometry(gfxC
   mDashOffset /= devUnitsPerSVGUnit;
 }
 
 /* static */ SVGContextPaint*
 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
   // const.  Trying to fix SVGContextPaint (perhaps by using |mutable|) is a
   // 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>
 SVGContextPaintImpl::GetFillPattern(const DrawTarget* aDrawTarget,
                                     float aOpacity,
                                     const gfxMatrix& aCTM,
                                     imgDrawingParams& aImgParams)
 {
@@ -320,49 +323,30 @@ SVGContextPaintImpl::Paint::GetPattern(c
     return nullptr;
   }
 
   mPatternCache.Put(aOpacity, pattern);
   return pattern.forget();
 }
 
 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);
 }
 
 
 // SVGEmbeddingContextPaint
 
 already_AddRefed<gfxPattern>
 SVGEmbeddingContextPaint::GetFillPattern(const DrawTarget* aDrawTarget,
                                          float aFillOpacity,
--- a/layout/svg/SVGContextPaint.h
+++ b/layout/svg/SVGContextPaint.h
@@ -21,16 +21,20 @@
 #include "ImgDrawResult.h"
 
 class gfxContext;
 class nsIDocument;
 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
  * keywords. See:
  *
  *   https://www.w3.org/TR/SVG2/painting.html#context-paint
  *
  * This feature allows the color in an SVG-in-OpenType glyph to come from the
@@ -134,24 +138,24 @@ private:
  * piece of SVG is being painted.  The context paint is set on the SVG's owner
  * document, as expected by SVGContextPaint::GetContextPaint.  Any pre-existing
  * context paint is restored after this class removes the context paint that it
  * set.
  */
 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;
 };
 
 
 /**
  * This class should be flattened into SVGContextPaint once we get rid of the
  * other sub-class (SimpleTextContextPaint).
  */
 struct SVGContextPaintImpl : public SVGContextPaint
--- a/xpcom/ds/nsGkAtomList.h
+++ b/xpcom/ds/nsGkAtomList.h
@@ -1687,17 +1687,16 @@ GK_ATOM(stroke_dashoffset, "stroke-dasho
 GK_ATOM(stroke_linecap, "stroke-linecap")
 GK_ATOM(stroke_linejoin, "stroke-linejoin")
 GK_ATOM(stroke_miterlimit, "stroke-miterlimit")
 GK_ATOM(stroke_opacity, "stroke-opacity")
 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")
 GK_ATOM(tableValues, "tableValues")
 GK_ATOM(targetX, "targetX")
 GK_ATOM(targetY, "targetY")
 GK_ATOM(text_anchor, "text-anchor")
 GK_ATOM(text_rendering, "text-rendering")