Bug 1455885: Make the SVG context paint not use a node property, but a member in SVGDocument. r=jwatt
authorEmilio Cobos Álvarez <emilio@crisal.io>
Sat, 21 Apr 2018 18:48:29 +0200
changeset 415817 e0fe3a00acc70b4f413cd8906576ec02f87cc6c7
parent 415816 e4bba542fa45c827481d8366691ccb6b02dbe53a
child 415818 3574187cbf667733e0105a48da473f6667c5eaf0
push id33910
push usershindli@mozilla.com
push dateThu, 26 Apr 2018 21:39:52 +0000
treeherdermozilla-central@63a0e2f626fe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwatt
bugs1455885
milestone61.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 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")