Bug 85612 - Web Inspector: console should allow JS execution in the context of an isolated world
: Web Inspector: console should allow JS execution in the context of an isolate...
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated)
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To: Yury Semikhatsky
:
Depends on: 85617 85621 85708
Blocks: 85709
  Show dependency treegraph
 
Reported: 2012-05-04 05:58 PDT by Yury Semikhatsky
Modified: 2012-05-11 02:54 PDT (History)
14 users (show)

See Also:


Attachments
Patch (45.09 KB, patch)
2012-05-04 06:06 PDT, Yury Semikhatsky
no flags Details | Formatted Diff | Diff
Patch (50.80 KB, patch)
2012-05-05 08:03 PDT, Yury Semikhatsky
pfeldman: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yury Semikhatsky 2012-05-04 05:58:23 PDT
Let's make it possible to use the console to execute JavaScript in the context of a content script embedded in the page. 

This is an upstream for Chromium issue: http://code.google.com/p/chromium/issues/detail?id=95865
Comment 1 Yury Semikhatsky 2012-05-04 06:06:43 PDT
Created attachment 140200 [details]
Patch
Comment 2 Early Warning System Bot 2012-05-04 06:27:20 PDT
Comment on attachment 140200 [details]
Patch

Attachment 140200 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12627111
Comment 3 Early Warning System Bot 2012-05-04 06:32:54 PDT
Comment on attachment 140200 [details]
Patch

Attachment 140200 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/12627112
Comment 4 Build Bot 2012-05-04 06:33:37 PDT
Comment on attachment 140200 [details]
Patch

Attachment 140200 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12626114
Comment 5 Build Bot 2012-05-04 06:34:27 PDT
Comment on attachment 140200 [details]
Patch

Attachment 140200 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12631144
Comment 6 Pavel Feldman 2012-05-04 06:38:00 PDT
Comment on attachment 140200 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=140200&action=review

> Source/WebCore/bindings/js/ScriptController.cpp:332
> +    return 0;

Please file a bug and insert FIXME here.

> Source/WebCore/bindings/v8/V8Proxy.cpp:637
> +        return v8::Local<v8::Context>();

Can this happen?

> Source/WebCore/bindings/v8/V8Proxy.h:255
> +#if ENABLE(INSPECTOR)

Chrome(i)um does not compile without inspector.

> Source/WebCore/inspector/CodeGeneratorInspector.py:188
> +    skip_js_bind_domains = set(["DOMDebugger"])

What does it mean?

> Source/WebCore/inspector/Inspector.json:432
> +                "id": "IsolatedContext",

EvaluationContext ?

> Source/WebCore/inspector/Inspector.json:436
> +                    { "name": "id", "type": "integer", "description": "World id. It can be used to specify in which world script evaluation should be performed." },

I would rename it to "contextId" or type "ContextId".

> Source/WebCore/inspector/Inspector.json:438
> +                    { "name": "frameId", "$ref": "Network.FrameId", "description": "Id of the owning frame." }

reorder by importance?

> Source/WebCore/inspector/Inspector.json:453
> +                    { "name": "contextId", "type": "integer", "optional": true, "description": "Specifies in which isolated context to perform evaluation. Each content script lives in an isolated context and this parameter may be used to specify on of those contexts. If the parameter is omitted or 0 the evaluation will be performed in the context of the inspected page.", "hidden": true },

I think it is better to combine the frame and context id into the single contextId and stick to that term in runtime / debugger.
Comment 7 Yury Semikhatsky 2012-05-05 07:50:15 PDT
(In reply to comment #6)
> (From update of attachment 140200 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=140200&action=review
> 
> > Source/WebCore/bindings/js/ScriptController.cpp:332
> > +    return 0;
> 
> Please file a bug and insert FIXME here.
> 
Filed https://bugs.webkit.org/show_bug.cgi?id=85709



> > Source/WebCore/bindings/v8/V8Proxy.cpp:637
> > +        return v8::Local<v8::Context>();
> 
> Can this happen?
> 
Removed that code.


> > Source/WebCore/bindings/v8/V8Proxy.h:255
> > +#if ENABLE(INSPECTOR)
> 
> Chrome(i)um does not compile without inspector.
> 
Removed.


> > Source/WebCore/inspector/CodeGeneratorInspector.py:188
> > +    skip_js_bind_domains = set(["DOMDebugger"])
> 
> What does it mean?
> 
It is a list of domains for which we don't generate register<Domain>Dispatcher() methods in the front-end.


> > Source/WebCore/inspector/Inspector.json:432
> > +                "id": "IsolatedContext",
> 
> EvaluationContext ?
> 
Renamed to ExecutionContextDescription, introduced ExecutionContextId.


> > Source/WebCore/inspector/Inspector.json:436
> > +                    { "name": "id", "type": "integer", "description": "World id. It can be used to specify in which world script evaluation should be performed." },
> 
> I would rename it to "contextId" or type "ContextId".
> 
Done ExecutionContextId.

> > Source/WebCore/inspector/Inspector.json:438
> > +                    { "name": "frameId", "$ref": "Network.FrameId", "description": "Id of the owning frame." }
> 
> reorder by importance?
> 
Done.

> > Source/WebCore/inspector/Inspector.json:453
> > +                    { "name": "contextId", "type": "integer", "optional": true, "description": "Specifies in which isolated context to perform evaluation. Each content script lives in an isolated context and this parameter may be used to specify on of those contexts. If the parameter is omitted or 0 the evaluation will be performed in the context of the inspected page.", "hidden": true },
> 
> I think it is better to combine the frame and context id into the single contextId and stick to that term in runtime / debugger.
Done.
Comment 8 Yury Semikhatsky 2012-05-05 08:03:19 PDT
Created attachment 140401 [details]
Patch
Comment 9 Yury Semikhatsky 2012-05-11 02:54:41 PDT
Committed r116744: <http://trac.webkit.org/changeset/116744>