bindings: Moves toV8 and v8SetReturnValue out to ToV8.h and V8Binding.h.

bindings: Moves toV8 and v8SetReturnValue out to ToV8.h and V8Binding.h.

Also retires [Custom=ToV8], but the actual code doesn't change.
We still have special treatment for Window and WorkerGlobalScope (and
EventTarget as Window's super class).

BUG=

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

[Yuki, 2015-04-24 11:45:47]

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

[Yuki, 2015-04-24 11:45:47]

Could you review this CL?

[haraken, 2015-04-24 11:57:39]

LGTM

https://codereview.chromium.org/1083003004/diff/1/Source/bindings/core/v8/ToV...
File Source/bindings/core/v8/ToV8.cpp (right):

https://codereview.chromium.org/1083003004/diff/1/Source/bindings/core/v8/ToV...
Source/bindings/core/v8/ToV8.cpp:16: // Notice that we explicitly ignore
creationContext because the DOMWindow is its own creationContext.

is => has

https://codereview.chromium.org/1083003004/diff/1/Source/bindings/core/v8/ToV...
Source/bindings/core/v8/ToV8.cpp:37: if (UNLIKELY(!impl))

Remove UNLIKELY for consistency.

https://codereview.chromium.org/1083003004/diff/1/Source/bindings/core/v8/ToV...
Source/bindings/core/v8/ToV8.cpp:47: // Notice that we explicitly ignore
creationContext because the WorkerGlobalScope is its own creationContext.

is => has

https://codereview.chromium.org/1083003004/diff/1/Source/bindings/core/v8/ToV8.h
File Source/bindings/core/v8/ToV8.h (right):

https://codereview.chromium.org/1083003004/diff/1/Source/bindings/core/v8/ToV...
Source/bindings/core/v8/ToV8.h:51: // Special versions for DOMWindow and
WorkerGlobalScope

and EventTarget

https://codereview.chromium.org/1083003004/diff/1/Source/bindings/core/v8/V8B...
File Source/bindings/core/v8/V8Binding.h (right):

https://codereview.chromium.org/1083003004/diff/1/Source/bindings/core/v8/V8B...
Source/bindings/core/v8/V8Binding.h:220: // Special versions for DOMWindow and
WorkerGlobalScope

and EventTarget

https://codereview.chromium.org/1083003004/diff/1/Source/bindings/core/v8/V8B...
Source/bindings/core/v8/V8Binding.h:290: // Special versions for DOMWindow and
WorkerGlobalScope

and EventTarget

https://codereview.chromium.org/1083003004/diff/1/Source/bindings/core/v8/V8B...
Source/bindings/core/v8/V8Binding.h:348: // Special versions for DOMWindow and
WorkerGlobalScope

and EventTarget

https://codereview.chromium.org/1083003004/diff/1/Source/bindings/tests/idls/...
File Source/bindings/tests/idls/core/TestInterface.idl (right):

https://codereview.chromium.org/1083003004/diff/1/Source/bindings/tests/idls/...
Source/bindings/tests/idls/core/TestInterface.idl:37:
Custom=(LegacyCallAsFunction),

You won't need ().

https://codereview.chromium.org/1083003004/diff/1/Source/bindings/tests/idls/...
File Source/bindings/tests/idls/modules/TestInterface5.idl (right):

https://codereview.chromium.org/1083003004/diff/1/Source/bindings/tests/idls/...
Source/bindings/tests/idls/modules/TestInterface5.idl:39:
Custom=(LegacyCallAsFunction),

You won't need ().

[Yuki, 2015-04-27 12:42:34]

Could you take another look about UNLIKELY?
What do you think of it?

https://codereview.chromium.org/1083003004/diff/1/Source/bindings/core/v8/ToV...
File Source/bindings/core/v8/ToV8.cpp (right):

https://codereview.chromium.org/1083003004/diff/1/Source/bindings/core/v8/ToV...
Source/bindings/core/v8/ToV8.cpp:16: // Notice that we explicitly ignore
creationContext because the DOMWindow is its own creationContext.
On 2015/04/24 11:57:38, haraken wrote:
> 
> is => has

Done.

https://codereview.chromium.org/1083003004/diff/1/Source/bindings/core/v8/ToV...
Source/bindings/core/v8/ToV8.cpp:37: if (UNLIKELY(!impl))
On 2015/04/24 11:57:38, haraken wrote:
> 
> Remove UNLIKELY for consistency.

Well, other versions such as ScriptWrappable* and Node* defined in ToV8.h have
UNLIKELY.  If we consider consistency, I think we should have UNLIKELY here. 
Also I have a concern about performance.  (The code was just copied, and we've
been having this UNLIKELY.  Droppping UNLIKELY may cause performance
regression.)

https://codereview.chromium.org/1083003004/diff/1/Source/bindings/core/v8/ToV...
Source/bindings/core/v8/ToV8.cpp:47: // Notice that we explicitly ignore
creationContext because the WorkerGlobalScope is its own creationContext.
On 2015/04/24 11:57:38, haraken wrote:
> 
> is => has

Done.

https://codereview.chromium.org/1083003004/diff/1/Source/bindings/core/v8/ToV8.h
File Source/bindings/core/v8/ToV8.h (right):

https://codereview.chromium.org/1083003004/diff/1/Source/bindings/core/v8/ToV...
Source/bindings/core/v8/ToV8.h:51: // Special versions for DOMWindow and
WorkerGlobalScope
On 2015/04/24 11:57:38, haraken wrote:
> 
> and EventTarget

Done.

https://codereview.chromium.org/1083003004/diff/1/Source/bindings/core/v8/V8B...
File Source/bindings/core/v8/V8Binding.h (right):

https://codereview.chromium.org/1083003004/diff/1/Source/bindings/core/v8/V8B...
Source/bindings/core/v8/V8Binding.h:220: // Special versions for DOMWindow and
WorkerGlobalScope
On 2015/04/24 11:57:38, haraken wrote:
> 
> and EventTarget

Done.

https://codereview.chromium.org/1083003004/diff/1/Source/bindings/core/v8/V8B...
Source/bindings/core/v8/V8Binding.h:290: // Special versions for DOMWindow and
WorkerGlobalScope
On 2015/04/24 11:57:38, haraken wrote:
> 
> and EventTarget

Done.

https://codereview.chromium.org/1083003004/diff/1/Source/bindings/core/v8/V8B...
Source/bindings/core/v8/V8Binding.h:348: // Special versions for DOMWindow and
WorkerGlobalScope
On 2015/04/24 11:57:38, haraken wrote:
> 
> and EventTarget

Done.

https://codereview.chromium.org/1083003004/diff/1/Source/bindings/tests/idls/...
File Source/bindings/tests/idls/core/TestInterface.idl (right):

https://codereview.chromium.org/1083003004/diff/1/Source/bindings/tests/idls/...
Source/bindings/tests/idls/core/TestInterface.idl:37:
Custom=(LegacyCallAsFunction),
On 2015/04/24 11:57:39, haraken wrote:
> 
> You won't need ().

Done.

https://codereview.chromium.org/1083003004/diff/1/Source/bindings/tests/idls/...
File Source/bindings/tests/idls/modules/TestInterface5.idl (right):

https://codereview.chromium.org/1083003004/diff/1/Source/bindings/tests/idls/...
Source/bindings/tests/idls/modules/TestInterface5.idl:39:
Custom=(LegacyCallAsFunction),
On 2015/04/24 11:57:39, haraken wrote:
> 
> You won't need ().

Done.

[haraken, 2015-04-28 00:56:42]

https://codereview.chromium.org/1083003004/diff/1/Source/bindings/core/v8/ToV...
File Source/bindings/core/v8/ToV8.cpp (right):

https://codereview.chromium.org/1083003004/diff/1/Source/bindings/core/v8/ToV...
Source/bindings/core/v8/ToV8.cpp:37: if (UNLIKELY(!impl))
On 2015/04/27 12:42:33, Yuki wrote:
> On 2015/04/24 11:57:38, haraken wrote:
> > 
> > Remove UNLIKELY for consistency.
> 
> Well, other versions such as ScriptWrappable* and Node* defined in ToV8.h have
> UNLIKELY.  If we consider consistency, I think we should have UNLIKELY here. 
> Also I have a concern about performance.  (The code was just copied, and we've
> been having this UNLIKELY.  Droppping UNLIKELY may cause performance
> regression.)

Sounds reasonable, then let's add UNLIKELY.

[Yuki, 2015-04-28 07:16:13]

The CQ bit was checked by yukishiino@chromium.org

[Yuki, 2015-04-28 07:16:14]

The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/1083003004/#ps60001
(title: "Addressed a review comment.")

[commit-bot: I haz the power, 2015-04-28 07:16:35]

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

[commit-bot: I haz the power, 2015-04-28 09:57:18]

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