[webkit-reviews] review denied: [Bug 133562] Add SPI for Injected Bundle to provide user agent for a given URL : [Attachment 233063] patch

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has denied Grant Kennell <gkennell at apple.com>'s
request for review:
Bug 133562: Add SPI for Injected Bundle to provide user agent for a given URL
https://bugs.webkit.org/show_bug.cgi?id=133562

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

------- Additional Comments from Sam Weinig <sam at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=233063&action=review


>
Source/WebKit2/WebProcess/InjectedBundle/InjectedBundlePageLoaderClient.cpp:39
> +#include <WebCore/FileSystem.h>

Why is this being added?

> Source/WebKit2/WebProcess/InjectedBundle/InjectedBundlePageLoaderClient.h:103

> +    API::String* userAgentForURL(WebFrame*, API::String*);

This should take an API::URL*

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:2285
> +String WebPage::userAgent(WebFrame *frame, const URL& webcoreURL) const

WebFrame *frame -> WebFrame* frame

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:2289
> +	   WKRetainPtr<WKStringRef> string =
adoptWK(m_loaderClient.client().userAgentForURL(toAPI(frame), toAPI(url.get()),
m_loaderClient.client().base.clientInfo));

This should be calling m_loaderClient.userAgentForURL()

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:2290
> +	   API::String *apiString = toImpl(string.get());

API::String *apiString -> API::String* apiString

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:-2291
> -    String userAgent = platformUserAgent(url);
> -    if (!userAgent.isEmpty())
> -	   return userAgent;

Why are you removing this?