Split V8Binding

Split V8Binding

This CL splits V8Binding into V8Binding.h/cpp (which will be kept in 
platform/bindings and used by files in platform/bindings) and 
V8BindingForCore.h/cpp which will be used everywhere else.

All functions that do not have any dependencies to classes still in core/ and 
bindings/core (eg: ExceptionState, ExecutionContext, LocalDOMWindow,
LocalFrame, etc..) are kept inside V8Binding.h/cpp, the rest are moved to 
V8BindingForCore.h/cpp.

BUG=682322
Review-Url: https://codereview.chromium.org/2834053003
Cr-Commit-Position: refs/heads/master@{#466709}
Committed: https://chromium.googlesource.com/chromium/src/+/d5b4dbdb62c46e7d2617d812b7fa9c5d201261fe

[adithyas, 2017-04-21 17:34:01]

Description was changed from

==========
Split V8Binding

This CL splits V8Binding into V8Binding (which will be kept in platform/bindings
and used by files in platform/bindings) and V8BindingForCore which will be used
everywhere else.

BUG=682322
==========

to

==========
Split V8Binding

This CL splits V8Binding into V8Binding.h (which will be kept in
platform/bindings and used by files in platform/bindings) and
V8BindingForCore.h/cpp which will be used everywhere else.

BUG=682322
==========

[adithyas, 2017-04-21 17:39:09]

adithyas@chromium.org changed reviewers:
+ haraken@chromium.org, jbroman@chromium.org

[jbroman, 2017-04-21 18:11:02]

Mind expanding the CL description to describe what criteria determine which end
up where? Use of ExceptionState seems to be the key thing, but it's not 100%
obvious and it's hard to skim a large diff.

Also, please wrap CL description to 80 cols.

https://codereview.chromium.org/2834053003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/V8BindingForCore.h (right):

https://codereview.chromium.org/2834053003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/V8BindingForCore.h:71: inline void
V8SetReturnValue(const CallbackInfo& callback_info, Node* impl) {
These would go away after yukishiino and you land your patches w.r.t. the
special treatment of EventTarget and Node, right?

[adithyas, 2017-04-21 19:13:52]

Description was changed from

==========
Split V8Binding

This CL splits V8Binding into V8Binding.h (which will be kept in
platform/bindings and used by files in platform/bindings) and
V8BindingForCore.h/cpp which will be used everywhere else.

BUG=682322
==========

to

==========
Split V8Binding

This CL splits V8Binding into V8Binding.h/cpp (which will be kept in 
platform/bindings and used by files in platform/bindings) and 
V8BindingForCore.h/cpp which will be used everywhere else.

All functions that do not have any dependencies to classes still in core/ and 
bindings/core (eg: ExceptionState, ExecutionContext, LocalDOMWindow,
LocalFrame, etc..) are kept inside V8Binding.h/cpp, the rest are moved to 
V8BindingForCore.h/cpp.

BUG=682322
==========

[adithyas, 2017-04-21 19:19:46]

I made some changes and moved some functions back to V8Binding.h/cpp (I had
forgotten to revert an earlier commit). 

I've tried to keep all the functions that currently can be moved to
platform/bindings inside V8Binding (and moved the rest to V8BindingForCore). So
yes, ExceptionState is a major factor for a lot of the functions being moved to
V8BindingForCore, but not the only one. I've updated the CL description to
reflect that.

https://codereview.chromium.org/2834053003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/V8BindingForCore.h (right):

https://codereview.chromium.org/2834053003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/V8BindingForCore.h:71: inline void
V8SetReturnValue(const CallbackInfo& callback_info, Node* impl) {
On 2017/04/21 at 18:11:02, jbroman wrote:
> These would go away after yukishiino and you land your patches w.r.t. the
special treatment of EventTarget and Node, right?

Yup, I'm just waiting for a bit to land my patch in case yukishiino@ has to
revert that change.

[adithyas, 2017-04-21 19:19:58]

The CQ bit was checked by adithyas@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-21 19:20:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-21 19:41:55]

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

[commit-bot: I haz the power, 2017-04-21 19:41:57]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jbroman, 2017-04-21 19:42:30]

You seem to have some compile errors. This lgtm, though there's a bit of stuff
we might want to move from one side to another in the future (as there's some
primitive stuff that seems to land on one side or another depending on whether
it can throw an exception, which is kinda weird).

[haraken, 2017-04-21 20:09:41]

LGTM

https://codereview.chromium.org/2834053003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/V8Binding.h (right):

https://codereview.chromium.org/2834053003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/V8Binding.h:50: 

Would you add a comment and mention what should be in V8Binding and what should
be in V8BindingForCore?

https://codereview.chromium.org/2834053003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/V8BindingForCore.h (right):

https://codereview.chromium.org/2834053003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/V8BindingForCore.h:58: 

Ditto.

https://codereview.chromium.org/2834053003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/V8BindingForCore.h:58: 

Ditto.

[adithyas, 2017-04-21 20:10:20]

The CQ bit was checked by adithyas@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-21 20:10:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-21 20:17:18]

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

