Bug 1163856 (Part 1) - Delay the image load event until the size decoder gets finalized. r=tn

image/Image.h

@@ -165,6 +165,9 @@ public:
    * 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;
@@ -172,6 +175,7 @@ public:
   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) {}

image/ImageFactory.cpp

@@ -159,7 +159,7 @@ ImageFactory::CreateAnonymousImage(const nsCString& aMimeType)
   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();

image/ProgressTracker.cpp

@@ -43,11 +43,11 @@ CheckProgressConsistency(Progress aProgress)
     // 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);

image/RasterImage.cpp

@@ -266,9 +266,11 @@ RasterImage::RasterImage(ImageURL* aURI /* = nullptr */) :
   mHasSize(false),
   mDecodeOnlyOnDraw(false),
   mTransient(false),
+  mSyncLoad(false),
   mDiscardable(false),
   mHasSourceData(false),
   mHasBeenDecoded(false),
+  mDownscaleDuringDecode(false),
   mPendingAnimation(false),
   mAnimationFinished(false),
   mWantFullDecode(false)
@@ -320,6 +322,7 @@ RasterImage::Init(const char* aMimeType,
   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.
@@ -332,10 +335,19 @@ RasterImage::Init(const char* aMimeType,
     SurfaceCache::LockImage(ImageKey(this));
   }
 
-  // Create the initial size decoder.
-  nsresult rv = Decode(Nothing(), DECODE_FLAGS_DEFAULT);
-  if (NS_FAILED(rv)) {
-    return NS_ERROR_FAILURE;
+  if (mSyncLoad) {
+    // We'll create a sync size decoder in OnImageDataComplete. For now, just
+    // verify that we *could* create such a decoder.
+    eDecoderType type = GetDecoderType(mSourceDataMimeType.get());
+    if (type == eDecoderType_unknown) {
+      return NS_ERROR_FAILURE;
+    }
+  } else {
+    // Create an async size decoder and verify that we succeed in doing so.
+    nsresult rv = Decode(Nothing(), DECODE_FLAGS_DEFAULT);
+    if (NS_FAILED(rv)) {
+      return NS_ERROR_FAILURE;
+    }
   }
 
   // Mark us as initialized
@@ -1179,10 +1191,10 @@ RasterImage::OnImageDataComplete(nsIRequest*, nsISupports*, nsresult aStatus,
   // 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);
   }
 
@@ -1198,28 +1210,47 @@ RasterImage::OnImageDataComplete(nsIRequest*, nsISupports*, nsresult aStatus,
     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
@@ -2170,6 +2201,13 @@ RasterImage::FinalizeDecoder(Decoder* aDecoder)
     } 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()) {

image/RasterImage.h

@@ -244,6 +244,7 @@ public:
                                        nsresult aStatus,
                                        bool aLastPart) override;
 
+  void NotifyForLoadEvent(Progress aProgress);
   void NotifyForDecodeOnlyOnDraw();
 
   /**
@@ -360,6 +361,9 @@ 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.
@@ -407,6 +411,7 @@ private: // data
   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?

image/test/unit/async_load_tests.js

@@ -108,20 +108,6 @@ function firstLoadDone(oldlistener, aRequest)
   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)
@@ -150,7 +136,7 @@ function checkSecondChannelLoad()
   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)
@@ -179,7 +165,7 @@ function run_loadImageWithChannel_tests()
   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)