Bug 841579 - If we start a decode (or a decode is already started) at the time OnStopRequest is fired, wait around for it to finish decoding before firing onload/onerror. r=khuey

--HG--
extra : rebase_source : e4a97b3f037df283e89dbcb21d330030b6e35c85

content/base/src/nsImageLoadingContent.cpp

@@ -83,6 +83,7 @@ nsImageLoadingContent::nsImageLoadingContent()
     mBroken(true),
     mUserDisabled(false),
     mSuppressed(false),
+    mFireEventsOnDecode(false),
     mNewRequestsWillNeedAnimationReset(false),
     mStateChangerDepth(0),
     mCurrentRequestRegistered(false),
@@ -161,6 +162,20 @@ nsImageLoadingContent::Notify(imgIRequest* aRequest,
     return OnStopRequest(aRequest, status);
   }
 
+  if (aType == imgINotificationObserver::DECODE_COMPLETE && mFireEventsOnDecode) {
+    mFireEventsOnDecode = false;
+
+    uint32_t reqStatus;
+    aRequest->GetImageStatus(&reqStatus);
+    if (reqStatus & imgIRequest::STATUS_ERROR) {
+      FireEvent(NS_LITERAL_STRING("error"));
+    } else {
+      FireEvent(NS_LITERAL_STRING("load"));
+    }
+
+    UpdateImageState(true);
+  }
+
   return NS_OK;
 }
 
@@ -213,20 +228,35 @@ nsImageLoadingContent::OnStopRequest(imgIRequest* aRequest,
 
   // XXXkhuey should this be GetOurCurrentDoc?  Decoding if we're not in
   // the document seems silly.
+  bool startedDecoding = false;
   nsIDocument* doc = GetOurOwnerDoc();
   nsIPresShell* shell = doc ? doc->GetShell() : nullptr;
   if (shell && shell->IsVisible() &&
       (!shell->DidInitialize() || shell->IsPaintingSuppressed())) {
 
-    mCurrentRequest->StartDecoding();
+    if (NS_SUCCEEDED(mCurrentRequest->StartDecoding())) {
+      startedDecoding = true;
+    }
   }
 
+  // We want to give the decoder a chance to find errors. If we haven't found
+  // an error yet and we've started decoding, either from the above
+  // StartDecoding or from some other place, we must only fire these events
+  // after we finish decoding.
+  uint32_t reqStatus;
+  aRequest->GetImageStatus(&reqStatus);
+  if (NS_SUCCEEDED(aStatus) && !(reqStatus & imgIRequest::STATUS_ERROR) &&
+      (reqStatus & imgIRequest::STATUS_DECODE_STARTED ||
+       (startedDecoding && !(reqStatus & imgIRequest::STATUS_DECODE_COMPLETE)))) {
+    mFireEventsOnDecode = true;
+  } else {
     // Fire the appropriate DOM event.
     if (NS_SUCCEEDED(aStatus)) {
       FireEvent(NS_LITERAL_STRING("load"));
     } else {
       FireEvent(NS_LITERAL_STRING("error"));
     }
+  }
 
   nsCOMPtr<nsINode> thisNode = do_QueryInterface(static_cast<nsIImageLoadingContent*>(this));
   nsSVGEffects::InvalidateDirectRenderingObservers(thisNode->AsElement());

content/base/src/nsImageLoadingContent.h

@@ -397,6 +397,7 @@ private:
   bool mBroken : 1;
   bool mUserDisabled : 1;
   bool mSuppressed : 1;
+  bool mFireEventsOnDecode : 1;
 
 protected:
   /**

image/public/imgIRequest.idl

@@ -55,6 +55,9 @@ interface imgIRequest : nsIRequest
    *
    * STATUS_ERROR: An error occurred loading the image.
    *
+   * STATUS_DECODE_STARTED: The decoding process has begun, but not yet
+   * finished.
+   *
    * STATUS_FRAME_COMPLETE: The first frame has been
    * completely decoded.
    *
@@ -66,8 +69,9 @@ interface imgIRequest : nsIRequest
   const long STATUS_LOAD_PARTIAL     = 0x2;
   const long STATUS_LOAD_COMPLETE    = 0x4;
   const long STATUS_ERROR            = 0x8;
-  const long STATUS_FRAME_COMPLETE   = 0x10;
-  const long STATUS_DECODE_COMPLETE  = 0x20;
+  const long STATUS_DECODE_STARTED   = 0x10;
+  const long STATUS_FRAME_COMPLETE   = 0x20;
+  const long STATUS_DECODE_COMPLETE  = 0x40;
   //@}
 
   /**

image/src/imgStatusTracker.cpp

@@ -476,6 +476,7 @@ imgStatusTracker::RecordDecoded()
   NS_ABORT_IF_FALSE(mImage, "RecordDecoded called before we have an Image");
   mState |= stateDecodeStarted | stateDecodeStopped | stateFrameStopped;
   mImageStatus |= imgIRequest::STATUS_FRAME_COMPLETE | imgIRequest::STATUS_DECODE_COMPLETE;
+  mImageStatus &= ~imgIRequest::STATUS_DECODE_STARTED;
 }
 
 void
@@ -483,6 +484,7 @@ imgStatusTracker::RecordStartDecode()
 {
   NS_ABORT_IF_FALSE(mImage, "RecordStartDecode without an Image");
   mState |= stateDecodeStarted;
+  mImageStatus |= imgIRequest::STATUS_DECODE_STARTED;
 }
 
 void
@@ -550,11 +552,13 @@ imgStatusTracker::RecordStopDecode(nsresult aStatus)
                     "RecordStopDecode called before we have an Image");
   mState |= stateDecodeStopped;
 
-  if (NS_SUCCEEDED(aStatus) && mImageStatus != imgIRequest::STATUS_ERROR)
+  if (NS_SUCCEEDED(aStatus) && mImageStatus != imgIRequest::STATUS_ERROR) {
     mImageStatus |= imgIRequest::STATUS_DECODE_COMPLETE;
+    mImageStatus &= ~imgIRequest::STATUS_DECODE_STARTED;
   // If we weren't successful, clear all success status bits and set error.
-  else
+  } else {
     mImageStatus = imgIRequest::STATUS_ERROR;
+  }
 }
 
 void
@@ -575,8 +579,9 @@ imgStatusTracker::RecordDiscard()
   mState &= ~stateBitsToClear;
 
   // Clear the status bits we no longer deserve.
-  uint32_t statusBitsToClear = imgIRequest::STATUS_FRAME_COMPLETE
-                               | imgIRequest::STATUS_DECODE_COMPLETE;
+  uint32_t statusBitsToClear = imgIRequest::STATUS_DECODE_STARTED |
+                               imgIRequest::STATUS_FRAME_COMPLETE |
+                               imgIRequest::STATUS_DECODE_COMPLETE;
   mImageStatus &= ~statusBitsToClear;
 }
 
@@ -645,6 +650,8 @@ imgStatusTracker::RecordStartRequest()
   mImageStatus &= ~imgIRequest::STATUS_LOAD_PARTIAL;
   mImageStatus &= ~imgIRequest::STATUS_LOAD_COMPLETE;
   mImageStatus &= ~imgIRequest::STATUS_FRAME_COMPLETE;
+  mImageStatus &= ~imgIRequest::STATUS_DECODE_STARTED;
+  mImageStatus &= ~imgIRequest::STATUS_DECODE_COMPLETE;
   mState &= ~stateRequestStarted;
   mState &= ~stateDecodeStarted;
   mState &= ~stateDecodeStopped;