Hook up CSSPaintValue::image to CSS Paint API callback.

third_party/WebKit/Source/modules/csspaint/CSSPaintDefinition.cpp

Index: third_party/WebKit/Source/modules/csspaint/CSSPaintDefinition.cpp
diff --git a/third_party/WebKit/Source/modules/csspaint/CSSPaintDefinition.cpp b/third_party/WebKit/Source/modules/csspaint/CSSPaintDefinition.cpp
index 339962f6cd6147786d9d8736a493e3331167b866..4c5702d3dccad2c74ff0c1094df9db3be66e2093 100644
--- a/third_party/WebKit/Source/modules/csspaint/CSSPaintDefinition.cpp
+++ b/third_party/WebKit/Source/modules/csspaint/CSSPaintDefinition.cpp
@@ -5,9 +5,37 @@
 #include "modules/csspaint/CSSPaintDefinition.h"
 
 #include "bindings/core/v8/ScriptState.h"
+#include "bindings/core/v8/V8Binding.h"
+#include "bindings/core/v8/V8BindingMacros.h"
+#include "core/dom/ExecutionContext.h"
+#include "core/frame/ConsoleTypes.h"
+#include "core/inspector/ConsoleMessage.h"
+#include "modules/csspaint/Geometry.h"
+#include "modules/csspaint/PaintRenderingContext2D.h"
+#include "platform/ScriptForbiddenScope.h"
+#include "platform/graphics/ImageBuffer.h"
+#include "platform/graphics/PaintGeneratedImage.h"
+#include "platform/graphics/RecordingImageBufferSurface.h"
+#include "platform/graphics/UnacceleratedImageBufferSurface.h"
 
 namespace blink {
 
+namespace {
+
+// TODO(ikilpatrick): Modify RecordingImageBufferSurface so that it cannot
+// fallback to a UnacceleratedImageBufferSurface.
+class UnacceleratedSurfaceFactory : public RecordingImageBufferFallbackSurfaceFactory {
+public:
+    virtual PassOwnPtr<ImageBufferSurface> createSurface(const IntSize& size, OpacityMode opacityMode)
+    {
+        return adoptPtr(new UnacceleratedImageBufferSurface(size, opacityMode));
+    }
+
+    virtual ~UnacceleratedSurfaceFactory() { }
+};
+
+} // namespace
+
 CSSPaintDefinition* CSSPaintDefinition::create(ScriptState* scriptState, v8::Local<v8::Function> constructor, v8::Local<v8::Function> paint)
 {
     return new CSSPaintDefinition(scriptState, constructor, paint);
@@ -24,4 +52,74 @@ CSSPaintDefinition::~CSSPaintDefinition()
 {
 }
 
+PassRefPtr<Image> CSSPaintDefinition::paint(const IntSize& size)
+{
+    v8::Isolate* isolate = m_scriptState->isolate();
+
+    ScriptState::Scope scope(m_scriptState.get());
+    v8::MicrotasksScope microtasksScope(isolate, v8::MicrotasksScope::kRunMicrotasks);

[haraken, 2016/04/11 02:18:14]

v8::MicrotasksScope is handled by V8ScriptRunner::xxx, so you won't need this.

[ikilpatrick, 2016/04/11 22:34:24]

Done.

+
+    maybeCreatePaintInstance();
+    v8::Local<v8::Object> instance = m_instance.newLocal(isolate);
+
+    // We may have failed to create an instance class, in which case produce an
+    // invalid image.
+    if (isUndefinedOrNull(instance))
+        return nullptr;
+
+    PaintRenderingContext2D* renderingContext = PaintRenderingContext2D::create(
+        ImageBuffer::create(adoptPtr(new RecordingImageBufferSurface(size, adoptPtr(new UnacceleratedSurfaceFactory())))));
+    Geometry* geom = Geometry::create(size);

[haraken, 2016/04/11 02:18:14]

geom => geometry

[ikilpatrick, 2016/04/11 22:34:24]

Done.

+
+    v8::Local<v8::Value> argv[] = {
+        toV8(renderingContext, m_scriptState->context()->Global(), isolate),
+        toV8(geom, m_scriptState->context()->Global(), isolate)
+    };
+
+    v8::Local<v8::Function> paint = m_paint.newLocal(isolate);
+
+    ScriptForbiddenScope::AllowUserAgentScript allowScripting;

[haraken, 2016/04/11 02:18:14]

Why do you need ScriptForbiddenScope::AllowUserAgentScript? It should not be
used unless you really need to execute scripts in a place where script execution
is forbidden.

[ikilpatrick, 2016/04/11 22:34:24]

Done.

+    v8::TryCatch block(isolate);

[haraken, 2016/04/11 02:18:14]

You won't need this TryCatch.

+
+    V8ScriptRunner::callFunction(paint, m_scriptState->getExecutionContext(), instance, 2, argv, isolate);
+
+    // The paint function may have produced an error, in which case produce an
+    // invalid image.
+    if (block.HasCaught()) {

[haraken, 2016/04/11 02:18:14]

What happens if the paint function didn't throw a JS exception but didn't
something wrong with painting? It looks a bit strange to special-case a case
where the paint function throws a JS exception (because that wouldn't be the
only case where we should return an invalid image).

Maybe do we want to just remove the block.HasCaught branch and unconditionally
return PaintGeneratedImage::create()?

[ikilpatrick, 2016/04/11 22:34:23]

Sorry didn't get "but didn't wrong with painting?"

We talked about this at the Houdini F2F, most people thought that it made more
sense to have an invalid image instead of a partially drawn image for JS Errors.

[haraken, 2016/04/12 00:44:20]

Thanks, makes sense.

+        // TODO(ikilpatrick): messageHandlerInMainThread doesn't currently know
+        // about main-thread worklets.
+        m_scriptState->getExecutionContext()->addConsoleMessage(ConsoleMessage::create(JSMessageSource, ErrorMessageLevel, "Failed to invoke the paint method."));
+        return nullptr;
+    }
+
+    return PaintGeneratedImage::create(renderingContext->imageBuffer()->getPicture(), size);
+}
+
+void CSSPaintDefinition::maybeCreatePaintInstance()
+{
+    ASSERT(m_invalidConstructor || m_instance.isEmpty());
+
+    if (m_invalidConstructor)
+        return;
+
+    if (!m_instance.isEmpty())
+        return;
+
+    v8::Isolate* isolate = m_scriptState->isolate();
+    v8::Local<v8::Function> constructor = m_constructor.newLocal(isolate);
+    ASSERT(!isUndefinedOrNull(constructor));
+
+    v8::Local<v8::Object> paintInstance;
+    v8::TryCatch block(isolate);

[haraken, 2016/04/11 02:18:14]

You won't need this v8::TryCatch.

[ikilpatrick, 2016/04/11 22:34:24]

Done.

+
+    if (!v8Call(constructor->NewInstance(m_scriptState->context()), paintInstance, block)) {

[haraken, 2016/04/11 02:18:14]

Can we use V8ObjectConstructor::newInstance? Then it will handle subtle stuff
like MicrotaskScope etc.

[ikilpatrick, 2016/04/11 22:34:23]

Done.

+        m_invalidConstructor = true;
+        // TODO(ikilpatrick): messageHandlerInMainThread doesn't currently

[haraken, 2016/04/11 02:18:14]

Or maybe you can use v8CallOrCrash until you find a better way to handle
scenarios where v8Call fails.

[ikilpatrick, 2016/04/11 22:34:24]

Just removed the log at the moment. Filed crbug.com/602445 to track
messageHandlerInMainThread issue.

+        // know about main-thread worklets.
+        m_scriptState->getExecutionContext()->addConsoleMessage(ConsoleMessage::create(JSMessageSource, ErrorMessageLevel, "Failed to create instance of class."));
+    } else {
+        m_instance.set(isolate, paintInstance);
+    }
+}
+
 } // namespace blink