bindings: Supports reentrance to ScriptWrappable::wrap through V8DOMWrapper::createWrapper.

bindings: Supports reentrance to ScriptWrappable::wrap through V8DOMWrapper::createWrapper.

Considering V8DOMWrapper::createWrapper invokes v8::FunctionTemplate::GetFunction
and v8::Function::NewInstance, it may run an arbitrary script and reenter to
ScriptWrappable::wrap with the same object.

This CL supports such a case and consistently uses the wrapper that is created
and associated first.

BUG=477425, 389445
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=197984

[Yuki, 2015-06-29 10:51:35]

yukishiino@chromium.org changed reviewers:
+ haraken@chromium.org

[Yuki, 2015-06-29 10:51:35]

Could you review this CL?

[haraken, 2015-06-29 12:00:30]

haraken@chromium.org changed reviewers:
+ jochen@chromium.org

[haraken, 2015-06-29 12:00:31]

LGTM

I hope this won't regress performance :)

https://codereview.chromium.org/1210083004/diff/1/Source/core/dom/Node.cpp
File Source/core/dom/Node.cpp (right):

https://codereview.chromium.org/1210083004/diff/1/Source/core/dom/Node.cpp#ne...
Source/core/dom/Node.cpp:2416: v8::Local<v8::Object> Node::wrap(v8::Isolate*
isolate, v8::Local<v8::Object> creationContext)

Not related to this CL, but why do we need the custom wrap for Node::wrap? This
looks like the same as ScriptWrappable::wrap.

[Yuki, 2015-06-29 12:42:41]

https://codereview.chromium.org/1210083004/diff/1/Source/core/dom/Node.cpp
File Source/core/dom/Node.cpp (right):

https://codereview.chromium.org/1210083004/diff/1/Source/core/dom/Node.cpp#ne...
Source/core/dom/Node.cpp:2416: v8::Local<v8::Object> Node::wrap(v8::Isolate*
isolate, v8::Local<v8::Object> creationContext)
On 2015/06/29 12:00:30, haraken wrote:
> 
> Not related to this CL, but why do we need the custom wrap for Node::wrap?
This
> looks like the same as ScriptWrappable::wrap.

The code looks completely the same, but we have two versions of
DOMDataStore::getWrapper(), one takes ScriptWrappable* and the other takes
Node*.  The latter version has Node-specific optimization.  We'd like to
propagate type information.

[Yuki, 2015-06-29 12:43:12]

The CQ bit was checked by yukishiino@chromium.org

[commit-bot: I haz the power, 2015-06-29 12:43:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1210083004/1

[commit-bot: I haz the power, 2015-06-29 12:46:10]

Committed patchset #1 (id:1) as
https://src.chromium.org/viewvc/blink?view=rev&revision=197984

[haraken, 2015-06-29 13:53:59]

Forgot to say... can we add a test for this in a follow-up?

[Yuki, 2015-07-01 07:20:38]

On 2015/06/29 13:53:59, haraken wrote:
> Forgot to say... can we add a test for this in a follow-up?

I tried valueOf, but it didn't work.  When running f = new Foo(), Foo.valueOf
wasn't called.  It seems not easy to repro the case.