Bug 1325297. Create a variant of imgIContainer::StartDecoding that returns if the current image frame is complete. r=aosmond
authorTimothy Nikkel <tnikkel@gmail.com>
Fri, 23 Dec 2016 01:07:45 -0600
changeset 327186 3382306ba5c5903ca6f72f2dc6f3a9b7097ede49
parent 327185 c82b1188b47a42cbd3c349050e1f366adf53f68b
child 327187 95f052c8d95aba668a4d1caca16674f374fa1e59
push id31122
push userphilringnalda@gmail.com
push dateSun, 25 Dec 2016 00:51:37 +0000
treeherdermozilla-central@1156db49e976 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaosmond
bugs1325297
milestone53.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 1325297. Create a variant of imgIContainer::StartDecoding that returns if the current image frame is complete. r=aosmond During painting we do some image decoding, but we want to send the image progress notifications from that decoding async. The CSS image renderer checks if the image is complete before painting it. So if the decoding we did during painting resulted in the images becoming complete there is no way to tell that during the same paint. Thus making that decoding a waste of time. So we add a limited way of telling if the result of a StartDecoding call has resulting in an image that is ready to paint so we can get that result during the same paint. I would have prefered to change StartDecoding to just return a bool but that would have made the bool an outparam, which would make every StartDecoding call uglier with extra code. Changing it to a notxpcom function would have fixed that, but I'm not sure if that is safe.
image/DynamicImage.cpp
image/ImageWrapper.cpp
image/RasterImage.cpp
image/RasterImage.h
image/VectorImage.cpp
image/imgIContainer.idl
image/imgIRequest.idl
image/imgRequestProxy.cpp
layout/style/nsStyleStruct.cpp
layout/style/nsStyleStruct.h
--- a/image/DynamicImage.cpp
+++ b/image/DynamicImage.cpp
@@ -246,16 +246,22 @@ DynamicImage::Draw(gfxContext* aContext,
 }
 
 NS_IMETHODIMP
 DynamicImage::StartDecoding(uint32_t aFlags)
 {
   return NS_OK;
 }
 
+bool
+DynamicImage::StartDecodingWithResult(uint32_t aFlags)
+{
+  return true;
+}
+
 NS_IMETHODIMP
 DynamicImage::RequestDecodeForSize(const nsIntSize& aSize, uint32_t aFlags)
 {
   return NS_OK;
 }
 
 NS_IMETHODIMP
 DynamicImage::LockImage()
--- a/image/ImageWrapper.cpp
+++ b/image/ImageWrapper.cpp
@@ -216,16 +216,22 @@ ImageWrapper::Draw(gfxContext* aContext,
 }
 
 NS_IMETHODIMP
 ImageWrapper::StartDecoding(uint32_t aFlags)
 {
   return mInnerImage->StartDecoding(aFlags);
 }
 
+bool
+ImageWrapper::StartDecodingWithResult(uint32_t aFlags)
+{
+  return mInnerImage->StartDecodingWithResult(aFlags);
+}
+
 NS_IMETHODIMP
 ImageWrapper::RequestDecodeForSize(const nsIntSize& aSize, uint32_t aFlags)
 {
   return mInnerImage->RequestDecodeForSize(aSize, aFlags);
 }
 
 NS_IMETHODIMP
 ImageWrapper::LockImage()
--- a/image/RasterImage.cpp
+++ b/image/RasterImage.cpp
@@ -1044,45 +1044,74 @@ RasterImage::StartDecoding(uint32_t aFla
     mWantFullDecode = true;
     return NS_OK;
   }
 
   uint32_t flags = (aFlags & FLAG_ASYNC_NOTIFY) | FLAG_SYNC_DECODE_IF_FAST;
   return RequestDecodeForSize(mSize, flags);
 }
 
+bool
+RasterImage::StartDecodingWithResult(uint32_t aFlags)
+{
+  if (mError) {
+    return false;
+  }
+
+  if (!mHasSize) {
+    mWantFullDecode = true;
+    return false;
+  }
+
+  uint32_t flags = (aFlags & FLAG_ASYNC_NOTIFY) | FLAG_SYNC_DECODE_IF_FAST;
+  DrawableSurface surface = RequestDecodeForSizeInternal(mSize, flags);
+  return surface && surface->IsFinished();
+}
+
 NS_IMETHODIMP
 RasterImage::RequestDecodeForSize(const IntSize& aSize, uint32_t aFlags)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   if (mError) {
     return NS_ERROR_FAILURE;
   }
 
