Fix data race problem in createImageBitmap(Blob)

third_party/WebKit/Source/core/imagebitmap/ImageBitmapFactories.cpp

Index: third_party/WebKit/Source/core/imagebitmap/ImageBitmapFactories.cpp
diff --git a/third_party/WebKit/Source/core/imagebitmap/ImageBitmapFactories.cpp b/third_party/WebKit/Source/core/imagebitmap/ImageBitmapFactories.cpp
index 22d2cd2190dfaf3ecd236a1e664806a31269b37e..e4810db249819952093a4a85c3604d4a8bf0b1b0 100644
--- a/third_party/WebKit/Source/core/imagebitmap/ImageBitmapFactories.cpp
+++ b/third_party/WebKit/Source/core/imagebitmap/ImageBitmapFactories.cpp
@@ -44,7 +44,7 @@
 #include "core/workers/WorkerGlobalScope.h"
 #include "platform/SharedBuffer.h"
 #include "platform/ThreadSafeFunctional.h"
-#include "platform/image-decoders/ImageDecoder.h"
+#include "platform/graphics/DeferredImageDecoder.h"
 #include "platform/threading/BackgroundTaskRunner.h"
 #include "public/platform/Platform.h"
 #include "public/platform/WebThread.h"
@@ -222,19 +222,19 @@ void ImageBitmapFactories::ImageBitmapLoader::decodeImageOnDecoderThread(WebTask
     ImageDecoder::AlphaOption alphaOp = ImageDecoder::AlphaPremultiplied;
     if (m_options.premultiplyAlpha() == "none")
         alphaOp = ImageDecoder::AlphaNotPremultiplied;
-    OwnPtr<ImageDecoder> decoder(ImageDecoder::create(*sharedBuffer, alphaOp, ImageDecoder::GammaAndColorProfileApplied));
+    OwnPtr<DeferredImageDecoder> decoder(DeferredImageDecoder::create(*sharedBuffer, alphaOp, ImageDecoder::GammaAndColorProfileApplied));
     if (decoder)
-        decoder->setData(sharedBuffer.get(), true);
+        decoder->setData(*sharedBuffer, true);
     taskRunner->postTask(BLINK_FROM_HERE, threadSafeBind(&ImageBitmapFactories::ImageBitmapLoader::resolvePromiseOnOriginalThread, AllowCrossThreadAccess(this), decoder.release()));
 }
 
-void ImageBitmapFactories::ImageBitmapLoader::resolvePromiseOnOriginalThread(PassOwnPtr<ImageDecoder> decoder)
+void ImageBitmapFactories::ImageBitmapLoader::resolvePromiseOnOriginalThread(PassOwnPtr<DeferredImageDecoder> decoder)
 {
     if (!decoder) {
         rejectPromise();
         return;
     }
-    RefPtr<SkImage> frame = ImageBitmap::getSkImageFromDecoder(decoder);
+    RefPtr<SkImage> frame = decoder->createFrameAtIndex(0);

[Justin Novosad, 2016/03/09 16:41:03]

Have you verified this in tracing? The DeferredImageDecoder uses a lazy decoding
scheme, and I think that in this case, the decoding will only be triggered here,
on the main thread, which is not what you intended.  Perhaps
decodeImageOnDecoderThread() should be passing the final SkImage rather that the
decoder. If you do that, I think you should just use ImageDecoder.

Even though the thread-safeness of DeferredImageDecoder solves the race
condition, you should not use this class unless deferred decoding is your
intent, which is not the case here.

[xidachen, 2016/03/09 17:49:26]

I agree that getting the SkImage on the worker thread and pass it back to the
main thread is the best strategy and I actually tried this when I implemented
decoding in a background thread. The problem is that when passing SkImage across
thread, bindings gives me trouble. To pass a SkImage across thread using
postTask, it goes to the CrossThreadCopier and that requires all the object
passed across thread to be ThreadSafeRefCounted, and SkImage is not, neither
RefPtr is, so that's why I eventually chose to pass the ImageDecoder across
thread. 

I can see that there are two options here:
1. Wrap the RefPtr<SkImage> using a OwnPtr and pass across threads. This doesn't
look that good.
2. Change the CrossThreadCopierBase to be able to pass an object that is
SkRefCnt.

The second one is more reasonable. But I am not sure whether it is reasonable to
include Sk headers under platform/, WDYT?

     ASSERT(!frame || (frame->width() && frame->height()));
     if (!frame) {
         rejectPromise();