Bug 1495330. Hide SVGObserverUtils implementation details from nsImageRenderer. r=longsonr
authorJonathan Watt <jwatt@jwatt.org>
Thu, 20 Sep 2018 13:50:23 +0100
changeset 494696 ee4151d16d1f71c87b8fab31de8c5d508c2d9631
parent 494695 6b0cb8668a09c92410b1674ca7c91984ea98ed06
child 494697 f9a8321c1ed7a38a3e9e5eb06506b875ddad2dd4
push id9984
push userffxbld-merge
push dateMon, 15 Oct 2018 21:07:35 +0000
treeherdermozilla-beta@183d27ea8570 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerslongsonr
bugs1495330
milestone64.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 1495330. Hide SVGObserverUtils implementation details from nsImageRenderer. r=longsonr Differential Revision: https://phabricator.services.mozilla.com/D7257
layout/painting/nsImageRenderer.cpp
layout/svg/SVGObserverUtils.cpp
layout/svg/SVGObserverUtils.h
--- a/layout/painting/nsImageRenderer.cpp
+++ b/layout/painting/nsImageRenderer.cpp
@@ -165,44 +165,26 @@ nsImageRenderer::PrepareImage()
       mPrepareResult = ImgDrawResult::SUCCESS;
       break;
     }
     case eStyleImageType_Gradient:
       mGradientData = mImage->GetGradientData();
       mPrepareResult = ImgDrawResult::SUCCESS;
       break;
     case eStyleImageType_Element: {
-      nsAutoString elementId =
-        NS_LITERAL_STRING("#") + nsDependentAtomString(mImage->GetElementId());
-      nsCOMPtr<nsIURI> targetURI;
-      nsCOMPtr<nsIURI> base = mForFrame->GetContent()->GetBaseURI();
-      nsContentUtils::NewURIWithDocumentCharset(
-        getter_AddRefs(targetURI),
-        elementId,
-        mForFrame->GetContent()->GetUncomposedDoc(),
-        base);
-      RefPtr<URLAndReferrerInfo> url = new URLAndReferrerInfo(
-        targetURI,
-        mForFrame->GetContent()->OwnerDoc()->GetDocumentURI(),
-        mForFrame->GetContent()->OwnerDoc()->GetReferrerPolicy());
-
-      nsSVGPaintingProperty* property = SVGObserverUtils::GetPaintingPropertyForURI(
-          url, mForFrame->FirstContinuation(),
-          SVGObserverUtils::BackgroundImageProperty());
-      if (!property) {
-        mPrepareResult = ImgDrawResult::BAD_IMAGE;
-        return false;
-      }
-
+      Element* paintElement = // may be null
+        SVGObserverUtils::GetAndObserveBackgroundImage(
+          mForFrame->FirstContinuation(), mImage->GetElementId());
       // If the referenced element is an <img>, <canvas>, or <video> element,
       // prefer SurfaceFromElement as it's more reliable.
-      mImageElementSurface = nsLayoutUtils::SurfaceFromElement(
-                               property->GetAndObserveReferencedElement());
+      mImageElementSurface = nsLayoutUtils::SurfaceFromElement(paintElement);
+
       if (!mImageElementSurface.GetSourceSurface()) {
-        nsIFrame* paintServerFrame = property->GetAndObserveReferencedFrame();
+        nsIFrame* paintServerFrame =
+          paintElement ? paintElement->GetPrimaryFrame() : nullptr;
         // If there's no referenced frame, or the referenced frame is
         // non-displayable SVG, then we have nothing valid to paint.
         if (!paintServerFrame ||
             (paintServerFrame->IsFrameOfType(nsIFrame::eSVG) &&
              !paintServerFrame->IsFrameOfType(nsIFrame::eSVGPaintServer) &&
              !static_cast<nsSVGDisplayableFrame*>(
                do_QueryFrame(paintServerFrame)))) {
           mPrepareResult = ImgDrawResult::BAD_IMAGE;
--- a/layout/svg/SVGObserverUtils.cpp
+++ b/layout/svg/SVGObserverUtils.cpp
@@ -615,16 +615,23 @@ GetEffectProperty(URLAndReferrerInfo* aU
   if (prop)
     return prop;
   prop = new T(aURI, aFrame, false);
   NS_ADDREF(prop);
   aFrame->SetProperty(aProperty, prop);
   return prop;
 }
 
+static nsSVGPaintingProperty*
+GetPaintingProperty(URLAndReferrerInfo* aURI, nsIFrame* aFrame,
+  const mozilla::FramePropertyDescriptor<nsSVGPaintingProperty>* aProperty)
+{
+  return GetEffectProperty(aURI, aFrame, aProperty);
+}
+
 bool
 SVGObserverUtils::GetMarkerFrames(nsIFrame* aMarkedFrame,
                                   nsSVGMarkerFrame*(*aFrames)[3])
 {
   MOZ_ASSERT(!aMarkedFrame->GetPrevContinuation() &&
              aMarkedFrame->IsSVGGeometryFrame() &&
              static_cast<SVGGeometryElement*>(aMarkedFrame->GetContent())->IsMarkable(),
              "Bad frame");
@@ -748,18 +755,18 @@ GetOrCreateClipPathObserver(nsIFrame* aC
   MOZ_ASSERT(!aClippedFrame->GetPrevContinuation(), "Require first continuation");
 
   const nsStyleSVGReset* svgStyleReset = aClippedFrame->StyleSVGReset();
   if (svgStyleReset->mClipPath.GetType() != StyleShapeSourceType::URL) {
     return nullptr;
   }
   css::URLValue* url = svgStyleReset->mClipPath.GetURL();
   RefPtr<URLAndReferrerInfo> pathURI = ResolveURLUsingLocalRef(aClippedFrame, url);
-  return SVGObserverUtils::GetPaintingProperty(pathURI, aClippedFrame,
-                                         SVGObserverUtils::ClipPathProperty());
+  return GetPaintingProperty(pathURI, aClippedFrame,
+                             SVGObserverUtils::ClipPathProperty());
 }
 
 SVGObserverUtils::ReferenceState
 SVGObserverUtils::GetAndObserveClipPath(nsIFrame* aClippedFrame,
                                         nsSVGClipPathFrame** aClipPathFrame)
 {
   if (aClipPathFrame) {
     *aClipPathFrame = nullptr;
@@ -938,45 +945,50 @@ SVGObserverUtils::GetTemplateFrame(nsIFr
 }
 
 void
 SVGObserverUtils::RemoveTemplateObserver(nsIFrame* aFrame)
 {
   aFrame->DeleteProperty(SVGObserverUtils::HrefToTemplateProperty());
 }
 
-nsSVGPaintingProperty*
-SVGObserverUtils::GetPaintingProperty(URLAndReferrerInfo* aURI, nsIFrame* aFrame,
-  const mozilla::FramePropertyDescriptor<nsSVGPaintingProperty>* aProperty)
+Element*
+SVGObserverUtils::GetAndObserveBackgroundImage(nsIFrame* aFrame,
+                                               const nsAtom* aHref)
 {
-  return GetEffectProperty(aURI, aFrame, aProperty);
-}
-
-nsSVGPaintingProperty*
-SVGObserverUtils::GetPaintingPropertyForURI(URLAndReferrerInfo* aURI,
-  nsIFrame* aFrame,
-  URIObserverHashtablePropertyDescriptor aProperty)
-{
-  if (!aURI)
-    return nullptr;
-
   SVGObserverUtils::URIObserverHashtable *hashtable =
-    aFrame->GetProperty(aProperty);
+    aFrame->GetProperty(BackgroundImageProperty());
   if (!hashtable) {
     hashtable = new SVGObserverUtils::URIObserverHashtable();
-    aFrame->SetProperty(aProperty, hashtable);
+    aFrame->SetProperty(BackgroundImageProperty(), hashtable);
   }
-  nsSVGPaintingProperty* prop =
-    static_cast<nsSVGPaintingProperty*>(hashtable->GetWeak(aURI));
-  if (!prop) {
-    bool watchImage = aProperty == SVGObserverUtils::BackgroundImageProperty();
-    prop = new nsSVGPaintingProperty(aURI, aFrame, watchImage);
-    hashtable->Put(aURI, prop);
+
+  nsAutoString elementId =
+    NS_LITERAL_STRING("#") + nsDependentAtomString(aHref);
+  nsCOMPtr<nsIURI> targetURI;
+  nsCOMPtr<nsIURI> base = aFrame->GetContent()->GetBaseURI();
+  nsContentUtils::NewURIWithDocumentCharset(
+    getter_AddRefs(targetURI),
+    elementId,
+    aFrame->GetContent()->GetUncomposedDoc(),
+    base);
+  RefPtr<URLAndReferrerInfo> url = new URLAndReferrerInfo(
+    targetURI,
+    aFrame->GetContent()->OwnerDoc()->GetDocumentURI(),
+    aFrame->GetContent()->OwnerDoc()->GetReferrerPolicy());
+
+  // XXXjwatt: this is broken - we're using the address of a new
+  // URLAndReferrerInfo as the hash key every time!
+  nsSVGPaintingProperty* observer =
+    static_cast<nsSVGPaintingProperty*>(hashtable->GetWeak(url));
+  if (!observer) {
+    observer = new nsSVGPaintingProperty(url, aFrame, /* aWatchImage */ true);
+    hashtable->Put(url, observer);
   }
-  return prop;
+  return observer->GetAndObserveReferencedElement();
 }
 
 nsSVGPaintServerFrame *
 SVGObserverUtils::GetPaintServer(nsIFrame* aTargetFrame,
                                  nsStyleSVGPaint nsStyleSVG::* aPaint)
 {
   // If we're looking at a frame within SVG text, then we need to look up
   // to find the right frame to get the painting property off.  We should at
@@ -997,17 +1009,17 @@ SVGObserverUtils::GetPaintServer(nsIFram
 
   RefPtr<URLAndReferrerInfo> paintServerURL =
     SVGObserverUtils::GetPaintURI(frame, aPaint);
   MOZ_ASSERT(aPaint == &nsStyleSVG::mFill || aPaint == &nsStyleSVG::mStroke);
   PaintingPropertyDescriptor propDesc = (aPaint == &nsStyleSVG::mFill) ?
                                         SVGObserverUtils::FillProperty() :
                                         SVGObserverUtils::StrokeProperty();
   nsSVGPaintingProperty *property =
-    SVGObserverUtils::GetPaintingProperty(paintServerURL, frame, propDesc);
+    GetPaintingProperty(paintServerURL, frame, propDesc);
   if (!property)
     return nullptr;
   nsIFrame* result = property->GetAndObserveReferencedFrame();
   if (!result)
     return nullptr;
 
   LayoutFrameType type = result->Type();
   if (type != LayoutFrameType::SVGLinearGradient &&
--- a/layout/svg/SVGObserverUtils.h
+++ b/layout/svg/SVGObserverUtils.h
@@ -800,30 +800,19 @@ public:
    * used as a callback to lazily get the href value, if necessary.
    */
   static nsIFrame*
   GetTemplateFrame(nsIFrame* aFrame, HrefToTemplateCallback aGetHref);
 
   static void
   RemoveTemplateObserver(nsIFrame* aFrame);
 
-  /**
-   * Get an nsSVGPaintingProperty for the frame, creating a fresh one if necessary
-   */
-  static nsSVGPaintingProperty*
-  GetPaintingProperty(URLAndReferrerInfo* aURI, nsIFrame* aFrame,
-      const mozilla::FramePropertyDescriptor<nsSVGPaintingProperty>* aProperty);
-  /**
-   * Get an nsSVGPaintingProperty for the frame for that URI, creating a fresh
-   * one if necessary
-   */
-  static nsSVGPaintingProperty*
-  GetPaintingPropertyForURI(URLAndReferrerInfo* aURI,
-                            nsIFrame* aFrame,
-                            URIObserverHashtablePropertyDescriptor aProp);
+  static Element*
+  GetAndObserveBackgroundImage(nsIFrame* aFrame,
+                               const nsAtom* aHref);
 
   /**
    * A helper function to resolve marker's URL.
    */
   static already_AddRefed<URLAndReferrerInfo>
   GetMarkerURI(nsIFrame* aFrame,
                RefPtr<mozilla::css::URLValue> nsStyleSVG::* aMarker);