Bug 704059 - Part 1: Decouple OnStopRequest and OnStopDecode for VectorImage. r=joe, r=dholbert

image/src/SVGDocumentWrapper.cpp

@@ -251,24 +251,7 @@ SVGDocumentWrapper::OnStopRequest(nsIRequest* aRequest, nsISupports* ctxt,
 {
   if (mListener) {
     mListener->OnStopRequest(aRequest, ctxt, status);
-    // A few levels up the stack, imgRequest::OnStopRequest is about to tell
-    // all of its observers that we know our size and are ready to paint.  That
-    // might not be true at this point, though -- so here, we synchronously
-    // finish parsing & layout in our helper-document to make sure we can hold
-    // up to this promise.
-    nsCOMPtr<nsIParser> parser = do_QueryInterface(mListener);
-    while (!parser->IsComplete()) {
-      parser->CancelParsingEvents();
-      parser->ContinueInterruptedParsing();
-    }
-    // XXX flushing is wasteful if embedding frame hasn't had initial reflow
-    FlushLayout();
     mListener = nullptr;
-
-    // In a normal document, this would be called by nsDocShell - but we don't
-    // have a nsDocShell. So we do it ourselves. (If we don't, painting will
-    // stay suppressed for a little while longer, for no good reason).
-    mViewer->LoadComplete(NS_OK);
   }
 
   return NS_OK;
@@ -418,6 +401,15 @@ SVGDocumentWrapper::FlushLayout()
   }
 }
 
