Bug 1128225 (Part 1) - Return a result enumeration from imgIContainer::Draw. r=tn

dom/canvas/CanvasRenderingContext2D.cpp

@@ -4452,13 +4452,15 @@ CanvasRenderingContext2D::DrawDirectlyToCanvas(
 
   SVGImageContext svgContext(scaledImageSize, Nothing(), CurrentState().globalAlpha);
 
-  nsresult rv = image.mImgContainer->
+  auto result = image.mImgContainer->
     Draw(context, scaledImageSize,
          ImageRegion::Create(gfxRect(src.x, src.y, src.width, src.height)),
          image.mWhichFrame, GraphicsFilter::FILTER_GOOD,
          Some(svgContext), modifiedFlags);
 
-  NS_ENSURE_SUCCESS_VOID(rv);
+  if (result != DrawResult::SUCCESS) {
+    NS_WARNING("imgIContainer::Draw failed");
+  }
 }
 
 void

image/public/imgIContainer.idl

@@ -34,13 +34,60 @@ class SVGImageContext;
 
 namespace mozilla {
 namespace image {
+
 class ImageRegion;
 struct Orientation;
+
+/**
+ * An enumeration representing the result of a drawing operation.
+ *
+ * Most users of DrawResult will only be interested in whether the value is
+ * SUCCESS or not. The other values are primarily useful for debugging and error
+ * handling.
+ *
+ * SUCCESS: We successfully drew a completely decoded frame of the requested
+ * size. Drawing again with FLAG_SYNC_DECODE would not change the result.
+ *
+ * INCOMPLETE: We successfully drew a frame that was partially decoded. (Note
+ * that successfully drawing a partially decoded frame may not actually draw any
+ * pixels!) Drawing again with FLAG_SYNC_DECODE would improve the result.
+ *
+ * WRONG_SIZE: We successfully drew a wrongly-sized frame that had to be scaled.
+ * This is only returned if drawing again with FLAG_SYNC_DECODE would improve
+ * the result; if the size requested was larger than the intrinsic size of the
+ * image, for example, we would generally have to scale whether FLAG_SYNC_DECODE
+ * was specified or not, and therefore we would not return WRONG_SIZE.
+ *
+ * NOT_READY: We failed to draw because no decoded version of the image was
+ * available. Drawing again with FLAG_SYNC_DECODE would improve the result.
+ * (Though FLAG_SYNC_DECODE will not necessarily work until after the image's
+ * load event!)
+ *
+ * TEMPORARY_ERROR: We failed to draw due to a temporary error. Drawing may
+ * succeed at a later time.
+ *
+ * BAD_IMAGE: We failed to draw because the image has an error. This is a
+ * permanent condition.
+ *
+ * BAD_ARGS: We failed to draw because bad arguments were passed to draw().
+ */
+enum class DrawResult : uint8_t
+{
+  SUCCESS,
+  INCOMPLETE,
+  WRONG_SIZE,
+  NOT_READY,
+  TEMPORARY_ERROR,
+  BAD_IMAGE,
+  BAD_ARGS
+};
+
 }
 }
 
 %}
 
+native DrawResult(mozilla::image::DrawResult);
 [ptr] native gfxContext(gfxContext);
 [ref] native gfxMatrix(gfxMatrix);
 [ref] native gfxRect(gfxRect);
@@ -69,7 +116,7 @@ native nsIntSizeByVal(nsIntSize);
  *
  * Internally, imgIContainer also manages animation of images.
  */
