[Webkit-unassigned] [Bug 106811] Web Inspector: Add iframe option to inspectedWindow.eval() extension API

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=106811





--- Comment #18 from Andrey Kosyakov <caseq at chromium.org>  2013-01-30 07:48:34 PST ---
(From update of attachment 185286)
View in context: https://bugs.webkit.org/attachment.cgi?id=185286&action=review

> Source/WebCore/inspector/front-end/ExtensionServer.js:348
> +        var status = this.evaluate(message.expression, true, true, message.evaluateOptions, port._extensionOrigin, callback.bind(this));
> +        if (status) {
> +            callback.call(this, "Extension server error: " + String.vsprintf(status.description, status.details));
> +        }

I still think this should not be necessary: the convention is that if the error condition is immediately detect by a handler, the handler returns the error and then ExtensionServer::_onmessage will dispatch the error to the client callback.

> Source/WebCore/inspector/front-end/ExtensionServer.js:795
> +                WebInspector.resourceTreeModel.forAllFrames(function(frame) {

Can we just resturn frames array from ResourceTreeModel. I think it would require fewer callbacks and make code more readable.

> LayoutTests/http/tests/inspector/extensions-eval.html:54
> +function extension_testEvalInIFrame(nextTest) {

Can you instead move it into a separate test file, like we do extensions-eval-content-script? Or, perhaps, just add it to the latter and move it to http?
Also, { needs to be on the next line.

> LayoutTests/http/tests/inspector/extensions-eval.html:55
> +    var options = extension_options();

I'd rather inline both options and loc for better readability, i.e.
var options = {
...
}
eval("window.location.pathname", options, callbacnAndNextTest(...));

> LayoutTests/http/tests/inspector/extensions-eval.html:75
> +    var segments = url.split('/'); // http://a.b.c:xxx/path
> +    var origin = segments[0] + '//' + segments[2];

I think just hard-coding it would make it more readable.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.