Implement Blink-in-JS for DOM methods

Implement Blink-in-JS for DOM methods

* This CL implements the core infrastructure of Blink-in-JS to support DOM methods. DOM attributes, DOM constants etc will be supported in follow-up CLs.

* Developers can use Blink-in-JS as follows:

(1) Implement DOM features in JavaScript.

// Foo.js
installClass("Foo", function(window) {
  var FooPrototype = {};
  FooPrototype.method = function(a, b) { ... } // DOM method
  return FooPrototype;
});

(2) Add an [ImplementedInPrivateScript] IDL attribute to Foo.idl.

// Foo.idl
interface Foo {
  [ImplementedInPrivateScript] DOMString method(short a, Node b);
};

* For more details, see changes to Internals.js and Internals.idl.

* bindings/scripts/v8_methods.py and bindings/templates/methods.cpp implement the binding layer that connects DOM methods with private scripts. Only primitive types and DOM wrappers are allowed as types of return values and parameter values of Blink-in-JS.

BUG=341031
TEST=fast/dom/private_script_unittest.html

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=177298

[haraken, 2014-06-30 13:04:17]

abarth@, dcarney@: PTAL.

Sorry about a large CL, but this is a minimum CL to support DOM methods in
Blink-in-JS with tests.

I added testing APIs to Internals.idl and wrote tests in layout tests. However,
I still need to add the byte stream of Internals.js into PrivateScriptSources.h
even in release builds. This increases the binary size by 2.3 KB. I couldn't
come up with an easy way to omit the byte stream from release builds. Probably
we can just omit the tests from the Blink trunk (I can test Blink-in-JS locally
at the moment; once we have enough real use cases of Blink-in-JS, they will
become test cases).

arv@, adamk@, tasak@, nbarth@: FYI

[abarth-chromium, 2014-06-30 17:54:17]

On 2014/06/30 at 13:04:17, haraken wrote:
> abarth@, dcarney@: PTAL.
> 
> Sorry about a large CL, but this is a minimum CL to support DOM methods in
Blink-in-JS with tests.

This looks great, but I didn't review the Python parts.

> I added testing APIs to Internals.idl and wrote tests in layout tests.
However, I still need to add the byte stream of Internals.js into
PrivateScriptSources.h even in release builds. This increases the binary size by
2.3 KB. I couldn't come up with an easy way to omit the byte stream from release
builds. Probably we can just omit the tests from the Blink trunk (I can test
Blink-in-JS locally at the moment; once we have enough real use cases of
Blink-in-JS, they will become test cases).

Yeah...  Once we use this feature for something, we can test the production code
that uses it.  That's what we do for Blink-in-CPP.  :)

[Jens Widell, 2014-06-30 18:32:37]

https://codereview.chromium.org/360703003/diff/40001/Source/bindings/scripts/...
File Source/bindings/scripts/idl_types.py (right):

https://codereview.chromium.org/360703003/diff/40001/Source/bindings/scripts/...
Source/bindings/scripts/idl_types.py:331: def is_typed_array_type(self):
A bit asymmetrical I suppose to implement this here, when
IdlType.is_typed_array_type is actually defined as an "extension" in
v8_types.py.

Doing

  IdlUnionType.is_typed_array_type = False

there instead would be more symmetric. (And of course, fixing the FIXME above to
make IdlUnionType inherit IdlType would fix it even better.)

https://codereview.chromium.org/360703003/diff/40001/Source/bindings/scripts/...
File Source/bindings/scripts/v8_methods.py (right):

https://codereview.chromium.org/360703003/diff/40001/Source/bindings/scripts/...
Source/bindings/scripts/v8_methods.py:117:
'argument_declarations_for_private_script':
argument_declarations_for_private_script(interface, method),
Wrap this line?

