Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(1)

Issue 1615523002: Transitively keep track of an isolated world's children scripts and worlds. (Closed)

Created:
3 years, 5 months ago by proberge
Modified:
3 years, 3 months ago
CC:
blink-reviews, blink-reviews-bindings_chromium.org, blink-reviews-dom_chromium.org, chromium-reviews, dglazkov+blink, eae+blinkwatch, gavinp+loader_chromium.org, Nate Chapin, loading-reviews_chromium.org, rwlbuis, sof, tyoshino+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Transitively keep track of an isolated world's children scripts and worlds. In blink, scripts from extensions run in an isolated world. When new scripts or script fragments are created by the extension, transitively keep track of the fact that an extension is responsible through the originWorld. BUG=579710

Patch Set 1 #

Total comments: 10

Patch Set 2 : Use a static world stack instead of a per-world private field #

Unified diffs Side-by-side diffs Delta from patch set Stats (+84 lines, -0 lines) Patch
M third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.h View 1 4 chunks +44 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.cpp View 1 1 chunk +7 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/bindings/core/v8/V8AbstractEventListener.h View 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/bindings/core/v8/V8AbstractEventListener.cpp View 1 2 chunks +4 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/ScriptLoader.h View 1 2 chunks +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/ScriptLoader.cpp View 1 3 chunks +9 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/frame/DOMTimer.h View 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/frame/DOMTimer.cpp View 1 2 chunks +6 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/loader/NavigationScheduler.cpp View 1 2 chunks +9 lines, -0 lines 0 comments Download

Messages