+  RequestDecodeForSizeInternal(aSize, aFlags);
+
+  return NS_OK;
+}
+
+DrawableSurface
+RasterImage::RequestDecodeForSizeInternal(const IntSize& aSize, uint32_t aFlags)
+{
+  MOZ_ASSERT(NS_IsMainThread());
+
+  if (mError) {
+    return DrawableSurface();
+  }
+
   if (!mHasSize) {
     mWantFullDecode = true;
-    return NS_OK;
+    return DrawableSurface();
   }
 
   // Decide whether to sync decode images we can decode quickly. Here we are
   // explicitly trading off flashing for responsiveness in the case that we're
   // redecoding an image (see bug 845147).
   bool shouldSyncDecodeIfFast =
     !mHasBeenDecoded && (aFlags & FLAG_SYNC_DECODE_IF_FAST);
 
   uint32_t flags = shouldSyncDecodeIfFast
                  ? aFlags
                  : aFlags & ~FLAG_SYNC_DECODE_IF_FAST;
 
   // Perform a frame lookup, which will implicitly start decoding if needed.
-  LookupFrame(aSize, flags, mAnimationState ? PlaybackType::eAnimated
-                                            : PlaybackType::eStatic);
-
-  return NS_OK;
+  return LookupFrame(aSize, flags, mAnimationState ? PlaybackType::eAnimated
+                                                   : PlaybackType::eStatic);
 }
 
 static bool
 LaunchDecodingTask(IDecodingTask* aTask,
                    RasterImage* aImage,
                    uint32_t aFlags,
                    bool aHaveSourceData)
 {
--- a/image/RasterImage.h
+++ b/image/RasterImage.h
@@ -475,16 +475,18 @@ private: // data
     RefPtr<RasterImage> mImage;
   };
 
   // Helpers
   bool CanDiscard();
 
   bool IsOpaque();
 
+  DrawableSurface RequestDecodeForSizeInternal(const gfx::IntSize& aSize, uint32_t aFlags);
+
 protected:
   explicit RasterImage(ImageURL* aURI = nullptr);
 
   bool ShouldAnimate() override;
 
   friend class ImageFactory;
 };
 
--- a/image/VectorImage.cpp
+++ b/image/VectorImage.cpp
@@ -1027,16 +1027,23 @@ VectorImage::RecoverFromLossOfSurfaces()
 
 NS_IMETHODIMP
 VectorImage::StartDecoding(uint32_t aFlags)
 {
   // Nothing to do for SVG images
   return NS_OK;
 }
 
+bool
+VectorImage::StartDecodingWithResult(uint32_t aFlags)
+{
+  // SVG images are ready to draw when they are loaded
+  return mIsFullyLoaded;
+}
+
 NS_IMETHODIMP
 VectorImage::RequestDecodeForSize(const nsIntSize& aSize, uint32_t aFlags)
 {
   // Nothing to do for SVG images, though in theory we could rasterize to the
   // provided size ahead of time if we supported off-main-thread SVG
   // rasterization...
   return NS_OK;
 }
