[webkit-reviews] review denied: [Bug 51090] WebKit2: WebPageWin needs implementations of hasLocalDataForURL and canHandleRequest : [Attachment 76622] [PATCH] Fix

[bugzilla-daemon at webkit.org]

Adam Roben (aroben) <aroben at apple.com> has denied Brian Weinstein
<bweinstein at apple.com>'s request for review:
Bug 51090: WebKit2: WebPageWin needs implementations of hasLocalDataForURL and
canHandleRequest
https://bugs.webkit.org/show_bug.cgi?id=51090

Attachment 76622: [PATCH] Fix
https://bugs.webkit.org/attachment.cgi?id=76622&action=review

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=76622&action=review

> WebKit2/ChangeLog:14
> +	   (WebKit::WebPage::hasLocalDataForURL): See if the URL is a file URL,
then see if it's in the WebCore cache,
> +	       and then fall back to the CFURL cache.
> +	   (WebKit::WebPage::canHandleRequest): Return the value of
CFURLProtocolCanHandleRequest and add a FIXME
> +	       for improving this function if needed.

Did this code come from somewhere else? You should mention it if so.

> WebKit2/WebProcess/WebPage/win/WebPageWin.cpp:256
> +    if (url.isLocalFile())
> +	   return true;
> +
> +    FrameLoader* frameLoader = m_page->mainFrame()->loader();
> +    DocumentLoader* documentLoader = frameLoader ?
frameLoader->documentLoader() : 0;
> +    if (documentLoader && documentLoader->subresource(url))
> +	   return true;

Seems like this code could all be cross-platform. Why is it sitting in this
platform-specific file?

> WebKit2/WebProcess/WebPage/win/WebPageWin.cpp:258
> +    RetainPtr<CFMutableURLRequestRef> request(AdoptCF,
CFURLRequestCreateMutable(0, url.createCFURL(),
kCFURLRequestCachePolicyReloadIgnoringCache, 60, 0));

Looks like you're leaking the CFURL here.

> WebKit2/WebProcess/WebPage/win/WebPageWin.cpp:259
> +    CFURLRequestSetHTTPHeaderFieldValue(request.get(), CFSTR("User-Agent"),
userAgent().createCFString());

Looks like you're leaking the CFString here.