Move toEventTarget to EventTarget

Move toEventTarget to EventTarget

For WrapperTypeInfo to move to platform/, it cannot have dependencies to core. This patch removes a dependency to EventTarget.

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

[adithyas, 2017-02-08 21:20:27]

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

[commit-bot: I haz the power, 2017-02-08 21:21:06]

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-02-09 01:28:00]

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

[commit-bot: I haz the power, 2017-02-09 01:28:00]

Dry run: This issue passed the CQ dry run.

[adithyas, 2017-02-09 15:02:40]

Description was changed from

==========
Move toEventTarget to EventTarget

BUG=
==========

to

==========
Move toEventTarget to EventTarget

For WrapperTypeInfo to move to platform/, it cannot have dependencies to core.
This patch removes a dependency to EventTarget.

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

[adithyas, 2017-02-10 15:23:56]

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

[jbroman, 2017-02-10 16:03:16]

https://codereview.chromium.org/2683993003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/DictionaryHelperForCore.cpp
(right):

https://codereview.chromium.org/2683993003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/DictionaryHelperForCore.cpp:239:
value = nullptr;
The rest of this method can be rewritten using V8Binding.h as:

value = toEventTarget(dictionary.isolate(), value);
return true;

(Besides, I don't think this method is actually used, as the code that I'm aware
of that used to use it has moved to dictionaries generated from IDL. But it's
probably better to keep it around for future-proofing, I guess.)

https://codereview.chromium.org/2683993003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/V8Binding.cpp (right):

https://codereview.chromium.org/2683993003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/V8Binding.cpp:802: return
EventTarget::toEventTarget(object);
Given that, you can inline the other toEventTarget here, as it becomes the sole
call site (at which point the hasInstance and the eventTargetInheritance bit are
redundant). So after inlining that here, you won't need any changes to
EventTarget.h and EventTarget.cpp.

If that all seems reasonable, we might be able to remove the
eventTargetInheritance bit from WrapperTypeInfo in a followup CL.

[jbroman, 2017-02-10 16:04:27]

https://codereview.chromium.org/2683993003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/V8Binding.cpp (right):

https://codereview.chromium.org/2683993003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/V8Binding.cpp:799: return
static_cast<EventTarget*>(window);
nit: While you're here, you could remove this static_cast (it's a little weird
to use static_cast to upcast). But that's not really part of this CL.

[jbroman, 2017-02-10 16:04:28]

https://codereview.chromium.org/2683993003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/V8Binding.cpp (right):

https://codereview.chromium.org/2683993003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/V8Binding.cpp:799: return
static_cast<EventTarget*>(window);
nit: While you're here, you could remove this static_cast (it's a little weird
to use static_cast to upcast). But that's not really part of this CL.

[adithyas, 2017-02-10 18:39:49]

https://codereview.chromium.org/2683993003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/V8Binding.cpp (right):

https://codereview.chromium.org/2683993003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/V8Binding.cpp:799: return
static_cast<EventTarget*>(window);
On 2017/02/10 at 16:04:27, jbroman wrote:
> nit: While you're here, you could remove this static_cast (it's a little weird
to use static_cast to upcast). But that's not really part of this CL.

Removed it.

https://codereview.chromium.org/2683993003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/V8Binding.cpp:802: return
EventTarget::toEventTarget(object);
On 2017/02/10 at 16:03:16, jbroman wrote:
> Given that, you can inline the other toEventTarget here, as it becomes the
sole call site (at which point the hasInstance and the eventTargetInheritance
bit are redundant). So after inlining that here, you won't need any changes to
EventTarget.h and EventTarget.cpp.
> 
> If that all seems reasonable, we might be able to remove the
eventTargetInheritance bit from WrapperTypeInfo in a followup CL.

Done! I'll remove the wrapperType eventTargetInheritance check (and the field
itself) in a separate CL.

[jbroman, 2017-02-10 19:00:26]

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

[commit-bot: I haz the power, 2017-02-10 19:01:13]

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

[jbroman, 2017-02-10 19:44:11]

lgtm

[commit-bot: I haz the power, 2017-02-10 22:35:08]

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

[commit-bot: I haz the power, 2017-02-10 22:35:09]

Dry run: This issue passed the CQ dry run.

[jbroman, 2017-02-11 01:28:34]

The CQ bit was checked by jbroman@chromium.org

[commit-bot: I haz the power, 2017-02-11 01:29:17]

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-02-11 01:34:47]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1486776514324300,
"parent_rev": "94eae2e63176106d8d7dac115c6de36b6a9563a5", "commit_rev":
"1d105d421b8d7468036f5777d8b7c9de45ce7463"}

[commit-bot: I haz the power, 2017-02-11 01:35:42]

Description was changed from

==========
Move toEventTarget to EventTarget

For WrapperTypeInfo to move to platform/, it cannot have dependencies to core.
This patch removes a dependency to EventTarget.

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

to

==========
Move toEventTarget to EventTarget

For WrapperTypeInfo to move to platform/, it cannot have dependencies to core.
This patch removes a dependency to EventTarget.

BUG=682322

Review-Url: https://codereview.chromium.org/2683993003
Cr-Commit-Position: refs/heads/master@{#449820}
Committed:
https://chromium.googlesource.com/chromium/src/+/1d105d421b8d7468036f5777d8b7...
==========

[commit-bot: I haz the power, 2017-02-11 01:35:43]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/1d105d421b8d7468036f5777d8b7...

[jbroman, 2017-02-11 02:23:15]

On 2017/02/11 at 01:35:43, commit-bot wrote:
> Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/1d105d421b8d7468036f5777d8b7...

Err, I should have asked you to update the description before I CQ-ed this.
Whoops.

[haraken, 2017-02-11 09:42:19]

LGTM

[Yuki, 2017-02-13 05:44:19]

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

[Yuki, 2017-02-13 05:44:20]

LGTM.