[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 #21 from johnjbarton <johnjbarton at chromium.org>  2013-02-04 12:31:53 PST ---
(In reply to comment #20)
> (In reply to comment #19)
> > (In reply to comment #18)
> > > (From update of attachment 185286 [details] [details] [details])
> > > 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.
> > 
> > But without this change the handler never sees the error condition! If evaluate() returns say, E_NOTFOUND, and we ignore the return value (as without this patch) how can _onMessage dispatch an error to the client? It does not even see the error. 
> > 
> 
> Here's the code that is responsible to dispatching the error to the client:
> 
> http://code.google.com/searchframe#OAMlx_jo-ck/src/third_party/WebKit/Source/WebCore/inspector/front-end/ExtensionServer.js&exact_package=chromium&q=onmessage%20file:extensionserver&l=724
> 
> (lines 724, 728 & 729)
> 
> You can hit it in existing code if you tweak one of the tests (duh -- we need a test for this as well). If you changle file:/// to something else in this line, you will see that the callback is invoked even upon error:
> 
> http://code.google.com/searchframe#OAMlx_jo-ck/src/third_party/WebKit/LayoutTests/inspector/extensions/extensions-eval-content-script.html&exact_package=chromium&q=file:extensions-eval-&type=cs&l=9
> 
> That said, the way we report these errors to the client is quite unfortunate. I'm about to fix that, see bug 108640.

In bug 108846 I added a test for a bad option, that is the dev sends useContentScriptContext option in a case where there is not content script world.  I hope that patch will clarify the error return issue.

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