+nsIDocument*
+SVGDocumentWrapper::GetDocument()
+{
+  if (!mViewer)
+    return nullptr;
+
+  return mViewer->GetDocument(); // May be nullptr.
+}
+
 SVGSVGElement*
 SVGDocumentWrapper::GetRootSVGElem()
 {

image/src/SVGDocumentWrapper.h

@@ -67,6 +67,11 @@ public:
    */
   bool      GetWidthOrHeight(Dimension aDimension, int32_t& aResult);
 
+  /**
+   * Returns the wrapped document, or nullptr on failure. (No AddRef.)
+   */
+  nsIDocument* GetDocument();
+
   /**
    * Returns the root <svg> element for the wrapped document, or nullptr on
    * failure.
@@ -91,15 +96,6 @@ public:
   inline nsresult  GetPresShell(nsIPresShell** aPresShell)
     { return mViewer->GetPresShell(aPresShell); }
 
-  /**
-   * Returns a bool indicating whether the wrapped document has been parsed
-   * successfully.
-   *
-   * @return true if the document has been parsed successfully,
-   *         false otherwise (e.g. if there's a syntax error in the SVG).
-   */
-  inline bool      ParsedSuccessfully()  { return !!GetRootSVGElem(); }
-
   /**
    * Modifier to update the viewport dimensions of the wrapped document. This
    * method performs a synchronous "Flush_Layout" on the wrapped document,
@@ -140,6 +136,11 @@ public:
   void StopAnimation();
   void ResetAnimation();
 
+  /**
+   * Force a layout flush of the underlying SVG document.
+   */
+  void FlushLayout();
+
 private:
   nsresult SetupViewer(nsIRequest *aRequest,
                        nsIContentViewer** aViewer,
@@ -148,8 +149,6 @@ private:
   void     RegisterForXPCOMShutdown();
   void     UnregisterForXPCOMShutdown();
 
-  void     FlushLayout();
-
   nsCOMPtr<nsIContentViewer>  mViewer;
   nsCOMPtr<nsILoadGroup>      mLoadGroup;
   nsCOMPtr<nsIStreamListener> mListener;

image/src/VectorImage.cpp

@@ -95,6 +95,123 @@ protected:
   VectorImage* mVectorImage;   // Raw pointer because it owns me.
 };
 
+class SVGParseCompleteListener MOZ_FINAL : public nsStubDocumentObserver {
+public:
+  NS_DECL_ISUPPORTS
+
+  SVGParseCompleteListener(nsIDocument* aDocument,
+                           VectorImage* aImage)
+    : mDocument(aDocument)
+    , mImage(aImage)
+  {
+    NS_ABORT_IF_FALSE(mDocument, "Need an SVG document");
+    NS_ABORT_IF_FALSE(mImage, "Need an image");
+
+    mDocument->AddObserver(this);
+  }
+
+  ~SVGParseCompleteListener()
+  { 
+    if (mDocument) {
+      // The document must have been destroyed before we got our event.
+      // Otherwise this can't happen, since documents hold strong references to
+      // their observers.
+      Cancel();
+    }
+  }
+
+  void EndLoad(nsIDocument* aDocument) MOZ_OVERRIDE
+  {
+    NS_ABORT_IF_FALSE(aDocument == mDocument, "Got EndLoad for wrong document?");
+
+    // OnSVGDocumentParsed will release our owner's reference to us, so ensure
+    // we stick around long enough to complete our work.
+    nsRefPtr<SVGParseCompleteListener> kungFuDeathGroup(this);
+
+    mImage->OnSVGDocumentParsed();
+  }
+
+  void Cancel()
+  {
+    NS_ABORT_IF_FALSE(mDocument, "Duplicate call to Cancel");
+    mDocument->RemoveObserver(this);
+    mDocument = nullptr;
+  }
+
+private:
+  nsCOMPtr<nsIDocument> mDocument;
+  VectorImage* mImage; // Raw pointer to owner.
+};
+
+NS_IMPL_ISUPPORTS1(SVGParseCompleteListener, nsIDocumentObserver)
+
+class SVGLoadEventListener MOZ_FINAL : public nsIDOMEventListener {
+public:
+  NS_DECL_ISUPPORTS
+
+  SVGLoadEventListener(nsIDocument* aDocument,
+                       VectorImage* aImage)
+    : mDocument(aDocument)
+    , mImage(aImage)
+  {
+    NS_ABORT_IF_FALSE(mDocument, "Need an SVG document");
+    NS_ABORT_IF_FALSE(mImage, "Need an image");
+
+    mDocument->AddEventListener(NS_LITERAL_STRING("MozSVGAsImageDocumentLoad"), this, true, false);
+    mDocument->AddEventListener(NS_LITERAL_STRING("SVGAbort"), this, true, false);
+    mDocument->AddEventListener(NS_LITERAL_STRING("SVGError"), this, true, false);
+  }
+
+  ~SVGLoadEventListener()
+  {
+    if (mDocument) {
+      // The document must have been destroyed before we got our event.
+      // Otherwise this can't happen, since documents hold strong references to
+      // their observers.
+      Cancel();
+    }
+  }
+
+  NS_IMETHOD HandleEvent(nsIDOMEvent* aEvent) MOZ_OVERRIDE
+  {
+    NS_ABORT_IF_FALSE(mDocument, "Need an SVG document. Received multiple events?");
+
+    // OnSVGDocumentLoaded/OnSVGDocumentError will release our owner's reference
+    // to us, so ensure we stick around long enough to complete our work.
+    nsRefPtr<SVGLoadEventListener> kungFuDeathGroup(this);
+
+    nsAutoString eventType;
+    aEvent->GetType(eventType);
+    NS_ABORT_IF_FALSE(eventType.EqualsLiteral("MozSVGAsImageDocumentLoad")  ||
+                      eventType.EqualsLiteral("SVGAbort")                   ||
+                      eventType.EqualsLiteral("SVGError"),
+                      "Received unexpected event");
+
+    if (eventType.EqualsLiteral("MozSVGAsImageDocumentLoad")) {
+      mImage->OnSVGDocumentLoaded();
+    } else {
+      mImage->OnSVGDocumentError();
+    }
+
+    return NS_OK;
+  }
+
+  void Cancel()
+  {
+    NS_ABORT_IF_FALSE(mDocument, "Duplicate call to Cancel");
+    mDocument->RemoveEventListener(NS_LITERAL_STRING("MozSVGAsImageDocumentLoad"), this, true);
+    mDocument->RemoveEventListener(NS_LITERAL_STRING("SVGAbort"), this, true);
+    mDocument->RemoveEventListener(NS_LITERAL_STRING("SVGError"), this, true);
+    mDocument = nullptr;
+  }
+
+private:
+  nsCOMPtr<nsIDocument> mDocument;
+  VectorImage* mImage; // Raw pointer to owner.
+};
+
+NS_IMPL_ISUPPORTS1(SVGLoadEventListener, nsIDOMEventListener)
+
 // Helper-class: SVGDrawingCallback
 class SVGDrawingCallback : public gfxDrawingCallback {
 public:
@@ -673,9 +790,27 @@ VectorImage::OnStartRequest(nsIRequest* aRequest, nsISupports* aCtxt)
   if (NS_FAILED(rv)) {
     mSVGDocumentWrapper = nullptr;
     mError = true;
+    return rv;
   }
 
-  return rv;
+  // Sending StartDecode will block page load until the document's ready.  (We
+  // unblock it by sending StopDecode in OnSVGDocumentLoaded or
+  // OnSVGDocumentError.)
+  if (mStatusTracker) {
+    mStatusTracker->GetDecoderObserver()->OnStartDecode();
+  }
+
+  // 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();
+  mLoadEventListener = new SVGLoadEventListener(document, this);
+  mParseCompleteListener = new SVGParseCompleteListener(document, this);
+
+  return NS_OK;
 }
 
 //******************************************************************************
@@ -688,38 +823,86 @@ VectorImage::OnStopRequest(nsIRequest* aRequest, nsISupports* aCtxt,
   if (mError)
     return NS_ERROR_FAILURE;
 
-  NS_ABORT_IF_FALSE(!mIsFullyLoaded && !mHaveAnimations,
-                    "these flags shouldn't get set until OnStopRequest. "
-                    "Duplicate calls to OnStopRequest?");
+  return mSVGDocumentWrapper->OnStopRequest(aRequest, aCtxt, aStatus);
+}
 
-  nsresult rv = mSVGDocumentWrapper->OnStopRequest(aRequest, aCtxt, aStatus);
-  if (!mSVGDocumentWrapper->ParsedSuccessfully()) {
-    // XXXdholbert Need to do something more here -- right now, this just
-    // makes us draw the "object" icon, rather than the (jagged) "broken image"
-    // icon.  See bug 594505.
-    mError = true;
-    return rv;
+void
+VectorImage::OnSVGDocumentParsed()
+{
+  NS_ABORT_IF_FALSE(mParseCompleteListener, "Should have the parse complete listener");
+  NS_ABORT_IF_FALSE(mLoadEventListener, "Should have the load event listener");
+
+  if (!mSVGDocumentWrapper->GetRootSVGElem()) {
+    // This is an invalid SVG document. It may have failed to parse, or it may
+    // be missing the <svg> root element, or the <svg> root element may not
+    // declare the correct namespace. In any of these cases, we'll never be
+    // notified that the SVG finished loading, so we need to treat this as an error.
+    OnSVGDocumentError();
   }
+}
+
+void
+VectorImage::CancelAllListeners()
+{
+  NS_ABORT_IF_FALSE(mParseCompleteListener, "Should have the parse complete listener");
+  NS_ABORT_IF_FALSE(mLoadEventListener, "Should have the load event listener");
+
+  if (mParseCompleteListener) {
+    mParseCompleteListener->Cancel();
+    mParseCompleteListener = nullptr;
+  }
+  if (mLoadEventListener) {
+    mLoadEventListener->Cancel();
+    mLoadEventListener = nullptr;
+  }
+}
+
+void
+VectorImage::OnSVGDocumentLoaded()
+{
+  NS_ABORT_IF_FALSE(mSVGDocumentWrapper->GetRootSVGElem(), "Should have parsed successfully");
+  NS_ABORT_IF_FALSE(!mIsFullyLoaded && !mHaveAnimations,
+                    "These flags shouldn't get set until OnSVGDocumentLoaded. "
+                    "Duplicate calls to OnSVGDocumentLoaded?");
+
+  CancelAllListeners();
+
+  // XXX Flushing is wasteful if embedding frame hasn't had initial reflow.
+  mSVGDocumentWrapper->FlushLayout();
 
   mIsFullyLoaded = true;
   mHaveAnimations = mSVGDocumentWrapper->IsAnimated();
 
-  // Start listening to our image for rendering updates
+  // Start listening to our image for rendering updates.
   mRenderingObserver = new SVGRootRenderingObserver(mSVGDocumentWrapper, this);
 
-  // Tell *our* observers that we're done loading
+  // Tell *our* observers that we're done loading.
   if (mStatusTracker) {
-    // NOTE: This signals that width/height are available.
     imgDecoderObserver* observer = mStatusTracker->GetDecoderObserver();
-    observer->OnStartContainer();
 
+    observer->OnStartContainer(); // Signal that width/height are available.
     observer->FrameChanged(&nsIntRect::GetMaxSizedIntRect());
     observer->OnStopFrame();
-    observer->OnStopDecode(NS_OK);
+    observer->OnStopDecode(NS_OK); // Unblock page load.
   }
-  EvaluateAnimation();
 
-  return rv;
+  EvaluateAnimation();
+}
+
+void
+VectorImage::OnSVGDocumentError()
+{
+  CancelAllListeners();
+
+  // XXXdholbert Need to do something more for the parsing failed case -- right
+  // now, this just makes us draw the "object" icon, rather than the (jagged)
+  // "broken image" icon.  See bug 594505.
+  mError = true;
+
+  if (mStatusTracker) {
+    // Unblock page load.
+    mStatusTracker->GetDecoderObserver()->OnStopDecode(NS_ERROR_FAILURE);
+  }
 }
 
 //------------------------------------------------------------------------------

image/src/VectorImage.h

@@ -23,6 +23,8 @@ namespace image {
 
 class SVGDocumentWrapper;
 class SVGRootRenderingObserver;
+class SVGLoadEventListener;
+class SVGParseCompleteListener;
 
 class VectorImage : public ImageResource,
                     public nsIStreamListener
@@ -56,9 +58,16 @@ public:
                                        nsresult status) MOZ_OVERRIDE;
   virtual nsresult OnNewSourceData() MOZ_OVERRIDE;
 
-  // Callback for SVGRootRenderingObserver
+  // Callback for SVGRootRenderingObserver.
   void InvalidateObserver();
 
+  // Callback for SVGParseCompleteListener.
+  void OnSVGDocumentParsed();
+
+  // Callbacks for SVGLoadEventListener.
+  void OnSVGDocumentLoaded();
+  void OnSVGDocumentError();
+
 protected:
   VectorImage(imgStatusTracker* aStatusTracker = nullptr, nsIURI* aURI = nullptr);
 
@@ -67,8 +76,12 @@ protected:
   virtual bool     ShouldAnimate();
 
 private:
+  void CancelAllListeners();
+
   nsRefPtr<SVGDocumentWrapper>       mSVGDocumentWrapper;
   nsRefPtr<SVGRootRenderingObserver> mRenderingObserver;
+  nsRefPtr<SVGLoadEventListener>     mLoadEventListener;
+  nsRefPtr<SVGParseCompleteListener> mParseCompleteListener;
 
   nsIntRect      mRestrictedRegion;       // If we were created by
                                           // ExtractFrame, this is the region
@@ -76,7 +89,7 @@ private:
                                           // Otherwise, this is ignored.
 
   bool           mIsInitialized:1;        // Have we been initalized?
-  bool           mIsFullyLoaded:1;        // Has OnStopRequest been called?
+  bool           mIsFullyLoaded:1;        // Has the SVG document finished loading?
   bool           mIsDrawing:1;            // Are we currently drawing?
   bool           mHaveAnimations:1;       // Is our SVG content SMIL-animated?
                                           // (Only set after mIsFullyLoaded.)