Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(102)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 149803005: Remove isolated world parameters from event listener attributes (Closed)

Created:
6 years, 10 months ago by haraken
Modified:
6 years, 10 months ago
CC:
blink-reviews, Nils Barth (inactive), kojih, arv+blink, jsbell+bindings_chromium.org, sof, eae+blinkwatch, abarth-chromium, marja+watch_chromium.org, dglazkov+blink, adamk+blink_chromium.org, haraken, Nate Chapin, watchdog-blink-watchlist_google.com, Raymond Toy, Inactive
Visibility:
Public.

Description

Remove isolated world parameters from event listener attributes Currently, when accessing an event listener attribute, we get the current isolated world in the binding layer, pass it down to the core layer, and then use it to request V8AbstractEventListener that belongs to the same isolated world. However, we don't need to pass the current isolated world around in this way. Alternately, we just need to get the current isolated world when V8AbstractEventListener is requested. This CL implements V8AbstractEventListener::belongToTheCurrentWorld() and removes a bunch of world parameters from the code base. Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=167001

Patch Set 1 #

Total comments: 4

Patch Set 2 : #

Patch Set 3 : #

Patch Set 4 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+63 lines, -72 lines) Patch
M Source/bindings/scripts/code_generator_v8.pm View 1 2 3 chunks +0 lines, -3 lines 0 comments Download
M Source/bindings/tests/results/V8TestInterface.cpp View 1 2 2 chunks +3 lines, -3 lines 0 comments Download
M Source/bindings/tests/results/V8TestInterfaceNode.cpp View 1 2 chunks +2 lines, -2 lines 0 comments Download
M Source/bindings/tests/results/V8TestInterfacePython.cpp View 1 2 2 chunks +3 lines, -3 lines 0 comments Download
M Source/bindings/tests/results/V8TestObject.cpp View 1 2 2 chunks +3 lines, -3 lines 0 comments Download
M Source/bindings/tests/results/V8TestObjectPython.cpp View 1 2 2 chunks +3 lines, -3 lines 0 comments Download
M Source/bindings/v8/V8AbstractEventListener.h View 1 1 chunk +1 line, -0 lines 0 comments Download
M Source/bindings/v8/V8AbstractEventListener.cpp View 1 2 3 1 chunk +5 lines, -0 lines 0 comments Download
M Source/bindings/v8/V8EventListener.h View 1 chunk +0 lines, -1 line 0 comments Download
M Source/core/dom/Document.h View 1 2 2 chunks +2 lines, -3 lines 0 comments Download
M Source/core/dom/Document.cpp View 1 2 1 chunk +4 lines, -4 lines 0 comments Download
M Source/core/dom/MessagePort.h View 1 chunk +3 lines, -3 lines 0 comments Download
M Source/core/events/EventListener.h View 1 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/events/EventTarget.h View 3 chunks +22 lines, -22 lines 0 comments Download
M Source/core/events/EventTarget.cpp View 1 2 chunks +7 lines, -18 lines 0 comments Download
M Source/modules/webaudio/AudioScheduledSourceNode.h View 1 chunk +2 lines, -2 lines 0 comments Download
M Source/modules/webaudio/AudioScheduledSourceNode.cpp View 1 chunk +2 lines, -2 lines 0 comments Download

Messages

