Support scheme-setting for GoogleURLTracker.

Support scheme-setting for GoogleURLTracker.

This switches the searchdomaincheck request from format=domain (".google.com") to format=url ("http://www.google.com/").  This in turn allows the server to direct us to use HTTPS searches by default, which should save a redirect on each search over the current situation when it's applicable.

We hide this detail from the user; we continue to prompt about TLD changes, but we don't prompt about scheme changes.

BUG=96636
TEST=Make sure the cookie store has cookies such that you get HTTPS search by default on Google.  After this patch, doing a search should not involve a redirect through HTTP.
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=136205

[Ilya Sherman, 2012-05-09 22:14:44]

LGTM

https://chromiumcodereview.appspot.com/10382091/diff/1/chrome/browser/google/...
File chrome/browser/google/google_url_tracker.cc (right):

https://chromiumcodereview.appspot.com/10382091/diff/1/chrome/browser/google/...
chrome/browser/google/google_url_tracker.cc:319:
google_util::DISALLOW_SUBDOMAIN))
nit: Please indent four more spaces

https://chromiumcodereview.appspot.com/10382091/diff/1/chrome/browser/google/...
chrome/browser/google/google_url_tracker.cc:367: if (!url.is_valid())
nit: Maybe remind the reader that |url| is contains the previously fetched URL? 
The std::swap is a ways away from here, and the variable name doesn't really
help explain what's happening here.

https://chromiumcodereview.appspot.com/10382091/diff/1/chrome/browser/google/...
chrome/browser/google/google_url_tracker.cc:372: for (InfoBarMap::iterator
i(infobar_map_.begin());
nit: Please use "it" rather than "i" for the iterator name.

https://chromiumcodereview.appspot.com/10382091/diff/1/chrome/browser/google/...
File chrome/browser/google/google_url_tracker_unittest.cc (right):

https://chromiumcodereview.appspot.com/10382091/diff/1/chrome/browser/google/...
chrome/browser/google/google_url_tracker_unittest.cc:5: #include
"chrome/browser/google/google_url_tracker.h"
nit: No need to promote this to the top of the file, since it's not the header
file corresponding to the test file (in fact, there is no such header file for
this test file).

https://chromiumcodereview.appspot.com/10382091/diff/1/chrome/browser/google/...
chrome/browser/google/google_url_tracker_unittest.cc:374: // Non-empty
path/query.
nit: Please also test the case where the path is non-empty but lacks a query
param.

[Peter Kasting, 2012-05-09 23:24:18]

Submitting.

https://chromiumcodereview.appspot.com/10382091/diff/1/chrome/browser/google/...
File chrome/browser/google/google_url_tracker.cc (right):

https://chromiumcodereview.appspot.com/10382091/diff/1/chrome/browser/google/...
chrome/browser/google/google_url_tracker.cc:319:
google_util::DISALLOW_SUBDOMAIN))
On 2012/05/09 22:14:44, Ilya Sherman wrote:
> nit: Please indent four more spaces

Rewrapped more significantly to gain clarity.

https://chromiumcodereview.appspot.com/10382091/diff/1/chrome/browser/google/...
chrome/browser/google/google_url_tracker.cc:367: if (!url.is_valid())
On 2012/05/09 22:14:44, Ilya Sherman wrote:
> nit: Maybe remind the reader that |url| is contains the previously fetched
URL? 
> The std::swap is a ways away from here, and the variable name doesn't really
> help explain what's happening here.

Yeah -- I originally named the temp "old_fetched_google_url" to make this
clearer, but then that made most of the usage near the declaration confusing.

Added a comment.

https://chromiumcodereview.appspot.com/10382091/diff/1/chrome/browser/google/...
chrome/browser/google/google_url_tracker.cc:372: for (InfoBarMap::iterator
i(infobar_map_.begin());
On 2012/05/09 22:14:44, Ilya Sherman wrote:
> nit: Please use "it" rather than "i" for the iterator name.

I don't know why but it really bugs me when people do this!  "i" seems like a
perfectly good name for any kind of loop index variable :)

https://chromiumcodereview.appspot.com/10382091/diff/1/chrome/browser/google/...
File chrome/browser/google/google_url_tracker_unittest.cc (right):

https://chromiumcodereview.appspot.com/10382091/diff/1/chrome/browser/google/...
chrome/browser/google/google_url_tracker_unittest.cc:5: #include
"chrome/browser/google/google_url_tracker.h"
On 2012/05/09 22:14:44, Ilya Sherman wrote:
> nit: No need to promote this to the top of the file, since it's not the header
> file corresponding to the test file (in fact, there is no such header file for
> this test file).

I had thought so too, but another readability reviewer caused me to recheck
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Names_and_Orde...
:

"In dir/foo.cc or dir/foo_test.cc, whose main purpose is to implement or test
the stuff in dir2/foo2.h, order your includes as follows: <puts foo2.h first>"

Note the test-specific callout.

https://chromiumcodereview.appspot.com/10382091/diff/1/chrome/browser/google/...
chrome/browser/google/google_url_tracker_unittest.cc:374: // Non-empty
path/query.
On 2012/05/09 22:14:44, Ilya Sherman wrote:
> nit: Please also test the case where the path is non-empty but lacks a query
> param.

OK, separated into distinct path/query/ref cases.

[Ilya Sherman, 2012-05-09 23:30:21]

https://chromiumcodereview.appspot.com/10382091/diff/1/chrome/browser/google/...
File chrome/browser/google/google_url_tracker.cc (right):

https://chromiumcodereview.appspot.com/10382091/diff/1/chrome/browser/google/...
chrome/browser/google/google_url_tracker.cc:372: for (InfoBarMap::iterator
i(infobar_map_.begin());
On 2012/05/09 23:24:18, Peter Kasting wrote:
> On 2012/05/09 22:14:44, Ilya Sherman wrote:
> > nit: Please use "it" rather than "i" for the iterator name.
> 
> I don't know why but it really bugs me when people do this!  "i" seems like a
> perfectly good name for any kind of loop index variable :)

"i" is an index/number; "it" is an iterator, which is a pointer.  I'd strongly
prefer "it" vs. "i" -- that's a very established, if unofficial, stylistic
convention in the Chromium source (at least all the parts that I've worked
with...)

https://chromiumcodereview.appspot.com/10382091/diff/1/chrome/browser/google/...
File chrome/browser/google/google_url_tracker_unittest.cc (right):

https://chromiumcodereview.appspot.com/10382091/diff/1/chrome/browser/google/...
chrome/browser/google/google_url_tracker_unittest.cc:5: #include
"chrome/browser/google/google_url_tracker.h"
On 2012/05/09 23:24:18, Peter Kasting wrote:
> On 2012/05/09 22:14:44, Ilya Sherman wrote:
> > nit: No need to promote this to the top of the file, since it's not the
header
> > file corresponding to the test file (in fact, there is no such header file
for
> > this test file).
> 
> I had thought so too, but another readability reviewer caused me to recheck
>
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Names_and_Orde...
> :
> 
> "In dir/foo.cc or dir/foo_test.cc, whose main purpose is to implement or test
> the stuff in dir2/foo2.h, order your includes as follows: <puts foo2.h first>"
> 
> Note the test-specific callout.

Ah, interesting.  Good to know, thanks for pointing that out.