[webkit-reviews] review denied: [Bug 52384] Plumb mixed script URL to FrameLoaderClient : [Attachment 78845] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has denied Adam Langley <agl at chromium.org>'s
request for review:
Bug 52384: Plumb mixed script URL to FrameLoaderClient
https://bugs.webkit.org/show_bug.cgi?id=52384

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

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=78845&action=review

This looks fine, module changing String -> URL and the extra changelog entries.
 It's fine not to plumb through objective C.  That's an API choice for that
port.  We'll also need fishd to review the WebKit API change.

> Tools/ChangeLog:12
> +2011-01-13  Adam Langley  <agl at chromium.org>

You've got two changelog entries.

> Tools/DumpRenderTree/chromium/WebViewHost.h:202
> -    virtual void didRunInsecureContent(WebKit::WebFrame*, const
WebKit::WebSecurityOrigin&);
> +    virtual void didRunInsecureContent(WebKit::WebFrame*, const
WebKit::WebSecurityOrigin&, const WebKit::WebString& target_url);

I tink we use webkit-style names here.

> WebKit/chromium/public/WebFrameClient.h:279
> -    virtual void didRunInsecureContent(WebFrame*, const WebSecurityOrigin&)
{ }
> +    virtual void didRunInsecureContent(WebFrame*, const WebSecurityOrigin&,
const WebString&) { }

As I commented in the Chromium bug, this should probably be a WebURL.