https://codereview.chromium.org/360703003/diff/40001/Source/bindings/scripts/...
Source/bindings/scripts/v8_methods.py:231: argument_declarations.extend(['%s*
holderImpl' % interface.name])
Could use append() instead of extend() here, to save a [] pair for future use.

https://codereview.chromium.org/360703003/diff/40001/Source/bindings/template...
File Source/bindings/templates/interface_base.cpp (left):

https://codereview.chromium.org/360703003/diff/40001/Source/bindings/template...
Source/bindings/templates/interface_base.cpp:89: {% for world_suffix in
method.world_suffixes %}
The method_implemented_in_private_script() macro doesn't appear to depend on
'world_suffix' (and ought to take it as an argument if it did, I think) but you
still call it once for each world suffix.

Intentional?

[haraken, 2014-07-01 04:09:56]

abarth@: PTAL at overall design. I enabled the unittests only in Debug builds.
Specifically, I put the byte stream of Internals.js into
PrivateScriptSourcesForTesting.h, which is built only in Debug builds.

jl@: PTAL at the code generator.

Thanks!

https://codereview.chromium.org/360703003/diff/40001/Source/bindings/scripts/...
File Source/bindings/scripts/idl_types.py (right):

https://codereview.chromium.org/360703003/diff/40001/Source/bindings/scripts/...
Source/bindings/scripts/idl_types.py:331: def is_typed_array_type(self):
On 2014/06/30 18:32:37, Jens Lindström wrote:
> A bit asymmetrical I suppose to implement this here, when
> IdlType.is_typed_array_type is actually defined as an "extension" in
> v8_types.py.
> 
> Doing
> 
>   IdlUnionType.is_typed_array_type = False
> 
> there instead would be more symmetric. (And of course, fixing the FIXME above
to
> make IdlUnionType inherit IdlType would fix it even better.)

Done.

https://codereview.chromium.org/360703003/diff/40001/Source/bindings/scripts/...
File Source/bindings/scripts/v8_methods.py (right):

https://codereview.chromium.org/360703003/diff/40001/Source/bindings/scripts/...
Source/bindings/scripts/v8_methods.py:117:
'argument_declarations_for_private_script':
argument_declarations_for_private_script(interface, method),
On 2014/06/30 18:32:37, Jens Lindström wrote:
> Wrap this line?

Done.

https://codereview.chromium.org/360703003/diff/40001/Source/bindings/scripts/...
Source/bindings/scripts/v8_methods.py:231: argument_declarations.extend(['%s*
holderImpl' % interface.name])
On 2014/06/30 18:32:37, Jens Lindström wrote:
> Could use append() instead of extend() here, to save a [] pair for future use.

Done.

https://codereview.chromium.org/360703003/diff/40001/Source/bindings/template...
File Source/bindings/templates/interface_base.cpp (left):

https://codereview.chromium.org/360703003/diff/40001/Source/bindings/template...
Source/bindings/templates/interface_base.cpp:89: {% for world_suffix in
method.world_suffixes %}
On 2014/06/30 18:32:37, Jens Lindström wrote:
> The method_implemented_in_private_script() macro doesn't appear to depend on
> 'world_suffix' (and ought to take it as an argument if it did, I think) but
you
> still call it once for each world suffix.
> 
> Intentional?

Good point. I moved method_implemented_in_private_script() out of the
world_suffix loop.

[abarth-chromium, 2014-07-01 04:52:31]

I didn't look at the code generator, but the generated code and the tests LGTM!

https://codereview.chromium.org/360703003/diff/80001/LayoutTests/TestExpectat...
File LayoutTests/TestExpectations (right):

https://codereview.chromium.org/360703003/diff/80001/LayoutTests/TestExpectat...
LayoutTests/TestExpectations:807: crbug.com/390377 [ Release ]
fast/dom/private_script_unittest.html [ Crash ]
You can just skip it in release.  You should add a comment explaining why.

[Jens Widell, 2014-07-01 05:31:55]

Code generator LGTM.

[haraken, 2014-07-01 07:52:27]

Thanks for review! Landing.

(I added Internals.js, which I forgot to add.)

[haraken, 2014-07-01 07:52:31]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-07-01 07:53:32]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/haraken@chromium.org/360703003/110001

[commit-bot: I haz the power, 2014-07-01 08:55:00]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_blink_compile_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_compile_dbg/bu...)
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/14786)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/27972)

[commit-bot: I haz the power, 2014-07-01 08:59:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-07-01 08:59:22]

Try jobs failed on following builders:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/14801)

[haraken, 2014-07-01 08:59:40]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-07-01 08:59:53]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/haraken@chromium.org/360703003/110001

[commit-bot: I haz the power, 2014-07-01 09:00:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-07-01 09:00:16]