Total messages: 37 (0 generated)
haraken
PTAL
6 years, 10 months ago (2014-02-05 11:31:39 UTC) #1
dcarney
lgtm with nit https://codereview.chromium.org/149803005/diff/1/Source/bindings/v8/V8AbstractEventListener.h File Source/bindings/v8/V8AbstractEventListener.h (right): https://codereview.chromium.org/149803005/diff/1/Source/bindings/v8/V8AbstractEventListener.h#newcode112 Source/bindings/v8/V8AbstractEventListener.h:112: virtual bool belongToTheCurrentWorld() const OVERRIDE FINAL; ...
6 years, 10 months ago (2014-02-05 15:29:41 UTC) #2
adamk
lgtm, nice cleanup. https://codereview.chromium.org/149803005/diff/1/Source/core/events/EventListener.h File Source/core/events/EventListener.h (right): https://codereview.chromium.org/149803005/diff/1/Source/core/events/EventListener.h#newcode46 Source/core/events/EventListener.h:46: virtual bool belongToTheCurrentWorld() const { return ...
6 years, 10 months ago (2014-02-05 17:05:08 UTC) #3
haraken
The CQ bit was checked by haraken@chromium.org
6 years, 10 months ago (2014-02-06 01:55:31 UTC) #4
haraken
https://codereview.chromium.org/149803005/diff/1/Source/bindings/v8/V8AbstractEventListener.h File Source/bindings/v8/V8AbstractEventListener.h (right): https://codereview.chromium.org/149803005/diff/1/Source/bindings/v8/V8AbstractEventListener.h#newcode112 Source/bindings/v8/V8AbstractEventListener.h:112: virtual bool belongToTheCurrentWorld() const OVERRIDE FINAL; On 2014/02/05 15:29:42, ...
6 years, 10 months ago (2014-02-06 01:55:52 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/haraken@chromium.org/149803005/100001
6 years, 10 months ago (2014-02-06 01:55:59 UTC) #6
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 10 months ago (2014-02-06 06:28:23 UTC) #7
commit-bot: I haz the power
Retried try job too often on mac_blink_rel for step(s) webkit_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_blink_rel&number=17676
6 years, 10 months ago (2014-02-06 06:28:24 UTC) #8
haraken
The CQ bit was checked by haraken@chromium.org
6 years, 10 months ago (2014-02-06 06:28:52 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/haraken@chromium.org/149803005/100001
6 years, 10 months ago (2014-02-06 06:28:59 UTC) #10
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 10 months ago (2014-02-06 08:26:04 UTC) #11
commit-bot: I haz the power
Retried try job too often on mac_blink_rel for step(s) webkit_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_blink_rel&number=17695
6 years, 10 months ago (2014-02-06 08:26:05 UTC) #12
haraken
The CQ bit was checked by haraken@chromium.org
6 years, 10 months ago (2014-02-06 12:13:46 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/haraken@chromium.org/149803005/100001
6 years, 10 months ago (2014-02-06 12:14:11 UTC) #14
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 10 months ago (2014-02-06 13:36:50 UTC) #15
commit-bot: I haz the power
Retried try job too often on linux_blink for step(s) webkit_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blink&number=11287
6 years, 10 months ago (2014-02-06 13:36:51 UTC) #16
haraken
The CQ bit was checked by haraken@chromium.org
6 years, 10 months ago (2014-02-07 00:36:22 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/haraken@chromium.org/149803005/100001
6 years, 10 months ago (2014-02-07 00:36:54 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/haraken@chromium.org/149803005/100001
6 years, 10 months ago (2014-02-07 02:10:30 UTC) #19
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 10 months ago (2014-02-07 05:12:15 UTC) #20
commit-bot: I haz the power
Retried try job too often on mac_blink_rel for step(s) webkit_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_blink_rel&number=17850
6 years, 10 months ago (2014-02-07 05:12:16 UTC) #21
haraken
The CQ bit was checked by haraken@chromium.org
6 years, 10 months ago (2014-02-07 05:32:31 UTC) #22
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/haraken@chromium.org/149803005/100001
6 years, 10 months ago (2014-02-07 05:32:54 UTC) #23
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 10 months ago (2014-02-07 07:43:38 UTC) #24
commit-bot: I haz the power
Retried try job too often on mac_blink_rel for step(s) webkit_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_blink_rel&number=17882
6 years, 10 months ago (2014-02-07 07:43:39 UTC) #25
haraken
The CQ bit was checked by haraken@chromium.org
6 years, 10 months ago (2014-02-10 01:11:06 UTC) #26
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/haraken@chromium.org/149803005/100001
6 years, 10 months ago (2014-02-10 01:11:20 UTC) #27
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 10 months ago (2014-02-10 03:02:05 UTC) #28
commit-bot: I haz the power
Retried try job too often on mac_blink_rel for step(s) webkit_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_blink_rel&number=18077
6 years, 10 months ago (2014-02-10 03:02:06 UTC) #29
haraken
The CQ bit was checked by haraken@chromium.org
6 years, 10 months ago (2014-02-12 06:55:43 UTC) #30
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/haraken@chromium.org/149803005/1110001
6 years, 10 months ago (2014-02-12 06:56:20 UTC) #31
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 10 months ago (2014-02-12 09:06:27 UTC) #32
commit-bot: I haz the power
Retried try job too often on mac_blink_rel for step(s) webkit_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_blink_rel&number=18384
6 years, 10 months ago (2014-02-12 09:06:28 UTC) #33
haraken
The CQ bit was checked by haraken@chromium.org
6 years, 10 months ago (2014-02-12 10:38:45 UTC) #34
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/haraken@chromium.org/149803005/1280001
6 years, 10 months ago (2014-02-12 10:38:51 UTC) #35
commit-bot: I haz the power
Change committed as 167001
6 years, 10 months ago (2014-02-12 11:50:19 UTC) #36
Nils Barth (inactive)
6 years, 10 months ago (2014-02-13 03:37:53 UTC) #37
Message was sent while issue was closed. 
Python side:
https://codereview.chromium.org/162583002/

Powered by Google App Engine
This is Rietveld 408576698