[webkit-reviews] review denied: [Bug 49836] Add WebKit2 API relevant to customization of context menus : [Attachment 74428] Patch v1

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Brady Eidson <beidson at apple.com>'s
request for review:
Bug 49836: Add WebKit2 API relevant to customization of context menus
https://bugs.webkit.org/show_bug.cgi?id=49836

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=74428&action=review

review- because of the leak

> WebKit2/Shared/WebURLRequest.h:47
> +    static PassRefPtr<WebURLRequest> create(const WebCore::KURL& url);

Should leave argument name out here.

> WebKit2/Shared/API/c/WKURLRequest.cpp:40
> +    return toAPI(WebURLRequest::create(toImpl(url)->string()).leakRef());

Can you use toCopiedURLAPI here?

> WebKit2/WebProcess/WebPage/mac/WebPageMac.mm:199
> +    if (equalIgnoringCase(url.protocol(), "file"))
> +	   return true;

You should use the isLocalFile member function.

> WebKit2/WebProcess/WebPage/mac/WebPageMac.mm:206
> +    NSMutableURLRequest* request = [[NSMutableURLRequest alloc]
initWithURL:url];

You need to release this request, otherwise it will leak.

> WebKit2/WebProcess/WebPage/mac/WebPageMac.mm:207
> +    [request setValue:(NSString*)(userAgent())
forHTTPHeaderField:@"User-Agent"];

Extra parentheses here around userAgent().

> WebKit2/WebProcess/WebPage/mac/WebPageMac.mm:219
> +    String scheme = request.url().protocol();
> +    // FIXME: Return true if this scheme is any one WebKit2 knows how to
handle.
> +    return equalIgnoringCase(scheme, "applewebdata");

This should use the protocolIs.