--- a/image/imgIContainer.idl
+++ b/image/imgIContainer.idl
@@ -416,16 +416,25 @@ interface imgIContainer : nsISupports
    * synchronously.
    *
    * @param aFlags Flags of the FLAG_* variety. Only FLAG_ASYNC_NOTIFY
    *               is accepted; all others are ignored.
    */
   [noscript] void startDecoding(in uint32_t aFlags);
 
   /*
+   * Exactly like startDecoding above except returns whether the current frame
+   * of the image is complete or not.
+   *
+   * @param aFlags Flags of the FLAG_* variety. Only FLAG_ASYNC_NOTIFY
+   *               is accepted; all others are ignored.
+   */
+  [noscript, notxpcom] boolean startDecodingWithResult(in uint32_t aFlags);
+
+  /*
    * This method triggers decoding for an image, but unlike startDecoding() it
    * enables the caller to provide more detailed information about the decode
    * request.
    *
    * @param aSize The size to which the image should be scaled while decoding,
    *              if possible. If the image cannot be scaled to this size while
    *              being decoded, it will be decoded at its intrinsic size.
    * @param aFlags Flags of the FLAG_* variety. Only the decode flags
--- a/image/imgIRequest.idl
+++ b/image/imgIRequest.idl
@@ -151,16 +151,22 @@ interface imgIRequest : nsIRequest
    * a decode before the container has necessarily been instantiated. Calling
    * startDecoding() on the imgIRequest simply forwards along the request if the
    * container already exists, or calls it once the container becomes available
    * if it does not yet exist.
    */
   void startDecoding(in uint32_t aFlags);
 
   /**
+   * Exactly like startDecoding above except returns whether the current frame
+   * of the image is complete or not.
+   */
+  [noscript, notxpcom] boolean startDecodingWithResult(in uint32_t aFlags);
+
+  /**
    * Locks an image. If the image does not exist yet, locks it once it becomes
    * available. The lock persists for the lifetime of the imgIRequest (until
    * unlockImage is called) even if the underlying image changes.
    *
    * If you don't call unlockImage() by the time this imgIRequest goes away, it
    * will be called for you automatically.
    *
    * @see imgIContainer::lockImage for documentation of the underlying call.
--- a/image/imgRequestProxy.cpp
+++ b/image/imgRequestProxy.cpp
@@ -370,16 +370,34 @@ imgRequestProxy::StartDecoding(uint32_t 
 
   if (GetOwner()) {
     GetOwner()->StartDecoding();
   }
 
   return NS_OK;
 }
 
+bool
+imgRequestProxy::StartDecodingWithResult(uint32_t aFlags)
+{
+  // Flag this, so we know to transfer the request if our owner changes
+  mDecodeRequested = true;
+
+  RefPtr<Image> image = GetImage();
+  if (image) {
+    return image->StartDecodingWithResult(aFlags);
+  }
+
+  if (GetOwner()) {
+    GetOwner()->StartDecoding();
+  }
+
+  return false;
+}
+
 NS_IMETHODIMP
 imgRequestProxy::LockImage()
 {
   mLockCount++;
   RefPtr<Image> image = GetImage();
   if (image) {
     return image->LockImage();
   }
--- a/layout/style/nsStyleStruct.cpp
+++ b/layout/style/nsStyleStruct.cpp
@@ -2252,27 +2252,28 @@ nsStyleImage::ComputeActualCropRect(nsIn
   aActualCropRect.IntersectRect(imageRect, cropRect);
 
   if (aIsEntireImage) {
     *aIsEntireImage = aActualCropRect.IsEqualInterior(imageRect);
   }
   return true;
 }
 
-nsresult
+bool
 nsStyleImage::StartDecoding() const
 {
   if (mType == eStyleImageType_Image) {
     imgRequestProxy* req = GetImageData();
     if (!req) {
-      return NS_ERROR_FAILURE;
+      return false;
     }
-    return req->StartDecoding(imgIContainer::FLAG_NONE);
+    return req->StartDecodingWithResult(imgIContainer::FLAG_NONE);
   }
-  return NS_OK;
+  // null image types always return false from IsComplete, so we do the same here.
+  return mType != eStyleImageType_Null ? true : false;
 }
 
 bool
 nsStyleImage::IsOpaque() const
 {
   if (!IsComplete()) {
     return false;
   }
--- a/layout/style/nsStyleStruct.h
+++ b/layout/style/nsStyleStruct.h
@@ -477,19 +477,22 @@ struct nsStyleImage
    * @param aIsEntireImage true iff |aActualCropRect| is identical to the
    * source image bounds.
    * @return true iff |aActualCropRect| holds a meaningful value.
    */
   bool ComputeActualCropRect(nsIntRect& aActualCropRect,
                                bool* aIsEntireImage = nullptr) const;
 
   /**
-   * Starts the decoding of a image.
+   * Starts the decoding of a image. Returns true if the current frame of the
+   * image is complete. The return value is intended to be used instead of
+   * IsComplete because IsComplete may not be up to date if notifications
+   * from decoding are pending because they are being sent async.
    */
-  nsresult StartDecoding() const;
+  bool StartDecoding() const;
   /**
    * @return true if the item is definitely opaque --- i.e., paints every
    * pixel within its bounds opaquely, and the bounds contains at least a pixel.
    */
   bool IsOpaque() const;
   /**
    * @return true if this image is fully loaded, and its size is calculated;
    * always returns true if |mType| is |eStyleImageType_Gradient| or