Implement Asynchronous image encoding for Canvas.toBlob

Source/core/html/HTMLCanvasElement.cpp

Index: Source/core/html/HTMLCanvasElement.cpp
diff --git a/Source/core/html/HTMLCanvasElement.cpp b/Source/core/html/HTMLCanvasElement.cpp
index 6dc6f92d1556336a741cd0d237d79c7bb972ab68..1d75f9f04a86cd6f52f075ba668554996996a25c 100644
--- a/Source/core/html/HTMLCanvasElement.cpp
+++ b/Source/core/html/HTMLCanvasElement.cpp
@@ -46,6 +46,8 @@
 #include "core/paint/DeprecatedPaintLayer.h"
 #include "platform/MIMETypeRegistry.h"
 #include "platform/RuntimeEnabledFeatures.h"
+#include "platform/Task.h"
+#include "platform/ThreadSafeFunctional.h"
 #include "platform/graphics/Canvas2DImageBufferSurface.h"
 #include "platform/graphics/ExpensiveCanvasHeuristicParameters.h"
 #include "platform/graphics/ImageBuffer.h"
@@ -56,7 +58,6 @@
 #include "platform/transforms/AffineTransform.h"
 #include "public/platform/Platform.h"
 #include "public/platform/WebTraceLocation.h"
-#include "wtf/Functional.h"
 #include <math.h>
 #include <v8.h>
 
@@ -78,6 +79,10 @@ const int MaxCanvasArea = 32768 * 8192; // Maximum canvas area in CSS pixels
 // In Skia, we will also limit width/height to 32767.
 const int MaxSkiaDim = 32767; // Maximum width/height in CSS pixels.
 
+// A default value of quality argument for toDataURL and toBlob
+// It is in an invalid range (outside 0.0 - 1.0) so that it will not be misinterpreted as a user-input value
+const int InvalidQualityValue = -1.0;

[Justin Novosad, 2015/09/23 20:32:32]

I would call this Undefined instead of Invalid. "Invalid" suggest this is an
error, "Undefined" suggest a default should be used.

+
 bool canCreateImageBuffer(const IntSize& size)
 {
     if (size.isEmpty())
@@ -99,6 +104,9 @@ PassRefPtr<Image> createTransparentImage(const IntSize& size)
 
 } // namespace
 
+static WebThread* s_thread = 0;

[Justin Novosad, 2015/09/23 20:32:32]

You should make these statics members of HTMLCanvasElement.  Also, the name is
not very descriptive. s_blobEncoderThread would be better.

+static int numCanvasInstances = 0;

[Justin Novosad, 2015/09/23 20:32:32]

missing s_

+
 DEFINE_EMPTY_DESTRUCTOR_WILL_BE_REMOVED(CanvasObserver);
 
 inline HTMLCanvasElement::HTMLCanvasElement(Document& document)
@@ -113,6 +121,7 @@ inline HTMLCanvasElement::HTMLCanvasElement(Document& document)
     , m_imageBufferIsClear(false)
 {
     setHasCustomStyleCallbacks();
+    numCanvasInstances++;
 }
 
 DEFINE_NODE_FACTORY(HTMLCanvasElement)
@@ -120,6 +129,9 @@ DEFINE_NODE_FACTORY(HTMLCanvasElement)
 HTMLCanvasElement::~HTMLCanvasElement()
 {
     v8::Isolate::GetCurrent()->AdjustAmountOfExternalAllocatedMemory(-m_externallyAllocatedMemory);
+    numCanvasInstances--;
+    if (numCanvasInstances == 0 && !!s_thread)
+        s_thread = 0;

[Justin Novosad, 2015/09/23 20:32:32]

This is a memory leak. You should make s_thread an OwnPtr<WebThread>, and call
clear() on it.

 #if !ENABLE(OILPAN)
     for (CanvasObserver* canvasObserver : m_observers)
         canvasObserver->canvasDestroyed(this);
@@ -510,7 +522,7 @@ ImageData* HTMLCanvasElement::toImageData(SourceDrawingBuffer sourceBuffer) cons
     return imageData;
 }
 
