Bug 1163856 (Part 1) - Delay the image load event until the size decoder gets finalized. r=tn
☠☠ backed out by cf2d7e67dab0 ☠ ☠
authorSeth Fowler <mark.seth.fowler@gmail.com>
Thu, 25 Jun 2015 17:09:57 -0700
changeset 250291 04239448fe0b82467db334a876ca4d59df921c60
parent 250290 531f100d2bd8861ee165e3e585f08c325ad0948f
child 250292 62c1c616f21c656a1000a655d889b96fb7195f34
push id28951
push usercbook@mozilla.com
push dateFri, 26 Jun 2015 11:19:38 +0000
treeherdermozilla-central@56e207dbb3bd [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstn
bugs1163856
milestone41.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 1163856 (Part 1) - Delay the image load event until the size decoder gets finalized. r=tn
image/Image.h
image/ImageFactory.cpp
image/ProgressTracker.cpp
image/RasterImage.cpp
image/RasterImage.h
image/test/unit/async_load_tests.js
--- a/image/Image.h
+++ b/image/Image.h
@@ -160,23 +160,27 @@ public:
    * before being destroyed. (For example, containers for
    * multipart/x-mixed-replace image parts fall into this category.) If this
    * flag is set, INIT_FLAG_DISCARDABLE and INIT_FLAG_DECODE_ONLY_ON_DRAW must
    * not be set.
    *
    * INIT_FLAG_DOWNSCALE_DURING_DECODE: The container should attempt to
    * downscale images during decoding instead of decoding them to their
    * intrinsic size.
+   *
+   * INIT_FLAG_SYNC_LOAD: The container is being loaded synchronously, so
+   * it should avoid relying on async workers to get the container ready.
    */
   static const uint32_t INIT_FLAG_NONE                     = 0x0;
   static const uint32_t INIT_FLAG_DISCARDABLE              = 0x1;
   static const uint32_t INIT_FLAG_DECODE_ONLY_ON_DRAW      = 0x2;
   static const uint32_t INIT_FLAG_DECODE_IMMEDIATELY       = 0x4;
   static const uint32_t INIT_FLAG_TRANSIENT                = 0x8;
   static const uint32_t INIT_FLAG_DOWNSCALE_DURING_DECODE  = 0x10;
+  static const uint32_t INIT_FLAG_SYNC_LOAD                = 0x20;
 
   virtual already_AddRefed<ProgressTracker> GetProgressTracker() = 0;
   virtual void SetProgressTracker(ProgressTracker* aProgressTracker) {}
 
   /**
    * The size, in bytes, occupied by the compressed source data of the image.
    * If MallocSizeOf does not work on this platform, uses a fallback approach to
    * ensure that something reasonable is always returned.
--- a/image/ImageFactory.cpp
+++ b/image/ImageFactory.cpp
@@ -154,17 +154,17 @@ ImageFactory::CreateAnonymousImage(const
   nsresult rv;
 
   nsRefPtr<RasterImage> newImage = new RasterImage();
 
   nsRefPtr<ProgressTracker> newTracker = new ProgressTracker();
   newTracker->SetImage(newImage);
   newImage->SetProgressTracker(newTracker);
 
-  rv = newImage->Init(aMimeType.get(), Image::INIT_FLAG_NONE);
+  rv = newImage->Init(aMimeType.get(), Image::INIT_FLAG_SYNC_LOAD);
   NS_ENSURE_SUCCESS(rv, BadImage(newImage));
 
   return newImage.forget();
 }
 
 /* static */ already_AddRefed<MultipartImage>
 ImageFactory::CreateMultipartImage(Image* aFirstPart,
                                    ProgressTracker* aProgressTracker)
--- a/image/ProgressTracker.cpp
+++ b/image/ProgressTracker.cpp
@@ -38,21 +38,21 @@ CheckProgressConsistency(Progress aProgr
   }
   if (aProgress & FLAG_FRAME_COMPLETE) {
     MOZ_ASSERT(aProgress & FLAG_DECODE_STARTED);
   }
   if (aProgress & FLAG_LOAD_COMPLETE) {
     // No preconditions.
   }
   if (aProgress & FLAG_ONLOAD_BLOCKED) {
-    MOZ_ASSERT(aProgress & FLAG_DECODE_STARTED);
+    // No preconditions.
   }
   if (aProgress & FLAG_ONLOAD_UNBLOCKED) {
     MOZ_ASSERT(aProgress & FLAG_ONLOAD_BLOCKED);
-    MOZ_ASSERT(aProgress & (FLAG_FRAME_COMPLETE | FLAG_HAS_ERROR));
+    MOZ_ASSERT(aProgress & (FLAG_SIZE_AVAILABLE | FLAG_HAS_ERROR));
   }
   if (aProgress & FLAG_IS_ANIMATED) {
     MOZ_ASSERT(aProgress & FLAG_DECODE_STARTED);
     MOZ_ASSERT(aProgress & FLAG_SIZE_AVAILABLE);
   }
   if (aProgress & FLAG_HAS_TRANSPARENCY) {
     MOZ_ASSERT(aProgress & FLAG_SIZE_AVAILABLE);
   }
--- a/image/RasterImage.cpp
+++ b/image/RasterImage.cpp
@@ -261,19 +261,21 @@ RasterImage::RasterImage(ImageURL* aURI 
   mFramesNotified(0),
 #endif
   mSourceBuffer(new SourceBuffer()),
   mFrameCount(0),
   mRetryCount(0),
   mHasSize(false),
   mDecodeOnlyOnDraw(false),
   mTransient(false),
+  mSyncLoad(false),
   mDiscardable(false),
   mHasSourceData(false),
   mHasBeenDecoded(false),
+  mDownscaleDuringDecode(false),
   mPendingAnimation(false),
   mAnimationFinished(false),
   mWantFullDecode(false)
 {
   Telemetry::GetHistogramById(Telemetry::IMAGE_DECODE_COUNT)->Add(0);
 }
 
 //******************************************************************************
@@ -315,32 +317,36 @@ RasterImage::Init(const char* aMimeType,
 
   // Store initialization data
   mSourceDataMimeType.Assign(aMimeType);
   mDiscardable = !!(aFlags & INIT_FLAG_DISCARDABLE);
   mDecodeOnlyOnDraw = !!(aFlags & INIT_FLAG_DECODE_ONLY_ON_DRAW);
   mWantFullDecode = !!(aFlags & INIT_FLAG_DECODE_IMMEDIATELY);
   mTransient = !!(aFlags & INIT_FLAG_TRANSIENT);
   mDownscaleDuringDecode = !!(aFlags & INIT_FLAG_DOWNSCALE_DURING_DECODE);
+  mSyncLoad = !!(aFlags & INIT_FLAG_SYNC_LOAD);
 
 #ifndef MOZ_ENABLE_SKIA
   // Downscale-during-decode requires Skia.
   mDownscaleDuringDecode = false;
 #endif
 
   // Lock this image's surfaces in the SurfaceCache if we're not discardable.
   if (!mDiscardable) {
     mLockCount++;
     SurfaceCache::LockImage(ImageKey(this));
   }
 
-  // Create the initial size decoder.
-  nsresult rv = Decode(Nothing(), DECODE_FLAGS_DEFAULT);
-  if (NS_FAILED(rv)) {
-    return NS_ERROR_FAILURE;
+  // Create an async size decoder if we're not being loaded synchronously.
+  // (If we are, we'll take care of the size decode in OnImageDataComplete.)
+  if (!mSyncLoad) {
+    nsresult rv = Decode(Nothing(), DECODE_FLAGS_DEFAULT);
+    if (NS_FAILED(rv)) {
+      return NS_ERROR_FAILURE;
+    }
   }
 
   // Mark us as initialized
   mInitialized = true;
 
   return NS_OK;
 }
 
@@ -1174,57 +1180,76 @@ RasterImage::OnImageDataComplete(nsIRequ
   MOZ_ASSERT(NS_IsMainThread());
 
   // Record that we have all the data we're going to get now.
   mHasSourceData = true;
 
   // Let decoders know that there won't be any more data coming.
   mSourceBuffer->Complete(aStatus);
 
-  if (!mHasSize) {
-    // We need to guarantee that we've gotten the image's size, or at least
-    // determined that we won't be able to get it, before we deliver the load
-    // event. That means we have to do a synchronous size decode here.
+  if (mSyncLoad && !mHasSize) {
+    // We're loading this image synchronously, so it needs to be usable after
+    // this call returns.  Since we haven't gotten our size yet, we need to do a
+    // synchronous size decode here.
     Decode(Nothing(), FLAG_SYNC_DECODE);
   }
 
   // Determine our final status, giving precedence to Necko failure codes. We
   // check after running the size decode above in case it triggered an error.
   nsresult finalStatus = mError ? NS_ERROR_FAILURE : NS_OK;
   if (NS_FAILED(aStatus)) {
     finalStatus = aStatus;
   }
 
   // If loading failed, report an error.
   if (NS_FAILED(finalStatus)) {
     DoError();
   }
 
+  Progress loadProgress = LoadCompleteProgress(aLastPart, mError, finalStatus);
+
+  if (!mHasSize && !mError) {
+    // We don't have our size yet, so we'll fire the load event in SetSize().
+    MOZ_ASSERT(!mSyncLoad, "Firing load asynchronously but mSyncLoad is set?");
+    NotifyProgress(FLAG_ONLOAD_BLOCKED);
+    mLoadProgress = Some(loadProgress);
+    return finalStatus;
+  }
+
+  NotifyForLoadEvent(loadProgress);
+
+  return finalStatus;
+}
+
+void
+RasterImage::NotifyForLoadEvent(Progress aProgress)
+{
   MOZ_ASSERT(mHasSize || mError, "Need to know size before firing load event");
   MOZ_ASSERT(!mHasSize ||
              (mProgressTracker->GetProgress() & FLAG_SIZE_AVAILABLE),
              "Should have notified that the size is available if we have it");
 
-  Progress loadProgress = LoadCompleteProgress(aLastPart, mError, finalStatus);
-
   if (mDecodeOnlyOnDraw) {
     // For decode-only-on-draw images, we want to send notifications as if we've
     // already finished decoding. Otherwise some observers will never even try
     // to draw. (We may have already sent some of these notifications from
     // NotifyForDecodeOnlyOnDraw(), but ProgressTracker will ensure no duplicate
     // notifications get sent.)
-    loadProgress |= FLAG_DECODE_STARTED |
-                    FLAG_FRAME_COMPLETE |
-                    FLAG_DECODE_COMPLETE;
+    aProgress |= FLAG_DECODE_STARTED |
+                 FLAG_FRAME_COMPLETE |
+                 FLAG_DECODE_COMPLETE;
+  }
+
+  // If we encountered an error, make sure we notify for that as well.
+  if (mError) {
+    aProgress |= FLAG_HAS_ERROR;
   }
 
   // Notify our listeners, which will fire this image's load event.
-  NotifyProgress(loadProgress);
-
-  return finalStatus;
+  NotifyProgress(aProgress);
 }
 
 void
 RasterImage::NotifyForDecodeOnlyOnDraw()
 {
   if (!NS_IsMainThread()) {
     nsCOMPtr<nsIRunnable> runnable =
       NS_NewRunnableMethod(this, &RasterImage::NotifyForDecodeOnlyOnDraw);
@@ -2165,16 +2190,23 @@ RasterImage::FinalizeDecoder(Decoder* aD
     }
 
     // Detect errors.
     if (aDecoder->HasError() && !aDecoder->WasAborted()) {
       DoError();
     } else if (wasSize && !mHasSize) {
       DoError();
     }
+
+    // If we were waiting to fire the load event, go ahead and fire it now.
+    if (mLoadProgress && wasSize) {
+      NotifyForLoadEvent(*mLoadProgress);
+      mLoadProgress = Nothing();
+      NotifyProgress(FLAG_ONLOAD_UNBLOCKED);
+    }
   }
 
   if (aDecoder->ImageIsLocked()) {
     // Unlock the image, balancing the LockImage call we made in CreateDecoder.
     UnlockImage();
   }
 
   // If we were a size decode and a full decode was requested, now's the time.
--- a/image/RasterImage.h
+++ b/image/RasterImage.h
@@ -239,16 +239,17 @@ public:
                                         nsIInputStream* aInStr,
                                         uint64_t aSourceOffset,
                                         uint32_t aCount) override;
   virtual nsresult OnImageDataComplete(nsIRequest* aRequest,
                                        nsISupports* aContext,
                                        nsresult aStatus,
                                        bool aLastPart) override;
 
+  void NotifyForLoadEvent(Progress aProgress);
   void NotifyForDecodeOnlyOnDraw();
 
   /**
    * A hint of the number of bytes of source data that the image contains. If
    * called early on, this can help reduce copying and reallocations by
    * appropriately preallocating the source data buffer.
    *
    * We take this approach rather than having the source data management code do
@@ -355,16 +356,19 @@ private:
    * existing frames and redecodes using the provided @aSize and @aFlags.
    */
   void RecoverFromLossOfFrames(const nsIntSize& aSize, uint32_t aFlags);
 
 private: // data
   nsIntSize                  mSize;
   Orientation                mOrientation;
 
+  /// If this has a value, we're waiting for SetSize() to send the load event.
+  Maybe<Progress>            mLoadProgress;
+
   nsCOMPtr<nsIProperties>   mProperties;
 
   /// If this image is animated, a FrameAnimator which manages its animation.
   UniquePtr<FrameAnimator> mAnim;
 
   // Image locking.
   uint32_t                   mLockCount;
 
@@ -402,16 +406,17 @@ private: // data
 
   // The number of times we've retried decoding this image.
   uint8_t                    mRetryCount;
 
   // Boolean flags (clustered together to conserve space):
   bool                       mHasSize:1;       // Has SetSize() been called?
   bool                       mDecodeOnlyOnDraw:1; // Decoding only on draw?
   bool                       mTransient:1;     // Is the image short-lived?
+  bool                       mSyncLoad:1;      // Are we loading synchronously?
   bool                       mDiscardable:1;   // Is container discardable?
   bool                       mHasSourceData:1; // Do we have source data?
   bool                       mHasBeenDecoded:1; // Decoded at least once?
   bool                       mDownscaleDuringDecode:1;
 
   // Whether we're waiting to start animation. If we get a StartAnimation() call
   // but we don't yet have more than one frame, mPendingAnimation is set so that
   // we know to start animation later if/when we have more frames.
--- a/image/test/unit/async_load_tests.js
+++ b/image/test/unit/async_load_tests.js
@@ -104,30 +104,16 @@ function checkSecondLoad()
 function firstLoadDone(oldlistener, aRequest)
 {
   checkSecondLoad(uri);
 
   do_test_finished();
 }
 
 // Return a closure that allows us to check the stream listener's status when the
-// image starts loading.
-function getChannelLoadImageStartCallback(streamlistener)
-{
-  return function channelLoadStart(imglistener, aRequest) {
-    // We must not have received all status before we get this start callback.
-    // If we have, we've broken people's expectations by delaying events from a
-    // channel we were given.
-    do_check_eq(streamlistener.requestStatus & STOP_REQUEST, 0);
-
-    checkClone(imglistener, aRequest);
-  }
-}
-
-// Return a closure that allows us to check the stream listener's status when the
 // image finishes loading.
 function getChannelLoadImageStopCallback(streamlistener, next)
 {
   return function channelLoadStop(imglistener, aRequest) {
 
     next();
 
     do_test_finished();
@@ -145,17 +131,17 @@ function checkSecondChannelLoad()
                                              null,      // aLoadingNode
                                              Services.scriptSecurityManager.getSystemPrincipal(),
                                              null,      // aTriggeringPrincipal
                                              Ci.nsILoadInfo.SEC_NORMAL,
                                              Ci.nsIContentPolicy.TYPE_OTHER);
   var channellistener = new ChannelListener();
   channel.asyncOpen(channellistener, null);
 
-  var listener = new ImageListener(getChannelLoadImageStartCallback(channellistener),
+  var listener = new ImageListener(null,
                                    getChannelLoadImageStopCallback(channellistener,
                                                                    all_done_callback));
   var outer = Cc["@mozilla.org/image/tools;1"].getService(Ci.imgITools)
                 .createScriptedObserver(listener);
   var outlistener = {};
   requests.push(gCurrentLoader.loadImageWithChannelXPCOM(channel, outer, null, outlistener));
   channellistener.outputListener = outlistener.value;
 
@@ -174,17 +160,17 @@ function run_loadImageWithChannel_tests(
                                              null,      // aLoadingNode
                                              Services.scriptSecurityManager.getSystemPrincipal(),
                                              null,      // aTriggeringPrincipal
                                              Ci.nsILoadInfo.SEC_NORMAL,
                                              Ci.nsIContentPolicy.TYPE_OTHER);
   var channellistener = new ChannelListener();
   channel.asyncOpen(channellistener, null);
 
-  var listener = new ImageListener(getChannelLoadImageStartCallback(channellistener),
+  var listener = new ImageListener(null,
                                    getChannelLoadImageStopCallback(channellistener,
                                                                    checkSecondChannelLoad));
   var outer = Cc["@mozilla.org/image/tools;1"].getService(Ci.imgITools)
                 .createScriptedObserver(listener);
   var outlistener = {};
   requests.push(gCurrentLoader.loadImageWithChannelXPCOM(channel, outer, null, outlistener));
   channellistener.outputListener = outlistener.value;