Bug 1631365. img.decode never fulfills or rejects if the image is too…

… big to fit into the surface cache. r=aosmond

Since we don't support downscaling animated images we'll need something like this even we were to try to request a smaller sized decode.

Differential Revision: https://phabricator.services.mozilla.com/D71523

dom/base/nsImageLoadingContent.cpp

@@ -440,9 +440,16 @@ void nsImageLoadingContent::MaybeResolveDecodePromises() {
   // before LOAD_COMPLETE because we want to start as soon as possible.
   uint32_t flags = imgIContainer::FLAG_HIGH_QUALITY_SCALING |
                    imgIContainer::FLAG_AVOID_REDECODE_FOR_SIZE;
-  if (!mCurrentRequest->RequestDecodeWithResult(flags)) {
+  imgIContainer::DecodeResult decodeResult =
+      mCurrentRequest->RequestDecodeWithResult(flags);
+  if (decodeResult == imgIContainer::DECODE_REQUESTED) {
     return;
   }
+  if (decodeResult == imgIContainer::DECODE_REQUEST_FAILED) {
+    RejectDecodePromises(NS_ERROR_DOM_IMAGE_BROKEN);
+    return;
+  }
+  MOZ_ASSERT(decodeResult == imgIContainer::DECODE_SURFACE_AVAILABLE);
 
   // We can only fulfill the promises once we have all the data.
   if (!(status & imgIRequest::STATUS_LOAD_COMPLETE)) {

image/DynamicImage.cpp

@@ -231,9 +231,9 @@ bool DynamicImage::StartDecodingWithResult(uint32_t aFlags,
   return true;
 }
 
-bool DynamicImage::RequestDecodeWithResult(uint32_t aFlags,
-                                           uint32_t aWhichFrame) {
-  return true;
+imgIContainer::DecodeResult DynamicImage::RequestDecodeWithResult(
+    uint32_t aFlags, uint32_t aWhichFrame) {
+  return imgIContainer::DECODE_SURFACE_AVAILABLE;
 }
 
 NS_IMETHODIMP

image/FrozenImage.cpp

@@ -102,8 +102,8 @@ bool FrozenImage::StartDecodingWithResult(uint32_t aFlags,
   return InnerImage()->StartDecodingWithResult(aFlags, FRAME_FIRST);
 }
 
-bool FrozenImage::RequestDecodeWithResult(uint32_t aFlags,
-                                          uint32_t aWhichFrame) {
+imgIContainer::DecodeResult FrozenImage::RequestDecodeWithResult(
+    uint32_t aFlags, uint32_t aWhichFrame) {
   return InnerImage()->RequestDecodeWithResult(aFlags, FRAME_FIRST);
 }
 

image/FrozenImage.h

@@ -62,7 +62,7 @@ class FrozenImage : public ImageWrapper {
   NS_IMETHOD StartDecoding(uint32_t aFlags, uint32_t aWhichFrame) override;
   NS_IMETHOD_(bool)
   StartDecodingWithResult(uint32_t aFlags, uint32_t aWhichFrame) override;
-  NS_IMETHOD_(bool)
+  NS_IMETHOD_(DecodeResult)
   RequestDecodeWithResult(uint32_t aFlags, uint32_t aWhichFrame) override;
   NS_IMETHOD RequestDecodeForSize(const nsIntSize& aSize, uint32_t aFlags,
                                   uint32_t aWhichFrame) override;

image/ImageWrapper.cpp

@@ -213,8 +213,8 @@ bool ImageWrapper::StartDecodingWithResult(uint32_t aFlags,
   return mInnerImage->StartDecodingWithResult(aFlags, aWhichFrame);
 }
 
-bool ImageWrapper::RequestDecodeWithResult(uint32_t aFlags,
-                                           uint32_t aWhichFrame) {
+imgIContainer::DecodeResult ImageWrapper::RequestDecodeWithResult(
+    uint32_t aFlags, uint32_t aWhichFrame) {
   return mInnerImage->RequestDecodeWithResult(aFlags, aWhichFrame);
 }
 

image/LookupResult.h

@@ -42,7 +42,8 @@ enum class MatchType : uint8_t {
  */
 class MOZ_STACK_CLASS LookupResult {
  public:
-  explicit LookupResult(MatchType aMatchType) : mMatchType(aMatchType) {
+  explicit LookupResult(MatchType aMatchType)
+      : mMatchType(aMatchType), mFailedToRequestDecode(false) {
     MOZ_ASSERT(
         mMatchType == MatchType::NOT_FOUND || mMatchType == MatchType::PENDING,
         "Only NOT_FOUND or PENDING make sense with no surface");
@@ -51,10 +52,13 @@ class MOZ_STACK_CLASS LookupResult {
   LookupResult(LookupResult&& aOther)
       : mSurface(std::move(aOther.mSurface)),
         mMatchType(aOther.mMatchType),
-        mSuggestedSize(aOther.mSuggestedSize) {}
+        mSuggestedSize(aOther.mSuggestedSize),
+        mFailedToRequestDecode(aOther.mFailedToRequestDecode) {}
 
   LookupResult(DrawableSurface&& aSurface, MatchType aMatchType)
-      : mSurface(std::move(aSurface)), mMatchType(aMatchType) {
+      : mSurface(std::move(aSurface)),
+        mMatchType(aMatchType),
+        mFailedToRequestDecode(false) {
     MOZ_ASSERT(!mSurface || !(mMatchType == MatchType::NOT_FOUND ||
                               mMatchType == MatchType::PENDING),
                "Only NOT_FOUND or PENDING make sense with no surface");
@@ -64,7 +68,9 @@ class MOZ_STACK_CLASS LookupResult {
   }
 
   LookupResult(MatchType aMatchType, const gfx::IntSize& aSuggestedSize)
-      : mMatchType(aMatchType), mSuggestedSize(aSuggestedSize) {
+      : mMatchType(aMatchType),
+        mSuggestedSize(aSuggestedSize),
+        mFailedToRequestDecode(false) {
     MOZ_ASSERT(
         mMatchType == MatchType::NOT_FOUND || mMatchType == MatchType::PENDING,
         "Only NOT_FOUND or PENDING make sense with no surface");
@@ -74,7 +80,8 @@ class MOZ_STACK_CLASS LookupResult {
                const gfx::IntSize& aSuggestedSize)
       : mSurface(std::move(aSurface)),
         mMatchType(aMatchType),
-        mSuggestedSize(aSuggestedSize) {
+        mSuggestedSize(aSuggestedSize),
+        mFailedToRequestDecode(false) {
     MOZ_ASSERT(!mSurface || !(mMatchType == MatchType::NOT_FOUND ||
                               mMatchType == MatchType::PENDING),
                "Only NOT_FOUND or PENDING make sense with no surface");
@@ -88,6 +95,7 @@ class MOZ_STACK_CLASS LookupResult {
     mSurface = std::move(aOther.mSurface);
     mMatchType = aOther.mMatchType;
     mSuggestedSize = aOther.mSuggestedSize;
+    mFailedToRequestDecode = aOther.mFailedToRequestDecode;
     return *this;
   }
 
@@ -101,6 +109,9 @@ class MOZ_STACK_CLASS LookupResult {
   /// @return what kind of match this is (exact, substitute, etc.)
   MatchType Type() const { return mMatchType; }
 
+  void SetFailedToRequestDecode() { mFailedToRequestDecode = true; }
+  bool GetFailedToRequestDecode() { return mFailedToRequestDecode; }
+
  private:
   LookupResult(const LookupResult&) = delete;
   LookupResult& operator=(const LookupResult& aOther) = delete;
@@ -114,6 +125,10 @@ class MOZ_STACK_CLASS LookupResult {
   /// all other results. If non-empty, it will always be the size the caller
   /// should request any decodes at.
   gfx::IntSize mSuggestedSize;
+
+  // True if we tried to start a decode but failed, likely because the image was
+  // too big to fit into the surface cache.
+  bool mFailedToRequestDecode;
 };
 
 }  // namespace image

image/RasterImage.cpp

@@ -371,7 +371,11 @@ LookupResult RasterImage::LookupFrame(const UnorientedIntSize& aSize,
           UnorientedIntSize::FromUnknownSize(result.SuggestedSize());
     }
 
-    bool ranSync = Decode(requestedSize, aFlags, aPlaybackType);
+    bool ranSync = false, failed = false;
+    Decode(requestedSize, aFlags, aPlaybackType, ranSync, failed);
+    if (failed) {
+      result.SetFailedToRequestDecode();
+    }
 
     // If we can or did sync decode, we should already have the frame.
     if (ranSync || syncDecode) {
@@ -1089,23 +1093,31 @@ bool RasterImage::StartDecodingWithResult(uint32_t aFlags,
 
   uint32_t flags = (aFlags & FLAG_ASYNC_NOTIFY) | FLAG_SYNC_DECODE_IF_FAST |
                    FLAG_HIGH_QUALITY_SCALING;
-  DrawableSurface surface =
+  LookupResult result =
       RequestDecodeForSizeInternal(ToUnoriented(mSize), flags, aWhichFrame);
+  DrawableSurface surface = std::move(result.Surface());
   return surface && surface->IsFinished();
 }
 
-bool RasterImage::RequestDecodeWithResult(uint32_t aFlags,
-                                          uint32_t aWhichFrame) {
+imgIContainer::DecodeResult RasterImage::RequestDecodeWithResult(
+    uint32_t aFlags, uint32_t aWhichFrame) {
   MOZ_ASSERT(NS_IsMainThread());
 
   if (mError) {
-    return false;
+    return imgIContainer::DECODE_REQUEST_FAILED;
   }
 
   uint32_t flags = aFlags | FLAG_ASYNC_NOTIFY;
-  DrawableSurface surface =
+  LookupResult result =
       RequestDecodeForSizeInternal(ToUnoriented(mSize), flags, aWhichFrame);
-  return surface && surface->IsFinished();
+  DrawableSurface surface = std::move(result.Surface());
+  if (surface && surface->IsFinished()) {
+    return imgIContainer::DECODE_SURFACE_AVAILABLE;
+  }
+  if (result.GetFailedToRequestDecode()) {
+    return imgIContainer::DECODE_REQUEST_FAILED;
+  }
+  return imgIContainer::DECODE_REQUESTED;
 }
 
 NS_IMETHODIMP
@@ -1124,21 +1136,23 @@ RasterImage::RequestDecodeForSize(const IntSize& aSize, uint32_t aFlags,
   return NS_OK;
 }
 
-DrawableSurface RasterImage::RequestDecodeForSizeInternal(
+LookupResult RasterImage::RequestDecodeForSizeInternal(
     const UnorientedIntSize& aSize, uint32_t aFlags, uint32_t aWhichFrame) {
   MOZ_ASSERT(NS_IsMainThread());
 
   if (aWhichFrame > FRAME_MAX_VALUE) {
-    return DrawableSurface();
+    return LookupResult(MatchType::NOT_FOUND);
   }
 
   if (mError) {
-    return DrawableSurface();
+    LookupResult result = LookupResult(MatchType::NOT_FOUND);
+    result.SetFailedToRequestDecode();
+    return result;
   }
 
   if (!mHasSize) {
     mWantFullDecode = true;
-    return DrawableSurface();
+    return LookupResult(MatchType::NOT_FOUND);
   }
 
   // Decide whether to sync decode images we can decode quickly. Here we are
@@ -1151,9 +1165,8 @@ DrawableSurface RasterImage::RequestDecodeForSizeInternal(
       shouldSyncDecodeIfFast ? aFlags : aFlags & ~FLAG_SYNC_DECODE_IF_FAST;
 
   // Perform a frame lookup, which will implicitly start decoding if needed.
-  LookupResult result = LookupFrame(aSize, flags, ToPlaybackType(aWhichFrame),
-                                    /* aMarkUsed = */ false);
-  return std::move(result.Surface());
+  return LookupFrame(aSize, flags, ToPlaybackType(aWhichFrame),
+                     /* aMarkUsed = */ false);
 }
 
 static bool LaunchDecodingTask(IDecodingTask* aTask, RasterImage* aImage,
@@ -1178,18 +1191,20 @@ static bool LaunchDecodingTask(IDecodingTask* aTask, RasterImage* aImage,
   return false;
 }
 
-bool RasterImage::Decode(const UnorientedIntSize& aSize, uint32_t aFlags,
-                         PlaybackType aPlaybackType) {
+void RasterImage::Decode(const UnorientedIntSize& aSize, uint32_t aFlags,
+                         PlaybackType aPlaybackType, bool& aOutRanSync,
+                         bool& aOutFailed) {
   MOZ_ASSERT(NS_IsMainThread());
 
   if (mError) {
-    return false;
+    aOutFailed = true;
+    return;
   }
 
   // If we don't have a size yet, we can't do any other decoding.
   if (!mHasSize) {
     mWantFullDecode = true;
-    return false;
+    return;
   }
 
   // We're about to decode again, which may mean that some of the previous sizes
@@ -1248,7 +1263,8 @@ bool RasterImage::Decode(const UnorientedIntSize& aSize, uint32_t aFlags,
     // managed to insert the new decoder. Pretend we did a sync call to make
     // the caller lookup in the surface cache again.
     MOZ_ASSERT(!task);
-    return true;
+    aOutRanSync = true;
+    return;
   }
 
   if (animated) {
@@ -1266,14 +1282,15 @@ bool RasterImage::Decode(const UnorientedIntSize& aSize, uint32_t aFlags,
   // Make sure DecoderFactory was able to create a decoder successfully.
   if (NS_FAILED(rv)) {
     MOZ_ASSERT(!task);
-    return false;
+    aOutFailed = true;
+    return;
   }
 
   MOZ_ASSERT(task);
   mDecodeCount++;
 
   // We're ready to decode; start the decoder.
-  return LaunchDecodingTask(task, this, aFlags, mAllSourceData);
+  aOutRanSync = LaunchDecodingTask(task, this, aFlags, mAllSourceData);
 }
 
 NS_IMETHODIMP
@@ -1314,17 +1331,19 @@ void RasterImage::RecoverFromInvalidFrames(const UnorientedIntSize& aSize,
     SurfaceCache::LockImage(ImageKey(this));
   }
 
+  bool unused1, unused2;
+
   // Animated images require some special handling, because we normally require
   // that they never be discarded.
   if (mAnimationState) {
     Decode(ToUnoriented(mSize), aFlags | FLAG_SYNC_DECODE,
-           PlaybackType::eAnimated);
+           PlaybackType::eAnimated, unused1, unused2);
     ResetAnimation();
     return;
   }
 
   // For non-animated images, it's fine to recover using an async decode.
-  Decode(aSize, aFlags, PlaybackType::eStatic);
+  Decode(aSize, aFlags, PlaybackType::eStatic, unused1, unused2);
 }
 
 static bool HaveSkia() {

image/RasterImage.h

@@ -337,10 +337,11 @@ class RasterImage final : public ImageResource,
    * It's an error to call Decode() before this image's intrinsic size is
    * available. A metadata decode must successfully complete first.
    *
-   * Returns true of the decode was run synchronously.
+   * aOutRanSync is set to true if the decode was run synchronously.
+   * aOutFailed is set to true if failed to start a decode.
    */
-  bool Decode(const UnorientedIntSize& aSize, uint32_t aFlags,
-              PlaybackType aPlaybackType);
+  void Decode(const UnorientedIntSize& aSize, uint32_t aFlags,
+              PlaybackType aPlaybackType, bool& aOutRanSync, bool& aOutFailed);
 
   /**
    * Creates and runs a metadata decoder, either synchronously or
@@ -528,9 +529,9 @@ class RasterImage final : public ImageResource,
 
   bool IsOpaque();
 
-  DrawableSurface RequestDecodeForSizeInternal(const UnorientedIntSize& aSize,
-                                               uint32_t aFlags,
-                                               uint32_t aWhichFrame);
+  LookupResult RequestDecodeForSizeInternal(const UnorientedIntSize& aSize,
+                                            uint32_t aFlags,
+                                            uint32_t aWhichFrame);
 
  protected:
   explicit RasterImage(nsIURI* aURI = nullptr);

image/VectorImage.cpp

@@ -1218,10 +1218,19 @@ bool VectorImage::StartDecodingWithResult(uint32_t aFlags,
   return mIsFullyLoaded;
 }
 
-bool VectorImage::RequestDecodeWithResult(uint32_t aFlags,
-                                          uint32_t aWhichFrame) {
-  // SVG images are ready to draw when they are loaded
-  return mIsFullyLoaded;
+imgIContainer::DecodeResult VectorImage::RequestDecodeWithResult(
+    uint32_t aFlags, uint32_t aWhichFrame) {
+  // SVG images are ready to draw when they are loaded and don't have an error.
+
+  if (mError) {
+    return imgIContainer::DECODE_REQUEST_FAILED;
+  }
+
+  if (!mIsFullyLoaded) {
+    return imgIContainer::DECODE_REQUESTED;
+  }
+
+  return imgIContainer::DECODE_SURFACE_AVAILABLE;
 }
 
 NS_IMETHODIMP

image/imgIContainer.idl

@@ -523,13 +523,22 @@ interface imgIContainer : nsISupports
    *
    * @param aFlags Flags of the FLAG_* variety.
    * @param aWhichFrame Frame specifier of the FRAME_* variety.
-   * @return True there is a surface that satisfies the request and it is
-   *         fully decoded, else false.
-   */
-  [noscript, notxpcom] boolean requestDecodeWithResult(in uint32_t aFlags, in uint32_t aWhichFrame);
+   * @return DECODE_SURFACE_AVAILABLE if is a surface that satisfies the
+   *         request and it is fully decoded.
+   *         DECODE_REQUESTED if we requested a decode.
+   *         DECODE_REQUEST_FAILED if we failed to request a decode. This means
+   *         that either there is an error in the image or we cannot allocate a
+   *         surface that big.
+   */
+   cenum DecodeResult : 8 {
+     DECODE_SURFACE_AVAILABLE = 0,
+     DECODE_REQUESTED = 1,
+     DECODE_REQUEST_FAILED = 2
+   };
+  [noscript, notxpcom] imgIContainer_DecodeResult requestDecodeWithResult(in uint32_t aFlags, in uint32_t aWhichFrame);
 
 %{C++
-  bool RequestDecodeWithResult(uint32_t aFlags) {
+  DecodeResult RequestDecodeWithResult(uint32_t aFlags) {
     return RequestDecodeWithResult(aFlags, FRAME_CURRENT);
   }
 %}