Total messages: 24 (3 generated)
proberge
3 years, 5 months ago (2016-01-20 22:40:26 UTC) #3
haraken
Thanks for splitting the CL! https://codereview.chromium.org/1615523002/diff/1/third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.h File third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.h (right): https://codereview.chromium.org/1615523002/diff/1/third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.h#newcode83 third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.h:83: DOMWrapperWorld& m_currentWorld; I'm wondering ...
3 years, 5 months ago (2016-01-21 10:16:15 UTC) #4
Joe Mason
https://codereview.chromium.org/1615523002/diff/1/third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.h File third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.h (right): https://codereview.chromium.org/1615523002/diff/1/third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.h#newcode83 third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.h:83: DOMWrapperWorld& m_currentWorld; On 2016/01/21 10:16:15, haraken wrote: > > ...
3 years, 5 months ago (2016-01-21 16:13:50 UTC) #5
haraken
On 2016/01/21 16:13:50, joenotcharles wrote: > https://codereview.chromium.org/1615523002/diff/1/third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.h > File third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.h (right): > > https://codereview.chromium.org/1615523002/diff/1/third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.h#newcode83 > ...
3 years, 5 months ago (2016-01-21 16:41:33 UTC) #6
proberge
On 2016/01/21 16:41:33, haraken wrote: > On 2016/01/21 16:13:50, joenotcharles wrote: > > > https://codereview.chromium.org/1615523002/diff/1/third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.h ...
3 years, 5 months ago (2016-01-21 16:45:26 UTC) #7
haraken
On 2016/01/21 16:45:26, proberge wrote: > On 2016/01/21 16:41:33, haraken wrote: > > On 2016/01/21 ...
3 years, 5 months ago (2016-01-21 16:51:42 UTC) #8
haraken
On 2016/01/21 16:51:42, haraken wrote: > On 2016/01/21 16:45:26, proberge wrote: > > On 2016/01/21 ...
3 years, 5 months ago (2016-01-21 17:38:46 UTC) #9
proberge
On 2016/01/21 17:38:46, haraken wrote: > On 2016/01/21 16:51:42, haraken wrote: > > On 2016/01/21 ...
3 years, 5 months ago (2016-01-21 21:58:42 UTC) #10
haraken
On 2016/01/21 21:58:42, proberge wrote: > On 2016/01/21 17:38:46, haraken wrote: > > On 2016/01/21 ...
3 years, 5 months ago (2016-01-21 22:46:10 UTC) #11
proberge
On 2016/01/21 22:46:10, haraken wrote: > On 2016/01/21 21:58:42, proberge wrote: > > On 2016/01/21 ...
3 years, 5 months ago (2016-01-22 18:59:18 UTC) #12
haraken
> Even ignoring the frame case, using a single static member doesn't seem to be ...
3 years, 5 months ago (2016-01-22 20:30:59 UTC) #13
proberge
On 2016/01/22 20:30:59, haraken wrote: > > Even ignoring the frame case, using a single ...
3 years, 5 months ago (2016-01-27 18:33:15 UTC) #14
haraken
On 2016/01/27 18:33:15, proberge wrote: > On 2016/01/22 20:30:59, haraken wrote: > > > Even ...
3 years, 5 months ago (2016-01-28 08:14:09 UTC) #15
proberge
On 2016/01/28 08:14:09, haraken wrote: > On 2016/01/27 18:33:15, proberge wrote: > > On 2016/01/22 ...
3 years, 5 months ago (2016-01-28 22:41:30 UTC) #16
haraken
On 2016/01/28 22:41:30, proberge wrote: > On 2016/01/28 08:14:09, haraken wrote: > > On 2016/01/27 ...
3 years, 5 months ago (2016-01-29 05:00:47 UTC) #17
proberge
On 2016/01/29 05:00:47, haraken wrote: > On 2016/01/28 22:41:30, proberge wrote: > > On 2016/01/28 ...
3 years, 5 months ago (2016-01-29 17:59:51 UTC) #18
proberge
https://codereview.chromium.org/1615523002/diff/1/third_party/WebKit/Source/bindings/core/v8/V8AbstractEventListener.cpp File third_party/WebKit/Source/bindings/core/v8/V8AbstractEventListener.cpp (right): https://codereview.chromium.org/1615523002/diff/1/third_party/WebKit/Source/bindings/core/v8/V8AbstractEventListener.cpp#newcode49 third_party/WebKit/Source/bindings/core/v8/V8AbstractEventListener.cpp:49: , m_originWorld(PassRefPtr<DOMWrapperWorld>(world.originWorld())) On 2016/01/21 10:16:15, haraken wrote: > > ...
3 years, 5 months ago (2016-01-29 18:00:48 UTC) #19
esprehn
I don't think this works, if you append an iframe it'll run script in the ...
3 years, 5 months ago (2016-01-30 11:01:59 UTC) #21
haraken
On 2016/01/30 11:01:59, esprehn wrote: > I don't think this works, if you append an ...
3 years, 5 months ago (2016-01-30 14:25:39 UTC) #22
esprehn
ex. content script does: var s = document.createElement('script'); s.textContent = "Promise.resolve().then(() => { "var xhr ...
3 years, 5 months ago (2016-02-10 01:55:35 UTC) #23
proberge
3 years, 5 months ago (2016-02-10 17:47:41 UTC) #24
On 2016/02/10 01:55:35, esprehn wrote:
> ex.
> 
> content script does:
> 
> var s = document.createElement('script');
> s.textContent = 
>   "Promise.resolve().then(() => {
>     "var xhr = new XMLHttpRequest(); " +
>     "xhr.open('GET', 'data:text/html,<div>'); " +
>     "xhr.responseType = 'document'; " +
>     "xhr.onload = () => { document.body.appendChild(xhr.response); }"
>   "}";
> document.body.appendChild(s);
> 
> you could also use a MutationObserver, or many other ways to run async scripts
> outside your scope. You could also probably use srcdoc which gets parsed
async.

The promise + xhr + onload example seems to be covered by the
V8AbstractEventListener scope. (the xhr.onload event listener created during the
MicroTask does get the isolated origin world).
However, MutationObserver indeed appears to not be covered by the code and would
require adding the private field and OriginWorldScope code to
V8MutationCallback.

I understand that we want to avoid adding this complexity to every single way to
run async scripts. Do you know of a place in the script execution chain that
"narrows" enough to cover most cases? As a bad example, we could have each
v8::Function keep track of which world created it.

Powered by Google App Engine
This is Rietveld 408576698