-[scriptable, builtinclass, uuid(3a630c1d-3365-4873-bc01-54468decf8c6)]
+[scriptable, builtinclass, uuid(4e7d0b17-cb57-4d68-860f-59b303a86dbd)]
 interface imgIContainer : nsISupports
 {
   /**
@@ -335,8 +382,11 @@ interface imgIContainer : nsISupports
    *                    node, and the size of the viewport that the full image
    *                    would occupy. Ignored for raster images.
    * @param aFlags Flags of the FLAG_* variety
+   * @return A DrawResult value indicating whether and to what degree the
+   *         drawing operation was successful.
    */
-  [noscript] void draw(in gfxContext aContext,
+  [noscript, notxpcom] DrawResult
+  draw(in gfxContext aContext,
        [const] in nsIntSize aSize,
        [const] in ImageRegion aRegion,
        in uint32_t aWhichFrame,

image/src/ClippedImage.cpp

@@ -297,7 +297,7 @@ MustCreateSurface(gfxContext* aContext,
   return willTile || willResample;
 }
 
-NS_IMETHODIMP
+NS_IMETHODIMP_(DrawResult)
 ClippedImage::Draw(gfxContext* aContext,
                    const nsIntSize& aSize,
                    const ImageRegion& aRegion,
@@ -318,7 +318,9 @@ ClippedImage::Draw(gfxContext* aContext,
     // GetFrame will call DrawSingleTile internally.
     RefPtr<SourceSurface> surface =
       GetFrameInternal(aSize, aSVGContext, aWhichFrame, aFlags);
-    NS_ENSURE_TRUE(surface, NS_ERROR_FAILURE);
+    if (!surface) {
+      return DrawResult::TEMPORARY_ERROR;
+    }
 
     // Create a drawable from that surface.
     nsRefPtr<gfxSurfaceDrawable> drawable =
@@ -328,7 +330,7 @@ ClippedImage::Draw(gfxContext* aContext,
     gfxUtils::DrawPixelSnapped(aContext, drawable, aSize, aRegion,
                                SurfaceFormat::B8G8R8A8, aFilter);
 
-    return NS_OK;
+    return DrawResult::SUCCESS;
   }
 
   return DrawSingleTile(aContext, aSize, aRegion, aWhichFrame,
@@ -352,7 +354,7 @@ UnclipViewport(const SVGImageContext& aOldContext,
                          aOldContext.GetPreserveAspectRatio());
 }
 
-nsresult
+DrawResult
 ClippedImage::DrawSingleTile(gfxContext* aContext,
                              const nsIntSize& aSize,
                              const ImageRegion& aRegion,

image/src/ClippedImage.h

@@ -39,7 +39,7 @@ public:
     GetFrame(uint32_t aWhichFrame, uint32_t aFlags) MOZ_OVERRIDE;
   NS_IMETHOD GetImageContainer(layers::LayerManager* aManager,
                                layers::ImageContainer** _retval) MOZ_OVERRIDE;
-  NS_IMETHOD Draw(gfxContext* aContext,
+  NS_IMETHOD_(DrawResult) Draw(gfxContext* aContext,
                                const nsIntSize& aSize,
                                const ImageRegion& aRegion,
                                uint32_t aWhichFrame,
@@ -66,7 +66,7 @@ private:
                      uint32_t aWhichFrame,
                      uint32_t aFlags);
   bool ShouldClip();
-  nsresult DrawSingleTile(gfxContext* aContext,
+  DrawResult DrawSingleTile(gfxContext* aContext,
                             const nsIntSize& aSize,
                             const ImageRegion& aRegion,
                             uint32_t aWhichFrame,

image/src/DynamicImage.cpp

@@ -186,12 +186,11 @@ DynamicImage::GetFrame(uint32_t aWhichFrame,
                                      SurfaceFormat::B8G8R8A8);
   nsRefPtr<gfxContext> context = new gfxContext(dt);
 
-  nsresult rv = Draw(context, size, ImageRegion::Create(size),
+  auto result = Draw(context, size, ImageRegion::Create(size),
                      aWhichFrame, GraphicsFilter::FILTER_NEAREST,
                      Nothing(), aFlags);
 
-  NS_ENSURE_SUCCESS(rv, nullptr);
-  return dt->Snapshot();
+  return result == DrawResult::SUCCESS ? dt->Snapshot() : nullptr;
 }
 
 NS_IMETHODIMP_(bool)
@@ -210,7 +209,7 @@ DynamicImage::GetImageContainer(LayerManager* aManager,
   return NS_OK;
 }
 
-NS_IMETHODIMP
+NS_IMETHODIMP_(DrawResult)
 DynamicImage::Draw(gfxContext* aContext,
                    const nsIntSize& aSize,
                    const ImageRegion& aRegion,
@@ -226,7 +225,7 @@ DynamicImage::Draw(gfxContext* aContext,
   if (aSize == drawableSize) {
     gfxUtils::DrawPixelSnapped(aContext, mDrawable, drawableSize, aRegion,
                                SurfaceFormat::B8G8R8A8, aFilter);
-    return NS_OK;
+    return DrawResult::SUCCESS;
   }
 
   gfxSize scale(double(aSize.width) / drawableSize.width,
@@ -240,7 +239,7 @@ DynamicImage::Draw(gfxContext* aContext,
 
   gfxUtils::DrawPixelSnapped(aContext, mDrawable, drawableSize, region,
                              SurfaceFormat::B8G8R8A8, aFilter);
-  return NS_OK;
+  return DrawResult::SUCCESS;
 }
 
 NS_IMETHODIMP

image/src/FrozenImage.cpp

@@ -58,7 +58,7 @@ FrozenImage::GetImageContainer(layers::LayerManager* aManager,
   return NS_OK;
 }
 
-NS_IMETHODIMP
+NS_IMETHODIMP_(DrawResult)
 FrozenImage::Draw(gfxContext* aContext,
                   const nsIntSize& aSize,
                   const ImageRegion& aRegion,

image/src/FrozenImage.h

@@ -39,7 +39,7 @@ public:
     GetFrame(uint32_t aWhichFrame, uint32_t aFlags) MOZ_OVERRIDE;
   NS_IMETHOD GetImageContainer(layers::LayerManager* aManager,
                                layers::ImageContainer** _retval) MOZ_OVERRIDE;
-  NS_IMETHOD Draw(gfxContext* aContext,
+  NS_IMETHOD_(DrawResult) Draw(gfxContext* aContext,
                                const nsIntSize& aSize,
                                const ImageRegion& aRegion,
                                uint32_t aWhichFrame,

image/src/ImageWrapper.cpp

@@ -199,7 +199,7 @@ ImageWrapper::GetImageContainer(LayerManager* aManager,
   return mInnerImage->GetImageContainer(aManager, _retval);
 }
 
-NS_IMETHODIMP
+NS_IMETHODIMP_(DrawResult)
 ImageWrapper::Draw(gfxContext* aContext,
                    const nsIntSize& aSize,
                    const ImageRegion& aRegion,

image/src/OrientedImage.cpp

@@ -251,7 +251,7 @@ OrientViewport(const SVGImageContext& aOldContext,
                          aOldContext.GetPreserveAspectRatio());
 }
 
-NS_IMETHODIMP
+NS_IMETHODIMP_(DrawResult)
 OrientedImage::Draw(gfxContext* aContext,
                     const nsIntSize& aSize,
                     const ImageRegion& aRegion,

image/src/OrientedImage.h

@@ -36,7 +36,7 @@ public:
     GetFrame(uint32_t aWhichFrame, uint32_t aFlags) MOZ_OVERRIDE;
   NS_IMETHOD GetImageContainer(layers::LayerManager* aManager,
                                layers::ImageContainer** _retval) MOZ_OVERRIDE;
-  NS_IMETHOD Draw(gfxContext* aContext,
+  NS_IMETHOD_(DrawResult) Draw(gfxContext* aContext,
                                const nsIntSize& aSize,
                                const ImageRegion& aRegion,
                                uint32_t aWhichFrame,

image/src/RasterImage.cpp

@@ -1699,7 +1699,7 @@ RasterImage::RequestScale(imgFrame* aFrame,
   }
 }
 
-void
+DrawResult
 RasterImage::DrawWithPreDownscaleIfNeeded(DrawableFrameRef&& aFrameRef,
                                           gfxContext* aContext,
                                           const nsIntSize& aSize,
@@ -1728,23 +1728,39 @@ RasterImage::DrawWithPreDownscaleIfNeeded(DrawableFrameRef&& aFrameRef,
 
   gfxContextMatrixAutoSaveRestore saveMatrix(aContext);
   ImageRegion region(aRegion);
+  bool frameIsComplete = true;  // We already checked HQ scaled frames.
   if (!frameRef) {
+    // There's no HQ scaled frame available, so we'll have to use the frame
+    // provided by the caller.
     frameRef = Move(aFrameRef);
+    frameIsComplete = frameRef->IsImageComplete();
   }
 
   // By now we may have a frame with the requested size. If not, we need to
   // adjust the drawing parameters accordingly.
   IntSize finalSize = frameRef->GetImageSize();
+  bool couldRedecodeForBetterFrame = false;
   if (ThebesIntSize(finalSize) != aSize) {
     gfx::Size scale(double(aSize.width) / finalSize.width,
                     double(aSize.height) / finalSize.height);
     aContext->Multiply(gfxMatrix::Scaling(scale.width, scale.height));
     region.Scale(1.0 / scale.width, 1.0 / scale.height);
+
+    couldRedecodeForBetterFrame = mDownscaleDuringDecode &&
+                                  CanDownscaleDuringDecode(aSize, aFlags);
   }
 
   if (!frameRef->Draw(aContext, region, aFilter, aFlags)) {
     RecoverFromLossOfFrames(aSize, aFlags);
+    return DrawResult::TEMPORARY_ERROR;
   }
+  if (!frameIsComplete) {
+    return DrawResult::INCOMPLETE;
+  }
+  if (couldRedecodeForBetterFrame) {
+    return DrawResult::WRONG_SIZE;
+  }
+  return DrawResult::SUCCESS;
 }
 
 //******************************************************************************
@@ -1757,7 +1773,7 @@ RasterImage::DrawWithPreDownscaleIfNeeded(DrawableFrameRef&& aFrameRef,
  *                      [const] in SVGImageContext aSVGContext,
  *                      in uint32_t aWhichFrame,
  *                      in uint32_t aFlags); */
-NS_IMETHODIMP
+NS_IMETHODIMP_(DrawResult)
 RasterImage::Draw(gfxContext* aContext,
                   const nsIntSize& aSize,
                   const ImageRegion& aRegion,
@@ -1767,18 +1783,20 @@ RasterImage::Draw(gfxContext* aContext,
                   uint32_t aFlags)
 {
   if (aWhichFrame > FRAME_MAX_VALUE)
-    return NS_ERROR_INVALID_ARG;
+    return DrawResult::BAD_ARGS;
 
   if (mError)
-    return NS_ERROR_FAILURE;
+    return DrawResult::BAD_IMAGE;
 
   // Illegal -- you can't draw with non-default decode flags.
   // (Disabling colorspace conversion might make sense to allow, but
   // we don't currently.)
   if (DecodeFlags(aFlags) != DECODE_FLAGS_DEFAULT)
-    return NS_ERROR_FAILURE;
+    return DrawResult::BAD_ARGS;
 
-  NS_ENSURE_ARG_POINTER(aContext);
+  if (!aContext) {
+    return DrawResult::BAD_ARGS;
+  }
 
   if (IsUnlocked() && mProgressTracker) {
     mProgressTracker->OnUnlockedDraw();
@@ -1797,13 +1815,13 @@ RasterImage::Draw(gfxContext* aContext,
     if (mDrawStartTime.IsNull()) {
       mDrawStartTime = TimeStamp::Now();
     }
-    return NS_OK;
+    return DrawResult::NOT_READY;
   }
 
   bool shouldRecordTelemetry = !mDrawStartTime.IsNull() &&
                                ref->IsImageComplete();
 
-  DrawWithPreDownscaleIfNeeded(Move(ref), aContext, aSize,
+  auto result = DrawWithPreDownscaleIfNeeded(Move(ref), aContext, aSize,
                                              aRegion, aFilter, flags);
 
   if (shouldRecordTelemetry) {
@@ -1813,7 +1831,7 @@ RasterImage::Draw(gfxContext* aContext,
       mDrawStartTime = TimeStamp();
   }
 
-  return NS_OK;
+  return result;
 }
 
 //******************************************************************************

image/src/RasterImage.h

@@ -288,7 +288,7 @@ public:
   }
 
 private:
-  void DrawWithPreDownscaleIfNeeded(DrawableFrameRef&& aFrameRef,
+  DrawResult DrawWithPreDownscaleIfNeeded(DrawableFrameRef&& aFrameRef,
                                           gfxContext* aContext,
                                           const nsIntSize& aSize,
                                           const ImageRegion& aRegion,

image/src/VectorImage.cpp

@@ -687,13 +687,12 @@ VectorImage::GetFrame(uint32_t aWhichFrame,
 
   nsRefPtr<gfxContext> context = new gfxContext(dt);
 
-  nsresult rv = Draw(context, imageIntSize,
+  auto result = Draw(context, imageIntSize,
                      ImageRegion::Create(imageIntSize),
                      aWhichFrame, GraphicsFilter::FILTER_NEAREST,
                      Nothing(), aFlags);
 
-  NS_ENSURE_SUCCESS(rv, nullptr);
-  return dt->Snapshot();
+  return result == DrawResult::SUCCESS ? dt->Snapshot() : nullptr;
 }
 
 //******************************************************************************
@@ -747,7 +746,7 @@ struct SVGDrawingParameters
  *                      in gfxGraphicsFilter aFilter,
  *                      [const] in MaybeSVGImageContext aSVGContext,
  *                      in uint32_t aFlags); */
-NS_IMETHODIMP
+NS_IMETHODIMP_(DrawResult)
 VectorImage::Draw(gfxContext* aContext,
                   const nsIntSize& aSize,
                   const ImageRegion& aRegion,
@@ -756,16 +755,25 @@ VectorImage::Draw(gfxContext* aContext,
                   const Maybe<SVGImageContext>& aSVGContext,
                   uint32_t aFlags)
 {
-  if (aWhichFrame > FRAME_MAX_VALUE)
-    return NS_ERROR_INVALID_ARG;
+  if (aWhichFrame > FRAME_MAX_VALUE) {
+    return DrawResult::BAD_ARGS;
+  }
 
-  NS_ENSURE_ARG_POINTER(aContext);
-  if (mError || !mIsFullyLoaded)
-    return NS_ERROR_FAILURE;
+  if (!aContext) {
+    return DrawResult::BAD_ARGS;
+  }
+
+  if (mError) {
+    return DrawResult::BAD_IMAGE;
+  }
+
+  if (!mIsFullyLoaded) {
+    return DrawResult::NOT_READY;
+  }
 
   if (mIsDrawing) {
     NS_WARNING("Refusing to make re-entrant call to VectorImage::Draw");
-    return NS_ERROR_FAILURE;
+    return DrawResult::TEMPORARY_ERROR;
   }
 
   if (mAnimationConsumers == 0 && mProgressTracker) {
@@ -786,7 +794,7 @@ VectorImage::Draw(gfxContext* aContext,
 
   if (aFlags & FLAG_BYPASS_SURFACE_CACHE) {
     CreateSurfaceAndShow(params);
-    return NS_OK;
+    return DrawResult::SUCCESS;
   }
 
   DrawableFrameRef frameRef =
@@ -802,7 +810,7 @@ VectorImage::Draw(gfxContext* aContext,
       nsRefPtr<gfxDrawable> svgDrawable =
         new gfxSurfaceDrawable(surface, ThebesIntSize(frameRef->GetSize()));
       Show(svgDrawable, params);
-      return NS_OK;
+      return DrawResult::SUCCESS;
     }
 
     // We lost our surface due to some catastrophic event.
@@ -811,7 +819,7 @@ VectorImage::Draw(gfxContext* aContext,
 
   CreateSurfaceAndShow(params);
 
-  return NS_OK;
+  return DrawResult::SUCCESS;
 }
 
 void