[webkit-reviews] review denied: [Bug 19872] Inspector should support cd(window) in the command line : [Attachment 102113] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Yury Semikhatsky
<yurys at chromium.org>'s request for review:
Bug 19872: Inspector should support cd(window) in the command line
https://bugs.webkit.org/show_bug.cgi?id=19872

Attachment 102113: Patch
https://bugs.webkit.org/attachment.cgi?id=102113&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=102113&action=review


> LayoutTests/http/tests/inspector/console-cd.html:11
> +    WebInspector.drawer.immediatelyFinishAnimation();

This is worth a separate InspectorTest.showConsole() function.

> Source/WebCore/inspector/Inspector.json:262
> +		       { "name": "type", "type": "string", "enum": ["frame"],
"description": "Type of the context." },

Context id should not be composite (or at least should be opaque to the
front-end side).

> Source/WebCore/inspector/Inspector.json:275
> +		       { "name": "context", "$ref": "ContextId", "optional":
true, "description": "Specifies in which context to perform evaluation" }

Can we use frame id here instead?

> Source/WebCore/inspector/front-end/ConsoleView.js:195
> +    get _currentEvaluationContextId()

We generally don't do private getters.

> Source/WebCore/inspector/front-end/ContextManager.js:1
> +/*

The name is too generic. JavaScriptWorldManager? How does this correlate with
the workers contexts? DebuggerModel?