Failed to apply patch for Source/bindings/tests/results/V8TestObject.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/bindings/tests/results/V8TestObject.cpp
  Hunk #1 FAILED at 35.
  Hunk #3 succeeded at 9627 (offset -1 lines).
  Hunk #4 succeeded at 10310 (offset -1 lines).
  1 out of 4 hunks FAILED -- saving rejects to file
Source/bindings/tests/results/V8TestObject.cpp.rej

Patch:       Source/bindings/tests/results/V8TestObject.cpp
Index: Source/bindings/tests/results/V8TestObject.cpp
diff --git a/Source/bindings/tests/results/V8TestObject.cpp
b/Source/bindings/tests/results/V8TestObject.cpp
index
fddb3f1b8623bf5771ea0c2af88a9435dc10c942..e733370ff042969722bb33fcb0a2fbecd4362531
100644
--- a/Source/bindings/tests/results/V8TestObject.cpp
+++ b/Source/bindings/tests/results/V8TestObject.cpp
@@ -35,6 +35,7 @@
 #include "bindings/v8/BindingSecurity.h"
 #include "bindings/v8/Dictionary.h"
 #include "bindings/v8/ExceptionState.h"
+#include "bindings/v8/PrivateScriptRunner.h"
 #include "bindings/v8/ScriptCallStackFactory.h"
 #include "bindings/v8/ScriptPromise.h"
 #include "bindings/v8/ScriptState.h"
@@ -53,6 +54,7 @@
 #include "core/dom/Document.h"
 #include "core/dom/TagCollection.h"
 #include "core/dom/custom/CustomElementCallbackDispatcher.h"
+#include "core/frame/LocalFrame.h"
 #include "core/frame/UseCounter.h"
 #include "core/html/HTMLCollection.h"
 #include "core/html/HTMLFormControlsCollection.h"
@@ -9627,6 +9629,318 @@ static void
voidMethodTestInterfaceWillBeGarbageCollectedArrayArgMethodCallback(
     TRACE_EVENT_SET_SAMPLING_STATE("v8", "V8Execution");
 }
 
