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
☠☠ backed out by 644043c3de95 ☠ ☠
authorJoe Drew <joe@drew.ca>
Fri, 15 Feb 2013 11:48:17 -0500
changeset 123735 0222cb04802822e4899cb1998b47de3063354322
parent 123734 37f8e72f5124d7bf6e0481a6913dbbeb709522a0
child 123736 d21d144e3db95d2bd24dc583430a04c01428770d
push id24021
push userjdrew@mozilla.com
push dateMon, 04 Mar 2013 20:44:24 +0000
treeherdermozilla-inbound@6d2565566ad2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskhuey
bugs841579
milestone22.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 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
content/base/src/nsImageLoadingContent.cpp
content/base/src/nsImageLoadingContent.h
image/public/imgIRequest.idl
image/src/imgStatusTracker.cpp
--- a/content/base/src/nsImageLoadingContent.cpp
+++ b/content/base/src/nsImageLoadingContent.cpp
@@ -78,16 +78,17 @@ nsImageLoadingContent::nsImageLoadingCon
     mImageBlockingStatus(nsIContentPolicy::ACCEPT),
     mLoadingEnabled(true),
     mIsImageStateForced(false),
     mLoading(false),
     // mBroken starts out true, since an image without a URI is broken....
     mBroken(true),
     mUserDisabled(false),
     mSuppressed(false),
+    mFireEventsOnDecode(false),
     mNewRequestsWillNeedAnimationReset(false),
     mStateChangerDepth(0),
     mCurrentRequestRegistered(false),
     mPendingRequestRegistered(false),
     mVisibleCount(0)
 {
   if (!nsContentUtils::GetImgLoaderForChannel(nullptr)) {
     mLoadingEnabled = false;
@@ -156,16 +157,30 @@ nsImageLoadingContent::Notify(imgIReques
   if (aType == imgINotificationObserver::LOAD_COMPLETE) {
     uint32_t reqStatus;
     aRequest->GetImageStatus(&reqStatus);
     nsresult status =
         reqStatus & imgIRequest::STATUS_ERROR ? NS_ERROR_FAILURE : NS_OK;
     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;
 }
 
 nsresult
 nsImageLoadingContent::OnStopRequest(imgIRequest* aRequest,
                                      nsresult aStatus)
 {
   uint32_t oldStatus;
@@ -208,29 +223,44 @@ nsImageLoadingContent::OnStopRequest(img
   // (*) IsPaintingSuppressed returns false if we haven't gotten the initial
   // reflow yet, so we have to test !DidInitialize || IsPaintingSuppressed.
   // It's possible for painting to be suppressed for reasons other than the
   // initial paint delay (for example, being in the bfcache), but we probably
   // aren't loading images in those situations.
 
   // 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;
+    }
   }
 
-  // Fire the appropriate DOM event.
-  if (NS_SUCCEEDED(aStatus)) {
-    FireEvent(NS_LITERAL_STRING("load"));
+  // 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 {
-    FireEvent(NS_LITERAL_STRING("error"));
+    // 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());
 
   return NS_OK;
 }
 
--- a/content/base/src/nsImageLoadingContent.h
+++ b/content/base/src/nsImageLoadingContent.h
@@ -392,16 +392,17 @@ private:
   /**
    * The state we had the last time we checked whether we needed to notify the
    * document of a state change.  These are maintained by UpdateImageState.
    */
   bool mLoading : 1;
   bool mBroken : 1;
   bool mUserDisabled : 1;
   bool mSuppressed : 1;
+  bool mFireEventsOnDecode : 1;
 
 protected:
   /**
    * A hack to get animations to reset, see bug 594771. On requests
    * that originate from setting .src, we mark them for needing their animation
    * reset when they are ready. mNewRequestsWillNeedAnimationReset is set to
    * true while preparing such requests (as a hack around needing to change an
    * interface), and the other two booleans store which of the current
--- a/image/public/imgIRequest.idl
+++ b/image/public/imgIRequest.idl
@@ -50,29 +50,33 @@ interface imgIRequest : nsIRequest
    * go away or become a private state flag within imgRequest.
    * Don't rely on it.
    *
    * STATUS_LOAD_COMPLETE: The data has been fully loaded
    * to memory, but not necessarily fully decoded.
    *
    * 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.
    *
    * STATUS_DECODE_COMPLETE: The whole image has been decoded.
    */
   //@{
   const long STATUS_NONE             = 0x0;
   const long STATUS_SIZE_AVAILABLE   = 0x1;
   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;
   //@}
 
   /**
    * Status flags of the STATUS_* variety.
    */
   readonly attribute unsigned long imageStatus;
 
   /**
--- a/image/src/imgStatusTracker.cpp
+++ b/image/src/imgStatusTracker.cpp
@@ -471,23 +471,25 @@ imgStatusTracker::RecordLoaded()
 }
 
 void
 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
 imgStatusTracker::RecordStartDecode()
 {
   NS_ABORT_IF_FALSE(mImage, "RecordStartDecode without an Image");
   mState |= stateDecodeStarted;
+  mImageStatus |= imgIRequest::STATUS_DECODE_STARTED;
 }
 
 void
 imgStatusTracker::SendStartDecode(imgRequestProxy* aProxy)
 {
   if (!aProxy->NotificationsDeferred())
     aProxy->OnStartDecode();
 }
@@ -545,21 +547,23 @@ imgStatusTracker::SendStopFrame(imgReque
 
 void
 imgStatusTracker::RecordStopDecode(nsresult aStatus)
 {
   NS_ABORT_IF_FALSE(mImage,
                     "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
 imgStatusTracker::SendStopDecode(imgRequestProxy* aProxy,
                                  nsresult aStatus)
 {
   if (!aProxy->NotificationsDeferred())
     aProxy->OnStopDecode();
@@ -570,18 +574,19 @@ imgStatusTracker::RecordDiscard()
 {
   NS_ABORT_IF_FALSE(mImage,
                     "RecordDiscard called before we have an Image");
   // Clear the state bits we no longer deserve.
   uint32_t stateBitsToClear = stateDecodeStopped;
   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;
 }
 
 void
 imgStatusTracker::RecordUnlockedDraw()
 {
   NS_ABORT_IF_FALSE(mImage,
                     "RecordUnlockedDraw called before we have an Image");
@@ -640,16 +645,18 @@ imgStatusTracker::SendFrameChanged(imgRe
 void
 imgStatusTracker::RecordStartRequest()
 {
   // We're starting a new load, so clear any status and state bits indicating
   // load/decode
   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;
   mState &= ~stateRequestStopped;
   mState &= ~stateBlockingOnload;
 
   mState |= stateRequestStarted;
 }