Do not override User-Agent header added by WebCore

Do not override User-Agent header added by WebCore

 - do not strip User-Agent while flattening headers in renderer 
 - do not override User-Agent header in the network stack if one is already supplied by higher level; 
 - added RequestHeaders::SetHeaderIfMissing

TEST=URLRequestTestHTTP.OverrideUserAgent
BUG=http://crbug.com/67063

[caseq, 2010-12-24 13:18:18]

The network stack currently sets User-Agent header unconditionally, overwriting
whatever could be supplied by the client (e.g. from WebCore). The proposed patch
preserves the client-set value of User-Agent in network stack and stops
stripping User-Agent header coming from WebCore. This is needed in order to be
able to override user agent from DevTools.

[willchan no longer on Chromium, 2010-12-24 20:38:21]

+darin since I suspect he cares about User-Agent handling.

The code is simple and looks fine, but I want to make sure that Darin is cool
with the overall approach.  I'm also concerned about how the User-Agent
filtering is disabled now, since from my ignorant point of view, we probably
only want to let DevTools set the User-Agent, but it appears things like XHR may
also be able to set it.  Is this an issue?

http://codereview.chromium.org/6065008/diff/1/net/http/http_request_headers.cc
File net/http/http_request_headers.cc (right):

http://codereview.chromium.org/6065008/diff/1/net/http/http_request_headers.c...
net/http/http_request_headers.cc:81: HeaderVector::iterator it =
FindHeader(key);
It's probably better just to replace these two lines with !HasHeader(key).

http://codereview.chromium.org/6065008/diff/1/webkit/glue/weburlloader_impl.cc
File webkit/glue/weburlloader_impl.cc (left):

http://codereview.chromium.org/6065008/diff/1/webkit/glue/weburlloader_impl.c...
webkit/glue/weburlloader_impl.cc:75: LowerCaseEqualsASCII(name_utf8,
"user-agent"))
So, does this mean that anything in WebCore can set User-Agent and the network
stack should respect it now?  How about XHR?

[caseq, 2010-12-26 07:08:07]

Hi Will, thanks for a prompt response!

On 2010/12/24 20:38:21, willchan wrote:
> +darin since I suspect he cares about User-Agent handling.
> 
> The code is simple and looks fine, but I want to make sure that Darin is cool
> with the overall approach.  I'm also concerned about how the User-Agent
> filtering is disabled now, since from my ignorant point of view, we probably
> only want to let DevTools set the User-Agent, but it appears things like XHR
may
> also be able to set it.  Is this an issue?

[...]

> webkit/glue/weburlloader_impl.cc:75: LowerCaseEqualsASCII(name_utf8,
> "user-agent"))
> So, does this mean that anything in WebCore can set User-Agent and the network
> stack should respect it now?  How about XHR?

It won't be an issue with XHR, as WebCore explicitly forbids setting User-Agent
with setRequestHeader (it must do so by standard, and if it weren't in WebCore,
there would be a potential issue for other browsers):

http://www.google.com/codesearch/p?hl=en#OAMlx_jo-ck/src/third_party/WebKit/W...

Here's a test for that:
http://www.google.com/codesearch/p?hl=en#OAMlx_jo-ck/src/chrome/test/data/lay...

As for the rest of WebCore being able to set User-Agent, I would consider this
to be a feature (think e.g. of an extension API) -- though, if anyone has strong
feelings against that, I can put it under a load flag indeed (I'm tempted to
rename LOAD_REPORT_RAW_HEADERS into something more generic that signifies
request is loaded from a page being inspected and share it for all tweaks we do
for DevTools).

[eroman, 2011-01-04 19:27:13]

Does WebKit end up setting this most of the time? Or will the common codepath
still be setting it from the browser?

Looks good to me, but I suggest having someone like Darin confirm the WebKit
side of things.

[caseq, 2011-01-20 13:48:35]

On 2011/01/04 19:27:13, eroman wrote:
> Does WebKit end up setting this most of the time? Or will the common codepath
> still be setting it from the browser?

Yes -- WebCore sets it in  ResourceLoadNotifier::willSendRequest() (which picks
it via FrameLoaderClient, ultimately from chrome's
WebKitClientImpl::userAgent(), so  we only expect the default value to kick in
for requests that are initiated internally by chrome).

> Looks good to me, but I suggest having someone like Darin confirm the WebKit
> side of things.

Darin, could you please have a look?

[darin (slow to review), 2011-01-20 17:09:02]

LGTM

XMLHttpRequest is prevented from overriding the User-Agent header.  See
XMLHttpRequest.cpp, and search for "user-agent" to see how it is implemented.

[darin (slow to review), 2011-01-20 17:09:53]

By the way, in case anyone was curious, the UA string was not sent to the
browser to reduce bytes sent over IPC.  It was probably premature optimization.

[willchan no longer on Chromium, 2011-01-20 17:28:15]

LOL

On Thu, Jan 20, 2011 at 9:09 AM, <darin@chromium.org> wrote:

> By the way, in case anyone was curious, the UA string was not sent to the
> browser to reduce bytes sent over IPC.  It was probably premature
> optimization.
>
>
> http://codereview.chromium.org/6065008/
>