[Webkit-unassigned] [Bug 133562] Add SPI for Injected Bundle to provide user agent for a given URL
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Jun 5 17:18:16 PDT 2014
https://bugs.webkit.org/show_bug.cgi?id=133562
Sam Weinig <sam at webkit.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #232594|review? |review-
Flag| |
--- Comment #3 from Sam Weinig <sam at webkit.org> 2014-06-05 17:18:40 PST ---
(From update of attachment 232594)
View in context: https://bugs.webkit.org/attachment.cgi?id=232594&action=review
> Source/WebKit2/ChangeLog:24
> +Added API for Injected Bundle to provide user agent for a given URL.
> +
> +Need the bug URL (OOPS!).
> +
> +Reviewed by NOBODY (OOPS!).
> +
> +* WebProcess/InjectedBundle/API/Cocoa/WKWebProcessPlugInLoadDelegate.h:
> + Added delegate method to WebProcess PluIn protocol to provide UserAgent per URL.
> +* WebProcess/InjectedBundle/API/c/WKBundlePageLoaderClient.h:
> + Added new typedef for function pointer for this new delegate call
> + and added a function pointer of that type to the struct.
> +* WebProcess/InjectedBundle/API/mac/WKWebProcessPlugInBrowserContextController.mm:
> +(userAgentForURL): Makes delegate call with the new method.
> +(setUpPageLoaderClient): Sets the struct's new function pointer to the new method.
> +* WebProcess/InjectedBundle/InjectedBundlePageLoaderClient.cpp:
> +(WebKit::InjectedBundlePageLoaderClient::userAgentForURL):
> +* WebProcess/InjectedBundle/InjectedBundlePageLoaderClient.h:
> +* WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
> +(WebKit::WebFrameLoaderClient::userAgent):
> +* WebProcess/WebPage/WebPage.cpp:
> +(WebKit::WebPage::userAgent): Began using the new API to ask for user agent
> + instead of simply returning what had been stored.
> +* WebProcess/WebPage/WebPage.h:
> +(WebKit::WebPage::userAgent): Deleted.
The changelog is formatted incorrectly. Please use prepare-changelog for the right way. It also should include the url of this bug.
> Source/WebKit2/WebProcess/InjectedBundle/API/Cocoa/WKWebProcessPlugInLoadDelegate.h:59
> +- (NSString *)webProcessPlugInBrowserContextController:(WKWebProcessPlugInBrowserContextController *)controller userAgentForURL:(NSString *)url;
I would pass the WKWebProcessPlugInFrame here as well, for a bit of context. You also should seperate this method from the others with a bit of whitespace, since it is not really part of the "Resource loading" group of functionality.
This should also pass a NSURL rather than an NSString.
> Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePageLoaderClient.h:65
> +typedef WKStringRef (*WKBundlePageUserAgentForURLCallback)(WKStringRef url, const void *clientInfo);
Again, I would pass the frame.
> Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePageLoaderClient.h:408
> + WKBundlePageUserAgentForURLCallback userAgentForURL;
I'm not 100% sure, but you may need to rev the version. We should ask Anders.
> Source/WebKit2/WebProcess/InjectedBundle/API/mac/WKWebProcessPlugInBrowserContextController.mm:191
> + return wkString;
> +
> + }
Extraneous whitespace.
> Source/WebKit2/WebProcess/InjectedBundle/InjectedBundlePageLoaderClient.cpp:357
> +WKStringRef InjectedBundlePageLoaderClient::userAgentForURL(WKStringRef url) {
> + if (!m_client.userAgentForURL)
> + return nullptr;
{ should go on the next line.
> Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:1190
> + return webPage->userAgent(url);
To add the frame to client callback, you will need to pass it here.
> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:2270
> +String WebPage::userAgent(const WebCore::URL& url) const {
{ Should go on the next line. You don't need the WebCore::.
> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:2274
> + RefPtr<API::String> urlString = API::String::create(url.string());
> + WKRetainPtr<WKStringRef> string = adoptWK(m_loaderClient.client().userAgentForURL(toAPI(urlString.get()), m_loaderClient.client().base.clientInfo));
> + API::String *apiString = toImpl(string.get());
> + return apiString->string();
This should return m_userAgent if the client isn't implemented or returns null. Since this will be called for each URL request, I would recommend checking for the an implementation, since then, you can avoid allocating the API::String for the URL.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list