Issue 2492793002: Support fetching attribute listeners from outside v8 context scopes.

Issue 2492793002: Support fetching attribute listeners from outside v8 context scopes. (Closed)

Created:
4 years, 1 month ago by sof

Modified:
4 years, 1 month ago

Reviewers:
haraken, Yuki

CC:
chromium-reviews, blink-reviews, blink-reviews-bindings_chromium.org

Target Ref:
refs/pending/heads/master

Project:
chromium

Visibility:
Public.

More Reviews

Description

Support fetching attribute listeners from outside v8 context scopes. A number of the <body> element's event handler attributes represent and expose event handlers on the window object, hence the parser will update & replace attribute event listeners while parsing the attributes. This may well happen while executing outside any v8 context; adjust the lookup of attribute event listeners to support such usage. R=haraken BUG=659911 Committed: https://crrev.com/05c2d71f2c3cab4c40c3d775b304d0bbfbd166bd Cr-Commit-Position: refs/heads/master@{#431509}

Patch Set 1 #

Patch Set 2 : add test #

Patch Set 3 : Sync expected output for wpt/ test #

Patch Set 4 : consistent js indentation #

Patch Set 5 : try to tidy up logic #

Total comments: 2

Created: 4 years, 1 month ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+53 lines, -7 lines)			Patch
A	third_party/WebKit/LayoutTests/fast/events/onload-body-replace.html	View	1 2 3	1 chunk	+28 lines, -0 lines	0 comments	Download
A	third_party/WebKit/LayoutTests/fast/events/onload-body-replace-expected.txt	View	1	1 chunk	+2 lines, -0 lines	0 comments	Download
M	third_party/WebKit/LayoutTests/imported/wpt/html/webappapis/scripting/processing-model-2/compile-error-in-body-onerror-expected.txt	View	1 2	1 chunk	+1 line, -1 line	0 comments	Download
M	third_party/WebKit/Source/bindings/core/v8/V8AbstractEventListener.h	View	1 2 3 4	1 chunk	+1 line, -1 line	0 comments	Download
M	third_party/WebKit/Source/bindings/core/v8/V8AbstractEventListener.cpp	View	1 2 3 4	2 chunks	+15 lines, -3 lines	2 comments	Download
M	third_party/WebKit/Source/core/events/EventListener.h	View	1 2 3 4	1 chunk	+3 lines, -1 line	0 comments	Download
M	third_party/WebKit/Source/core/events/EventTarget.cpp	View	1 2 3 4	1 chunk	+3 lines, -1 line	0 comments	Download

Messages

Total messages: 32 (21 generated)

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages

sof

please take a look. cleanest i could think of, perhaps you have simpler suggestions?

4 years, 1 month ago (2016-11-10 15:30:21 UTC) #12

haraken

On 2016/11/10 15:30:21, sof wrote: > please take a look. > > cleanest i could ...

4 years, 1 month ago (2016-11-10 16:07:46 UTC) #13

sof

On 2016/11/10 16:07:46, haraken wrote: > On 2016/11/10 15:30:21, sof wrote: > > please take ...

4 years, 1 month ago (2016-11-10 21:59:44 UTC) #19

haraken

LGTM https://codereview.chromium.org/2492793002/diff/80001/third_party/WebKit/Source/bindings/core/v8/V8AbstractEventListener.cpp File third_party/WebKit/Source/bindings/core/v8/V8AbstractEventListener.cpp (right): https://codereview.chromium.org/2492793002/diff/80001/third_party/WebKit/Source/bindings/core/v8/V8AbstractEventListener.cpp#newcode211 third_party/WebKit/Source/bindings/core/v8/V8AbstractEventListener.cpp:211: if (!isolate()->InContext() && executionContext->isDocument()) { Maybe the following ...

4 years, 1 month ago (2016-11-11 03:54:40 UTC) #22

sof

https://codereview.chromium.org/2492793002/diff/80001/third_party/WebKit/Source/bindings/core/v8/V8AbstractEventListener.cpp File third_party/WebKit/Source/bindings/core/v8/V8AbstractEventListener.cpp (right): https://codereview.chromium.org/2492793002/diff/80001/third_party/WebKit/Source/bindings/core/v8/V8AbstractEventListener.cpp#newcode211 third_party/WebKit/Source/bindings/core/v8/V8AbstractEventListener.cpp:211: if (!isolate()->InContext() && executionContext->isDocument()) { On 2016/11/11 03:54:40, haraken ...

4 years, 1 month ago (2016-11-11 06:27:04 UTC) #23

haraken

On 2016/11/11 06:27:04, sof wrote: > https://codereview.chromium.org/2492793002/diff/80001/third_party/WebKit/Source/bindings/core/v8/V8AbstractEventListener.cpp > File third_party/WebKit/Source/bindings/core/v8/V8AbstractEventListener.cpp > (right): > > https://codereview.chromium.org/2492793002/diff/80001/third_party/WebKit/Source/bindings/core/v8/V8AbstractEventListener.cpp#newcode211 ...

4 years, 1 month ago (2016-11-11 06:27:33 UTC) #25

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2492793002/80001

4 years, 1 month ago (2016-11-11 06:29:59 UTC) #27

commit-bot: I haz the power

Patchset 5 (id:??) landed as https://crrev.com/05c2d71f2c3cab4c40c3d775b304d0bbfbd166bd Cr-Commit-Position: refs/heads/master@{#431509}

4 years, 1 month ago (2016-11-11 06:39:21 UTC) #31

Message was sent while issue was closed.

lgtm

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages