[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 #16 from johnjbarton <johnjbarton at chromium.org>  2013-01-29 12:23:15 PST ---
(In reply to comment #15)
> (From update of attachment 185025 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=185025&action=review
> 
> > Source/WebCore/ChangeLog:91
> >          (WebCore::RenderBox::exclusionShapeOutsideInfo): Ditto.
> >  
> > +        * inspector/front-end/ExtensionServer.js:
> 
> Looks like a merge error.

I don't really understand the ChangeLog entry gets created by webkit-patch upload. I'll try to get it to start over.

> 
> > 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));
> > +        }
> 
> Why is this necessary? 

On error, this.evaluate() doe not callback() but returns errors, eg 
   return this._status.E_NOTFOUND(contextSecurityOrigin);
If the caller does not process the return value, these error messages are lost, and the callback never fires.

> I don't think this will pass type validation by closure compiler.

I corrected the type signature of the callback.

> 
> > Source/WebCore/inspector/front-end/ExtensionServer.js:794
> > +            } else if (options.scriptExecutionContext) {
> 
> Please drop redundant braces.

done

> 
> > Source/WebCore/inspector/front-end/ExtensionServer.js:797
> > +            var url;
> 
> Redundant?
yup

The next patch is quite different for the rest of the comments.

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