[commit-bot: I haz the power, 2017-04-21 20:17:20]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[adithyas, 2017-04-21 20:34:00]

The CQ bit was checked by adithyas@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-21 20:34:29]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-21 21:56:48]

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

[commit-bot: I haz the power, 2017-04-21 21:56:50]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[adithyas, 2017-04-24 14:38:25]

The CQ bit was checked by adithyas@chromium.org

[adithyas, 2017-04-24 14:38:27]

https://codereview.chromium.org/2834053003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/V8Binding.h (right):

https://codereview.chromium.org/2834053003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/V8Binding.h:50: 
On 2017/04/21 at 20:09:41, haraken wrote:
> Would you add a comment and mention what should be in V8Binding and what
should be in V8BindingForCore?

Done.

[adithyas, 2017-04-24 14:38:28]

The patchset sent to the CQ was uploaded after l-g-t-m from
jbroman@chromium.org, haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/2834053003/#ps90001
(title: "Rebase")

[commit-bot: I haz the power, 2017-04-24 14:38:53]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-24 19:17:12]

CQ is committing da patch.

Bot data: {"patchset_id": 90001, "attempt_start_ts": 1493044705367290,
"parent_rev": "ac53e53db3b6db49588a3ad69ba4231308091009", "commit_rev":
"d5b4dbdb62c46e7d2617d812b7fa9c5d201261fe"}

[commit-bot: I haz the power, 2017-04-24 19:17:26]

Description was changed from

==========
Split V8Binding

This CL splits V8Binding into V8Binding.h/cpp (which will be kept in 
platform/bindings and used by files in platform/bindings) and 
V8BindingForCore.h/cpp which will be used everywhere else.

All functions that do not have any dependencies to classes still in core/ and 
bindings/core (eg: ExceptionState, ExecutionContext, LocalDOMWindow,
LocalFrame, etc..) are kept inside V8Binding.h/cpp, the rest are moved to 
V8BindingForCore.h/cpp.

BUG=682322
==========

to

==========
Split V8Binding

This CL splits V8Binding into V8Binding.h/cpp (which will be kept in 
platform/bindings and used by files in platform/bindings) and 
V8BindingForCore.h/cpp which will be used everywhere else.

All functions that do not have any dependencies to classes still in core/ and 
bindings/core (eg: ExceptionState, ExecutionContext, LocalDOMWindow,
LocalFrame, etc..) are kept inside V8Binding.h/cpp, the rest are moved to 
V8BindingForCore.h/cpp.

BUG=682322

Review-Url: https://codereview.chromium.org/2834053003
Cr-Commit-Position: refs/heads/master@{#466709}
Committed:
https://chromium.googlesource.com/chromium/src/+/d5b4dbdb62c46e7d2617d812b7fa...
==========

[commit-bot: I haz the power, 2017-04-24 19:17:28]

Committed patchset #6 (id:90001) as
https://chromium.googlesource.com/chromium/src/+/d5b4dbdb62c46e7d2617d812b7fa...