Fix a SkImage leak in ImageBitmap:cropImage()

third_party/WebKit/Source/core/frame/ImageBitmap.cpp

Index: third_party/WebKit/Source/core/frame/ImageBitmap.cpp
diff --git a/third_party/WebKit/Source/core/frame/ImageBitmap.cpp b/third_party/WebKit/Source/core/frame/ImageBitmap.cpp
index 19d831308e299cff8b57d5825a9bd4400b1ecd03..a982f2fd39a22a73f5fe5dc6cbe998b10ef7ed2c 100644
--- a/third_party/WebKit/Source/core/frame/ImageBitmap.cpp
+++ b/third_party/WebKit/Source/core/frame/ImageBitmap.cpp
@@ -147,13 +147,13 @@ static PassRefPtr<StaticBitmapImage> cropImage(Image* image, const IntRect& crop
     if (cropRect == srcRect) {
         if (flipY)
             return StaticBitmapImage::create(flipSkImageVertically(skiaImage->newSubset(srcRect), premultiplyAlpha ? PremultiplyAlpha : DontPremultiplyAlpha));
-        SkImage* croppedSkImage = skiaImage->newSubset(srcRect);
+        RefPtr<SkImage> croppedSkImage = fromSkSp(skiaImage->makeSubset(srcRect));
         // Special case: The first parameter image is unpremul but we need to turn it into premul.
         if (premultiplyAlpha && imageFormat == DontPremultiplyAlpha)
-            return StaticBitmapImage::create(unPremulSkImageToPremul(croppedSkImage));
+            return StaticBitmapImage::create(unPremulSkImageToPremul(croppedSkImage.get()));
         // Call preroll to trigger image decoding.
         croppedSkImage->preroll();

[f(malita), 2016/06/15 08:41:54]

Unrelated to this CL: I think I understand why we want to preroll before
resolving the promise (http://crbug.com/580202), but I'm a bit concerned about
doing it in this place.  cropImage is called from most ImageBitmap constructors
- isn't this potentially clogging the main thread, when the ImageBitmap is
unrelated to a promise?

It seems a safer place to force the decode would be ImageBitmapLoader, and only
when resolving a promise.  WDYT?

[xidachen, 2016/06/15 08:56:11]

The problem is that ImageBitmapLoader is not always used. For example, when
createImageBitmap from an HTMLImageElement, it doesn't use the
ImageBitmapLoader, but calls the constructor of ImageBitmap(HTMLImageElement)
directly which jumps to cropImage.

Is there a way to check whether the croppedSkImage is always decoded or not? So
that we can call preroll() only in the case that it is not decoded yet?

[xidachen, 2016/06/15 08:57:29]

sorry, I meant is there a way to check whether the croppedSkImage is *already*
decoded or not?

-        return StaticBitmapImage::create(adoptRef(croppedSkImage));
+        return StaticBitmapImage::create(croppedSkImage.release());
     }
 
     sk_sp<SkSurface> surface = SkSurface::MakeRasterN32Premul(cropRect.width(), cropRect.height());