-String HTMLCanvasElement::toDataURLInternal(const String& mimeType, const double* quality, SourceDrawingBuffer sourceBuffer) const
+String HTMLCanvasElement::toDataURLInternal(const String& mimeType, const double& quality, SourceDrawingBuffer sourceBuffer) const
 {
     if (!isPaintable())
         return String("data:,");
@@ -529,52 +541,70 @@ String HTMLCanvasElement::toDataURL(const String& mimeType, const ScriptValue& q
         exceptionState.throwSecurityError("Tainted canvases may not be exported.");
         return String();
     }
-    double quality;
-    double* qualityPtr = nullptr;
+    double quality = InvalidQualityValue;
     if (!qualityArgument.isEmpty()) {
         v8::Local<v8::Value> v8Value = qualityArgument.v8Value();
         if (v8Value->IsNumber()) {
             quality = v8Value.As<v8::Number>()->Value();
-            qualityPtr = &quality;
         }
     }
-    return toDataURLInternal(mimeType, qualityPtr, BackBuffer);
+    return toDataURLInternal(mimeType, quality, BackBuffer);
+}
+
+void HTMLCanvasElement::encodeImageAsync(DOMUint8ClampedArray* imageData, IntSize imageSize, FileCallback* callback, const String& mimeType, double quality)
+{
+    OwnPtr<Vector<char>> encodedImage(adoptPtr(new Vector<char>()));
+
+    if (!ImageDataBuffer(imageSize, imageData->data()).encodeImage(mimeType, quality, encodedImage.get())) {
+        Platform::current()->mainThread()->taskRunner()->postTask(FROM_HERE, bind(&FileCallback::handleEvent, callback, nullptr));
+    } else {
+        Platform::current()->mainThread()->taskRunner()->postTask(FROM_HERE, threadSafeBind(&HTMLCanvasElement::createBlobAndCall, AllowCrossThreadAccess(this), encodedImage.release(), mimeType, AllowCrossThreadAccess(callback)));
+    }
 }
 
-void HTMLCanvasElement::toBlob(FileCallback* callback, const String& mimeType, const ScriptValue& qualityArgument, ExceptionState& exceptionState) const
+void HTMLCanvasElement::createBlobAndCall(PassOwnPtr<Vector<char>> encodedImage, const String& mimeType, FileCallback* callback)
+{
+    // The main thread takes ownership of encoded image vector
+    OwnPtr<Vector<char>> enc(encodedImage);
+
+    File* resultBlob = File::create(enc->data(), enc->size(), mimeType);
+    Platform::current()->mainThread()->taskRunner()->postTask(FROM_HERE, bind(&FileCallback::handleEvent, callback, resultBlob));
+}
+
+void HTMLCanvasElement::toBlob(FileCallback* callback, const String& mimeType, const ScriptValue& qualityArgument, ExceptionState& exceptionState)
 {
     if (!originClean()) {
         exceptionState.throwSecurityError("Tainted canvases may not be exported.");
         return;
     }
 
-    File* resultBlob = nullptr;
     if (!isPaintable()) {
         // If the canvas element's bitmap has no pixels
+        Platform::current()->mainThread()->taskRunner()->postTask(FROM_HERE, bind(&FileCallback::handleEvent, callback, nullptr));
         return;
     }
 
-    double quality;
-    double* qualityPtr = nullptr;
+    double quality = InvalidQualityValue;
     if (!qualityArgument.isEmpty()) {
         v8::Local<v8::Value> v8Value = qualityArgument.v8Value();
         if (v8Value->IsNumber()) {
             quality = v8Value.As<v8::Number>()->Value();
-            qualityPtr = &quality;
         }
     }
 
     String encodingMimeType = toEncodingMimeType(mimeType);
 
     ImageData* imageData = toImageData(BackBuffer);
+    // ImageData object will be disposed on leaving stack scope; only imageData->data and imageData->size are passed to s_thread

[Justin Novosad, 2015/09/23 20:32:32]

This comment describes poorly what is happening. The scoped disposal forces the
imageData to unref it's data, which we keep alive for the thread by taking a
refptr. This way, the life time of the data is tied to the completion of the
async encode, rather than to the garbage collection of imageData.

     ScopedDisposal<ImageData> disposer(imageData);
 
-    // Perform image encoding
-    Vector<char> encodedImage;
-    ImageDataBuffer(imageData->size(), imageData->data()->data()).encodeImage(encodingMimeType, qualityPtr, &encodedImage);
-    resultBlob = File::create(encodedImage.data(), encodedImage.size(), encodingMimeType);
+    // Add a ref to keep image data alive until completion of encoding
+    RefPtr<DOMUint8ClampedArray> imageDataRef(imageData->data());
 
-    Platform::current()->mainThread()->taskRunner()->postTask(FROM_HERE, bind(&FileCallback::handleEvent, callback, resultBlob));
+    if (!s_thread) {
+        s_thread = Platform::current()->createThread("Async toBlob");
+    }
+    s_thread->taskRunner()->postTask(FROM_HERE, new Task(threadSafeBind(&HTMLCanvasElement::encodeImageAsync, AllowCrossThreadAccess(this), AllowCrossThreadAccess(imageDataRef.release().leakRef()), imageData->size(), AllowCrossThreadAccess(callback), encodingMimeType, quality)));

[Justin Novosad, 2015/09/23 20:32:32]

This is unsafe. We cannot guarantee that 'this' will stay alive long enough. 
You should add a layout test that does this:
1) create an offscreen canvas with: var canvas =
document.createElement('canvas');
2) draw to the canvas and call toBlob
3) dereference the canvas (set the variable to null)
4) force an immediate garbage collection (there is a test API for this either on
internals or testRunner)
5) wait for the callback to be invoked and verify the blob for correctness.

 }
 
 SecurityOrigin* HTMLCanvasElement::securityOrigin() const