Add support for UA spoofing, and spoof Safari's UA string when loading URLs...

Add support for UA spoofing, and spoof Safari's UA string when loading URLs
from *.mail.live.com (to fix hotmail).

BUG=4111
R=wtc


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=8764

[wtc, 2009-01-27 19:19:24]

LGTM.

It shouldn't be hard to backport this CL to the
release-stable branch.

http://codereview.chromium.org/19025/diff/1/7
File net/url_request/url_request_context.h (right):

http://codereview.chromium.org/19025/diff/1/7#newcode61
Line 61: // valid URL, then the default UA string should be returned. 
Subclasses
It's not clear what you meant by the "default UA string".
Is it the empty string that the base class returns?

Update: after seeing how you use this method in
url_request_inet_job.cc, I understand what you meant.
You may want to rephrase the comment like this:
    Pass an invalid URL (such as GURL()) to get the
    default UA string.
This stresses the fact that it is the proper way to
get the default UA string, rather than an error
handling mode.

http://codereview.chromium.org/19025/diff/1/7#newcode70
Line 70: virtual ~URLRequestContext() {}
Just curious: can we make the destructor private?

http://codereview.chromium.org/19025/diff/1/5
File net/url_request/url_request_inet_job.cc (right):

http://codereview.chromium.org/19025/diff/1/5#newcode86
Line 86: InitializeTheInternet(request->context() ?
Nit: there is a lint error in one of these lines (line
ends in white space).

http://codereview.chromium.org/19025/diff/1/9
File webkit/glue/webframeloaderclient_impl.cc (right):

http://codereview.chromium.org/19025/diff/1/9#newcode1266
Line 1266: String WebFrameLoaderClient::userAgent(const KURL& url) {
So WebFrameLoaderClient::userAgent is already designed to
allow for URL-specific UA string, right?

http://codereview.chromium.org/19025/diff/1/8
File webkit/glue/webkit_glue.cc (right):

http://codereview.chromium.org/19025/diff/1/8#newcode356
Line 356: product = "Version/3.2.1";
Nit: We should move the hardcoded Safari version 3.2.1 to a
more discoverable header file.

http://codereview.chromium.org/19025/diff/1/8#newcode418
Line 418: static std::string mimic_safari_user_agent;
Because you're using a static object here, please make
sure this method is only called by a single thread.

http://codereview.chromium.org/19025/diff/1/8#newcode419
Line 419: // For hotmail, we need to spoof as Safari (bug 4111).
Nit: is this comment over-indented?

http://codereview.chromium.org/19025/diff/1/10
File webkit/glue/webkit_glue.h (right):

http://codereview.chromium.org/19025/diff/1/10#newcode109
Line 109: // overriden by a call to SetUserAgent() (which should be done at
startup).
Nit: the comment should be updated to mention the new 'url'
parameter.

[darin (slow to review), 2009-01-27 19:52:06]

http://codereview.chromium.org/19025/diff/1/7
File net/url_request/url_request_context.h (right):

http://codereview.chromium.org/19025/diff/1/7#newcode61
Line 61: // valid URL, then the default UA string should be returned. 
Subclasses
Nice suggestion, thanks.

http://codereview.chromium.org/19025/diff/1/7#newcode70
Line 70: virtual ~URLRequestContext() {}
yes!

http://codereview.chromium.org/19025/diff/1/9
File webkit/glue/webframeloaderclient_impl.cc (right):

http://codereview.chromium.org/19025/diff/1/9#newcode1266
Line 1266: String WebFrameLoaderClient::userAgent(const KURL& url) {
Yup.

http://codereview.chromium.org/19025/diff/1/8
File webkit/glue/webkit_glue.cc (right):

http://codereview.chromium.org/19025/diff/1/8#newcode356
Line 356: product = "Version/3.2.1";
Agreed, but I'll defer that to a subsequent patch.

http://codereview.chromium.org/19025/diff/1/8#newcode418
Line 418: static std::string mimic_safari_user_agent;
hmm... good point.  i believe it is.

http://codereview.chromium.org/19025/diff/1/8#newcode419
Line 419: // For hotmail, we need to spoof as Safari (bug 4111).
oops.. a <TAB> slipped in there.

[darin (slow to review), 2009-01-27 20:00:11]

http://codereview.chromium.org/19025/diff/1/7
File net/url_request/url_request_context.h (right):

http://codereview.chromium.org/19025/diff/1/7#newcode70
Line 70: virtual ~URLRequestContext() {}
actually, nevermind.  a private destructor prevents inheritance.