[webkit-reviews] review denied: [Bug 17375] Set user-agent from application : [Attachment 27004] Implement "user-agent" web settings property

[bugzilla-daemon at webkit.org]

Mark Rowe (bdash) <mrowe at apple.com> has denied Christian Dywan
<christian at imendio.com>'s request for review:
Bug 17375: Set user-agent from application
https://bugs.webkit.org/show_bug.cgi?id=17375

Attachment 27004: Implement "user-agent" web settings property
https://bugs.webkit.org/attachment.cgi?id=27004&action=review

------- Additional Comments from Mark Rowe (bdash) <mrowe at apple.com>
I'm don't think that it makes sense to store the user agent on a single
FrameLoaderClient instance.  The FIXME's indicate that you were also unsure
about this.  Setting the user agent on a WebView should affect all frames in
that view, rather than just the main frame.  The way that this is handled on
the Mac is that FrameLoaderClient::userAgent asks the WebView for the user
agent for a given URL, which it computes if needed or returns the overridden
value if the application has set one.  This will ensure that subframes get the
correct behaviour.

I'm going to say r- on this basis.
   

> diff --git a/WebCore/loader/FrameLoaderClient.h
b/WebCore/loader/FrameLoaderClient.h
> index b90cecd..f4b9861 100644
> --- a/WebCore/loader/FrameLoaderClient.h
> +++ b/WebCore/loader/FrameLoaderClient.h
> @@ -179,6 +179,9 @@ namespace WebCore {
>	   virtual PassRefPtr<DocumentLoader> createDocumentLoader(const
ResourceRequest&, const SubstituteData&) = 0;
>	   virtual void setTitle(const String& title, const KURL&) = 0;
>  
> +#if PLATFORM(GTK)
> +	   virtual void setUserAgent(const String&) = 0;
> +#endif
>	   virtual String userAgent(const KURL&) = 0;
>	   
>	   virtual void savePlatformDataToCachedPage(CachedPage*) = 0;

It doesn't make sense for this to be #if'd in cross-platform code in WebCore. 
It would live in FrameLoaderClientGtk.h in WebKit.  There's also no need for it
to be declared virtual.