+static bool
voidMethodImplementedInPrivateScriptMethodImplementedInPrivateScript(LocalFrame*
frame, TestObject* holderImpl)
+{
+    if (!frame)
+        return false;
+    v8::Handle<v8::Context> context = toV8Context(frame,
DOMWrapperWorld::privateScriptIsolatedWorld());
+    if (context.IsEmpty())
+        return false;
+    ScriptState* scriptState = ScriptState::from(context);
+    if (!scriptState->executionContext())
+        return false;
+
+    ScriptState::Scope scope(scriptState);
+    v8::Handle<v8::Value> holder = toV8(holderImpl,
scriptState->context()->Global(), scriptState->isolate());
+
+    v8::Handle<v8::Value> *argv = 0;
+    // FIXME: Support exceptions thrown from Blink-in-JS.
+    v8::TryCatch block;
+    PrivateScriptRunner::runDOMMethod(scriptState, "TestObject",
"voidMethodImplementedInPrivateScript", holder, 0, argv);
+    if (block.HasCaught())
+        return false;
+    return true;
+}
+
+static void voidMethodImplementedInPrivateScriptMethod(const
v8::FunctionCallbackInfo<v8::Value>& info)
+{
+    TestObject* impl = V8TestObject::toNative(info.Holder());
+   
voidMethodImplementedInPrivateScriptMethodImplementedInPrivateScript(toFrameIfNotDetached(info.GetIsolate()->GetCurrentContext()),
impl);
+}
+
+static void voidMethodImplementedInPrivateScriptMethodCallback(const
v8::FunctionCallbackInfo<v8::Value>& info)
+{
+    TRACE_EVENT_SET_SAMPLING_STATE("blink", "DOMMethod");
+    TestObjectV8Internal::voidMethodImplementedInPrivateScriptMethod(info);
+    TRACE_EVENT_SET_SAMPLING_STATE("v8", "V8Execution");
+}
+
+static bool
shortMethodImplementedInPrivateScriptMethodImplementedInPrivateScript(LocalFrame*
frame, TestObject* holderImpl, int* result)
+{
+    if (!frame)
+        return false;
+    v8::Handle<v8::Context> context = toV8Context(frame,
DOMWrapperWorld::privateScriptIsolatedWorld());
+    if (context.IsEmpty())
+        return false;
+    ScriptState* scriptState = ScriptState::from(context);
+    if (!scriptState->executionContext())
+        return false;
+
+    ScriptState::Scope scope(scriptState);
+    v8::Handle<v8::Value> holder = toV8(holderImpl,
scriptState->context()->Global(), scriptState->isolate());
+
+    v8::Handle<v8::Value> *argv = 0;
+    // FIXME: Support exceptions thrown from Blink-in-JS.
+    v8::TryCatch block;
+    v8::Handle<v8::Value> v8Value =
PrivateScriptRunner::runDOMMethod(scriptState, "TestObject",
"shortMethodImplementedInPrivateScript", holder, 0, argv);
+    if (block.HasCaught())
+        return false;
+    ExceptionState exceptionState(ExceptionState::ExecutionContext,
"shortMethodImplementedInPrivateScript", "TestObject",
scriptState->context()->Global(), scriptState->isolate());
+    int cppValue = toInt16(v8Value, exceptionState);
+    if (block.HasCaught())
+        return false;
+    *result = cppValue;
+    return true;
+}
+
+static void shortMethodImplementedInPrivateScriptMethod(const
v8::FunctionCallbackInfo<v8::Value>& info)
+{
+    TestObject* impl = V8TestObject::toNative(info.Holder());
+    int result;
+    if
(!shortMethodImplementedInPrivateScriptMethodImplementedInPrivateScript(toFrameIfNotDetached(info.GetIsolate()->GetCurrentContext()),
impl, &result))
+        return;
+    v8SetReturnValueInt(info, result);
+}
+
+static void shortMethodImplementedInPrivateScriptMethodCallback(const
v8::FunctionCallbackInfo<v8::Value>& info)
+{
+    TRACE_EVENT_SET_SAMPLING_STATE("blink", "DOMMethod");
+    TestObjectV8Internal::shortMethodImplementedInPrivateScriptMethod(info);
+    TRACE_EVENT_SET_SAMPLING_STATE("v8", "V8Execution");
+}
+
+static bool
shortMethodWithShortArgumentImplementedInPrivateScriptMethodImplementedInPrivateScript(LocalFrame*
frame, TestObject* holderImpl, int value, int* result)
+{
+    if (!frame)
+        return false;
+    v8::Handle<v8::Context> context = toV8Context(frame,
DOMWrapperWorld::privateScriptIsolatedWorld());
+    if (context.IsEmpty())
+        return false;
+    ScriptState* scriptState = ScriptState::from(context);
+    if (!scriptState->executionContext())
+        return false;
+
+    ScriptState::Scope scope(scriptState);
+    v8::Handle<v8::Value> holder = toV8(holderImpl,
scriptState->context()->Global(), scriptState->isolate());
+
+    v8::Handle<v8::Value> valueHandle =
v8::Integer::New(scriptState->isolate(), value);
+    v8::Handle<v8::Value> argv[] = { valueHandle };
+    // FIXME: Support exceptions thrown from Blink-in-JS.
+    v8::TryCatch block;
+    v8::Handle<v8::Value> v8Value =
PrivateScriptRunner::runDOMMethod(scriptState, "TestObject",
"shortMethodWithShortArgumentImplementedInPrivateScript", holder, 1, argv);
+    if (block.HasCaught())
+        return false;
+    ExceptionState exceptionState(ExceptionState::ExecutionContext,
"shortMethodWithShortArgumentImplementedInPrivateScript", "TestObject",
scriptState->context()->Global(), scriptState->isolate());
+    int cppValue = toInt16(v8Value, exceptionState);
+    if (block.HasCaught())
+        return false;
+    *result = cppValue;
+    return true;
+}
+
+static void shortMethodWithShortArgumentImplementedInPrivateScriptMethod(const
v8::FunctionCallbackInfo<v8::Value>& info)
+{
+    ExceptionState exceptionState(ExceptionState::ExecutionContext,
"shortMethodWithShortArgumentImplementedInPrivateScript", "TestObject",
info.Holder(), info.GetIsolate());
+    if (UNLIKELY(info.Length() < 1)) {
+        throwMinimumArityTypeError(exceptionState, 1, info.Length());
+        return;
+    }
+    TestObject* impl = V8TestObject::toNative(info.Holder());
+    int value;
+    {
+        v8::TryCatch block;
+        V8RethrowTryCatchScope rethrow(block);
+        TONATIVE_VOID_EXCEPTIONSTATE_INTERNAL(value, toInt16(info[0],
exceptionState), exceptionState);
+    }
+    int result;
+    if
(!shortMethodWithShortArgumentImplementedInPrivateScriptMethodImplementedInPrivateScript(toFrameIfNotDetached(info.GetIsolate()->GetCurrentContext()),
impl, value, &result))
+        return;
+    v8SetReturnValueInt(info, result);
+}
+
+static void
shortMethodWithShortArgumentImplementedInPrivateScriptMethodCallback(const
v8::FunctionCallbackInfo<v8::Value>& info)
+{
+    TRACE_EVENT_SET_SAMPLING_STATE("blink", "DOMMethod");
+   
TestObjectV8Internal::shortMethodWithShortArgumentImplementedInPrivateScriptMethod(info);
+    TRACE_EVENT_SET_SAMPLING_STATE("v8", "V8Execution");
+}
+
+static bool
stringMethodWithStringArgumentImplementedInPrivateScriptMethodImplementedInPrivateScript(LocalFrame*
frame, TestObject* holderImpl, String value, String* result)
+{
+    if (!frame)
+        return false;
+    v8::Handle<v8::Context> context = toV8Context(frame,
DOMWrapperWorld::privateScriptIsolatedWorld());
+    if (context.IsEmpty())
+        return false;
+    ScriptState* scriptState = ScriptState::from(context);
+    if (!scriptState->executionContext())
+        return false;
+
+    ScriptState::Scope scope(scriptState);
+    v8::Handle<v8::Value> holder = toV8(holderImpl,
scriptState->context()->Global(), scriptState->isolate());
+
+    v8::Handle<v8::Value> valueHandle = v8String(scriptState->isolate(),
value);
+    v8::Handle<v8::Value> argv[] = { valueHandle };
+    // FIXME: Support exceptions thrown from Blink-in-JS.
+    v8::TryCatch block;
+    v8::Handle<v8::Value> v8Value =
PrivateScriptRunner::runDOMMethod(scriptState, "TestObject",
"stringMethodWithStringArgumentImplementedInPrivateScript", holder, 1, argv);
+    if (block.HasCaught())
+        return false;
+    ExceptionState exceptionState(ExceptionState::ExecutionContext,
"stringMethodWithStringArgumentImplementedInPrivateScript", "TestObject",
scriptState->context()->Global(), scriptState->isolate());
+    V8StringResource<> cppValue = v8Value;
+    if (block.HasCaught())
+        return false;
+    *result = cppValue;
+    return true;
+}
+
+static void
stringMethodWithStringArgumentImplementedInPrivateScriptMethod(const
v8::FunctionCallbackInfo<v8::Value>& info)
+{
+    if (UNLIKELY(info.Length() < 1)) {
+       
throwMinimumArityTypeErrorForMethod("stringMethodWithStringArgumentImplementedInPrivateScript",
"TestObject", 1, info.Length(), info.GetIsolate());
+        return;
+    }
+    TestObject* impl = V8TestObject::toNative(info.Holder());
+    V8StringResource<> value;
+    {
+        TOSTRING_VOID_INTERNAL(value, info[0]);
+    }
+    String r…
(message too large)

[haraken, 2014-07-01 09:43:53]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-07-01 09:45:49]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/haraken@chromium.org/360703003/130001

[commit-bot: I haz the power, 2014-07-01 10:28:15]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_blink_compile_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/android_blink_compile_db...)
  android_blink_compile_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/android_blink_compile_re...)
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/1...)
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/1...)
  linux_chromium_gn_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_chromium_gn_rel/bu...)
  mac_blink_compile_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_compile_dbg/bu...)
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/13756)
  win_blink_compile_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_compile_dbg/bu...)
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/14812)
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/23599)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/28006)

[commit-bot: I haz the power, 2014-07-01 10:35:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-07-01 10:35:12]

Try jobs failed on following builders:
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/1...)

[haraken, 2014-07-01 10:43:43]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-07-01 10:44:29]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/haraken@chromium.org/360703003/150001

[commit-bot: I haz the power, 2014-07-01 11:50:34]

Change committed as 177298