Client Hints

Client Hints

Client Hints can be used as input to proactive content negotiation;
just as the Accept header allowed clients to indicate what formats
they prefer, Client Hints allow clients to indicate a list of device
and agent specific preferences.

See:
https://github.com/igrigorik/http-client-hints/blob/draft2/draft-grigorik-http-client-hints-01.txt

Patched from https://codereview.chromium.org/23654014/, which was
patched from https://codereview.chromium.org/11970002

BUG=170388
CONTRIBUTOR=yoav@yoav.ws

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=225425

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=227178

[bengr, 2013-09-24 22:47:18]

This CL replaces my last one on Client Hints. The only real changes are to
chrome/browser/net/client_hints*.

You both said lgtm on the last CL, but PTAL.

thestig: chrome/browser/io_thread.cc
mmenke: everything else.

[Lei Zhang, 2013-09-24 23:04:44]

lg with mmenke's approval.

https://codereview.chromium.org/24451003/diff/1/AUTHORS
File AUTHORS (right):

https://codereview.chromium.org/24451003/diff/1/AUTHORS#newcode289
AUTHORS:289: Yoav Weiss <yoav@yoav.ws>
W before Z

[mmenke, 2013-09-25 16:26:19]

https://codereview.chromium.org/24451003/diff/1/chrome/browser/net/chrome_net...
File chrome/browser/net/chrome_network_delegate.cc (right):

https://codereview.chromium.org/24451003/diff/1/chrome/browser/net/chrome_net...
chrome/browser/net/chrome_network_delegate.cc:84: const char
kClientHintsDevicePixelRatioHeader[] = "CH-DPR";
optional:  This may make more sense as a static member of the client hints
class.

https://codereview.chromium.org/24451003/diff/1/chrome/browser/net/chrome_net...
chrome/browser/net/chrome_network_delegate.cc:84: const char
kClientHintsDevicePixelRatioHeader[] = "CH-DPR";
ERm...This seems to violate the spec linked in the other file.  You're adding
CH-DPR: 1.0.

Shouldn't it be CH: DPR=1.0?  Or is there a newer spec?

https://codereview.chromium.org/24451003/diff/1/chrome/browser/net/client_hin...
File chrome/browser/net/client_hints.cc (right):

https://codereview.chromium.org/24451003/diff/1/chrome/browser/net/client_hin...
chrome/browser/net/client_hints.cc:46: base::Owned(devicePixelRatio)));
How about just using PostTaskAndReplyWithResult in src/base/task_runner_util.h
(It does the same thing more intuitively - though, admittedly, the adapter it
uses internally to convert a return value to an argument adds more overhead).

https://codereview.chromium.org/24451003/diff/1/chrome/browser/net/client_hin...
chrome/browser/net/client_hints.cc:54: if (device_pixel_ratio_value &&
*device_pixel_ratio_value > 0.0) {
If something can't happen, you shouldn't be testing for it.  In this case, I
don't think we should have the "device_pixel_ratio_value" check.  Using
PostTaskAndReplyWithResult would make this a moot point, since you'd just pass
in a float, of course.

https://codereview.chromium.org/24451003/diff/1/chrome/browser/net/client_hin...
File chrome/browser/net/client_hints.h (right):

https://codereview.chromium.org/24451003/diff/1/chrome/browser/net/client_hin...
chrome/browser/net/client_hints.h:27: // returned by the JavaScript command
window.devicePixelRatio
nit:  .

https://codereview.chromium.org/24451003/diff/1/chrome/browser/net/client_hin...
chrome/browser/net/client_hints.h:28: const std::string& GetString() const;
If the new ClientHints spec uses multiple fields, instead of a single CH field
with multiple values, I think this function name is wrong.  It should be
GetDeviceHeaderRatio();  (I know I suggested GetString(), but that was under the
assumption the spec was correct).

https://codereview.chromium.org/24451003/diff/1/chrome/common/chrome_switches.h
File chrome/common/chrome_switches.h (right):

https://codereview.chromium.org/24451003/diff/1/chrome/common/chrome_switches...
chrome/common/chrome_switches.h:169: extern const char kEnableClientHints[];
I'm assuming the plan is to get rid of this, once other browsers support it,
and/or when the spec is standardized?

[bengr, 2013-09-25 17:25:51]

https://codereview.chromium.org/24451003/diff/1/AUTHORS
File AUTHORS (right):

https://codereview.chromium.org/24451003/diff/1/AUTHORS#newcode289
AUTHORS:289: Yoav Weiss <yoav@yoav.ws>
On 2013/09/24 23:04:44, Lei Zhang wrote:
> W before Z

Done.

https://codereview.chromium.org/24451003/diff/1/chrome/browser/net/chrome_net...
File chrome/browser/net/chrome_network_delegate.cc (right):

https://codereview.chromium.org/24451003/diff/1/chrome/browser/net/chrome_net...
chrome/browser/net/chrome_network_delegate.cc:84: const char
kClientHintsDevicePixelRatioHeader[] = "CH-DPR";
On 2013/09/25 16:26:19, mmenke wrote:
> ERm...This seems to violate the spec linked in the other file.  You're adding
> CH-DPR: 1.0.
> 
> Shouldn't it be CH: DPR=1.0?  Or is there a newer spec?

There's a newer spec. The CL points to it but a comment in the code pointed to
the old one. I've updated the comment.

https://codereview.chromium.org/24451003/diff/1/chrome/browser/net/chrome_net...
chrome/browser/net/chrome_network_delegate.cc:84: const char
kClientHintsDevicePixelRatioHeader[] = "CH-DPR";
On 2013/09/25 16:26:19, mmenke wrote:
> optional:  This may make more sense as a static member of the client hints
> class.

Done.

https://codereview.chromium.org/24451003/diff/1/chrome/browser/net/client_hin...
File chrome/browser/net/client_hints.cc (right):

https://codereview.chromium.org/24451003/diff/1/chrome/browser/net/client_hin...
chrome/browser/net/client_hints.cc:46: base::Owned(devicePixelRatio)));
On 2013/09/25 16:26:19, mmenke wrote:
> How about just using PostTaskAndReplyWithResult in src/base/task_runner_util.h
> (It does the same thing more intuitively - though, admittedly, the adapter it
> uses internally to convert a return value to an argument adds more overhead).

Done. I hadn't done this before b/c I was returning a bunch of values, but it
definitely makes sense now. (It probably made sense to do so even when I was
returning multiple values.)

https://codereview.chromium.org/24451003/diff/1/chrome/browser/net/client_hin...
chrome/browser/net/client_hints.cc:54: if (device_pixel_ratio_value &&
*device_pixel_ratio_value > 0.0) {
On 2013/09/25 16:26:19, mmenke wrote:
> If something can't happen, you shouldn't be testing for it.  In this case, I
> don't think we should have the "device_pixel_ratio_value" check.  Using
> PostTaskAndReplyWithResult would make this a moot point, since you'd just pass
> in a float, of course.

Done.

https://codereview.chromium.org/24451003/diff/1/chrome/browser/net/client_hin...
File chrome/browser/net/client_hints.h (right):

https://codereview.chromium.org/24451003/diff/1/chrome/browser/net/client_hin...
chrome/browser/net/client_hints.h:27: // returned by the JavaScript command
window.devicePixelRatio
On 2013/09/25 16:26:19, mmenke wrote:
> nit:  .

Done.

https://codereview.chromium.org/24451003/diff/1/chrome/browser/net/client_hin...
chrome/browser/net/client_hints.h:28: const std::string& GetString() const;
On 2013/09/25 16:26:19, mmenke wrote:
> If the new ClientHints spec uses multiple fields, instead of a single CH field
> with multiple values, I think this function name is wrong.  It should be
> GetDeviceHeaderRatio();  (I know I suggested GetString(), but that was under
the
> assumption the spec was correct).

Changed to GetDevicePixelRatioHeader.

https://codereview.chromium.org/24451003/diff/1/chrome/common/chrome_switches.h
File chrome/common/chrome_switches.h (right):

https://codereview.chromium.org/24451003/diff/1/chrome/common/chrome_switches...
chrome/common/chrome_switches.h:169: extern const char kEnableClientHints[];
On 2013/09/25 16:26:19, mmenke wrote:
> I'm assuming the plan is to get rid of this, once other browsers support it,
> and/or when the spec is standardized?

Yes.

[mmenke, 2013-09-25 17:37:39]

https://codereview.chromium.org/24451003/diff/11001/chrome/browser/net/client...
File chrome/browser/net/client_hints.cc (right):

https://codereview.chromium.org/24451003/diff/11001/chrome/browser/net/client...
chrome/browser/net/client_hints.cc:12: #include "ui/gfx/screen.h"
nit:  #include "base/task_runner_util.h"

https://codereview.chromium.org/24451003/diff/11001/chrome/browser/net/client...
chrome/browser/net/client_hints.cc:63: it != device_pixel_ratio.end(); ++it)
nit:  Use braces {}.

https://codereview.chromium.org/24451003/diff/11001/chrome/browser/net/client...
File chrome/browser/net/client_hints.h (right):

https://codereview.chromium.org/24451003/diff/11001/chrome/browser/net/client...
chrome/browser/net/client_hints.h:10: #include "base/memory/weak_ptr.h"
nit:  include basictypes.h for DISALLOW_COPY_AND_ASSIGN

https://codereview.chromium.org/24451003/diff/11001/chrome/browser/net/client...
chrome/browser/net/client_hints.h:25: bool RetrieveScreenInfo();
Optional:  Suggest "UpdateScreenInfo" to make it clearer that it modifies this,
instead of just retrieves a value.

Also, should this be public?  Ideally, the ClientHints object would watch for
changes itself, or through its own listener on the UI thread.

https://codereview.chromium.org/24451003/diff/11001/chrome/browser/net/client...
chrome/browser/net/client_hints.h:29: // returned by the JavaScript command
window.devicePixelRatio.
Comment is also incorrect.

Suggest something along the lines of "Returns the pixel ration as a string.  Its
value is the same as window.devicePixelRatio in Javascript."

[bengr, 2013-09-25 20:04:12]

https://codereview.chromium.org/24451003/diff/11001/chrome/browser/net/client...
File chrome/browser/net/client_hints.cc (right):

https://codereview.chromium.org/24451003/diff/11001/chrome/browser/net/client...
chrome/browser/net/client_hints.cc:12: #include "ui/gfx/screen.h"
On 2013/09/25 17:37:40, mmenke wrote:
> nit:  #include "base/task_runner_util.h"

Done.

https://codereview.chromium.org/24451003/diff/11001/chrome/browser/net/client...
chrome/browser/net/client_hints.cc:63: it != device_pixel_ratio.end(); ++it)
On 2013/09/25 17:37:40, mmenke wrote:
> nit:  Use braces {}.

Done.

https://codereview.chromium.org/24451003/diff/11001/chrome/browser/net/client...
File chrome/browser/net/client_hints.h (right):

https://codereview.chromium.org/24451003/diff/11001/chrome/browser/net/client...
chrome/browser/net/client_hints.h:10: #include "base/memory/weak_ptr.h"
On 2013/09/25 17:37:40, mmenke wrote:
> nit:  include basictypes.h for DISALLOW_COPY_AND_ASSIGN

Done.

https://codereview.chromium.org/24451003/diff/11001/chrome/browser/net/client...
chrome/browser/net/client_hints.h:25: bool RetrieveScreenInfo();
On 2013/09/25 17:37:40, mmenke wrote:
> Optional:  Suggest "UpdateScreenInfo" to make it clearer that it modifies
this,
> instead of just retrieves a value.
> 
> Also, should this be public?  Ideally, the ClientHints object would watch for
> changes itself, or through its own listener on the UI thread.

I changed the comment slightly instead. There's already an UpdateScreenInfo(),
so I wanted to avoid having to rename the other one as well.

https://codereview.chromium.org/24451003/diff/11001/chrome/browser/net/client...
chrome/browser/net/client_hints.h:29: // returned by the JavaScript command
window.devicePixelRatio.
On 2013/09/25 17:37:40, mmenke wrote:
> Comment is also incorrect.
> 
> Suggest something along the lines of "Returns the pixel ration as a string. 
Its
> value is the same as window.devicePixelRatio in Javascript."

Done.

[mmenke, 2013-09-25 20:07:48]

LGTM

https://codereview.chromium.org/24451003/diff/34001/chrome/browser/net/client...
File chrome/browser/net/client_hints.h (right):

https://codereview.chromium.org/24451003/diff/34001/chrome/browser/net/client...
chrome/browser/net/client_hints.h:31: bool RetrieveScreenInfo();
nit:  Since nothing inherits from ClientHints, this should be private.

[Lei Zhang, 2013-09-25 20:36:32]

lgtm with some nits

https://codereview.chromium.org/24451003/diff/34001/chrome/browser/net/chrome...
File chrome/browser/net/chrome_network_delegate.h (right):

https://codereview.chromium.org/24451003/diff/34001/chrome/browser/net/chrome...
chrome/browser/net/chrome_network_delegate.h:19: class ClientHints;
nit: alphabetical order

https://codereview.chromium.org/24451003/diff/34001/chrome/browser/net/chrome...
chrome/browser/net/chrome_network_delegate.h:214: scoped_ptr<ClientHints>
client_hints_;
include scoped_ptr.h

https://codereview.chromium.org/24451003/diff/34001/chrome/browser/net/client...
File chrome/browser/net/client_hints.h (right):

https://codereview.chromium.org/24451003/diff/34001/chrome/browser/net/client...
chrome/browser/net/client_hints.h:18: static const char*
kDevicePixelRatioHeader;
Please change this to const char kDevicePixelRatioHeader[];

https://codereview.chromium.org/24451003/diff/34001/chrome/browser/net/client...
File chrome/browser/net/client_hints_unittest.cc (right):

https://codereview.chromium.org/24451003/diff/34001/chrome/browser/net/client...
chrome/browser/net/client_hints_unittest.cc:10: #include
"base/memory/scoped_ptr.h"
nit: not used

[bengr, 2013-09-25 21:39:45]

https://codereview.chromium.org/24451003/diff/34001/chrome/browser/net/chrome...
File chrome/browser/net/chrome_network_delegate.h (right):

https://codereview.chromium.org/24451003/diff/34001/chrome/browser/net/chrome...
chrome/browser/net/chrome_network_delegate.h:19: class ClientHints;
On 2013/09/25 20:36:32, Lei Zhang wrote:
> nit: alphabetical order

Done.

https://codereview.chromium.org/24451003/diff/34001/chrome/browser/net/chrome...
chrome/browser/net/chrome_network_delegate.h:214: scoped_ptr<ClientHints>
client_hints_;
On 2013/09/25 20:36:32, Lei Zhang wrote:
> include scoped_ptr.h

Done.

https://codereview.chromium.org/24451003/diff/34001/chrome/browser/net/client...
File chrome/browser/net/client_hints.h (right):

https://codereview.chromium.org/24451003/diff/34001/chrome/browser/net/client...
chrome/browser/net/client_hints.h:18: static const char*
kDevicePixelRatioHeader;
On 2013/09/25 20:36:32, Lei Zhang wrote:
> Please change this to const char kDevicePixelRatioHeader[];

Done.

https://codereview.chromium.org/24451003/diff/34001/chrome/browser/net/client...
chrome/browser/net/client_hints.h:31: bool RetrieveScreenInfo();
On 2013/09/25 20:07:48, mmenke wrote:
> nit:  Since nothing inherits from ClientHints, this should be private.

Done.

https://codereview.chromium.org/24451003/diff/34001/chrome/browser/net/client...
File chrome/browser/net/client_hints_unittest.cc (right):

https://codereview.chromium.org/24451003/diff/34001/chrome/browser/net/client...
chrome/browser/net/client_hints_unittest.cc:10: #include
"base/memory/scoped_ptr.h"
On 2013/09/25 20:36:32, Lei Zhang wrote:
> nit: not used

Done.

[commit-bot: I haz the power, 2013-09-25 21:50:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bengr@chromium.org/24451003/41001

[commit-bot: I haz the power, 2013-09-26 09:43:31]

Change committed as 225425

[Avi (use Gerrit), 2013-09-26 19:21:25]

https://codereview.chromium.org/24451003/diff/41001/chrome/browser/net/client...
File chrome/browser/net/client_hints_unittest.cc (right):

https://codereview.chromium.org/24451003/diff/41001/chrome/browser/net/client...
chrome/browser/net/client_hints_unittest.cc:29: setlocale(LC_ALL,
"fr_FR.UTF-8");
Nooooo. Please don't set a locale and fail to reset it. It wrecks all following
tests!

[groby-ooo-7-16, 2013-09-26 19:25:04]

See base::test::ScopedLocale for locale handling :)

[Avi (use Gerrit), 2013-09-26 19:44:04]

On 2013/09/26 19:21:25, Avi wrote:
>
https://codereview.chromium.org/24451003/diff/41001/chrome/browser/net/client...
> File chrome/browser/net/client_hints_unittest.cc (right):
> 
>
https://codereview.chromium.org/24451003/diff/41001/chrome/browser/net/client...
> chrome/browser/net/client_hints_unittest.cc:29: setlocale(LC_ALL,
> "fr_FR.UTF-8");
> Nooooo. Please don't set a locale and fail to reset it. It wrecks all
following
> tests!

This appears to break a wide variety of tests; see http://crbug.com/298893 and
http://crbug.com/298900.

[Avi (use Gerrit), 2013-09-26 19:44:35]

On 2013/09/26 19:44:04, Avi wrote:
> On 2013/09/26 19:21:25, Avi wrote:
> >
>
https://codereview.chromium.org/24451003/diff/41001/chrome/browser/net/client...
> > File chrome/browser/net/client_hints_unittest.cc (right):
> > 
> >
>
https://codereview.chromium.org/24451003/diff/41001/chrome/browser/net/client...
> > chrome/browser/net/client_hints_unittest.cc:29: setlocale(LC_ALL,
> > "fr_FR.UTF-8");
> > Nooooo. Please don't set a locale and fail to reset it. It wrecks all
> following
> > tests!
> 
> This appears to break a wide variety of tests; see http://crbug.com/298893 and
> http://crbug.com/298900.

And I have http://crrev.com/24809002 ready to land as soon as the tree re-opens.

[Avi (use Gerrit), 2013-09-26 21:17:53]

BTW, this also breaks android:

http://build.chromium.org/p/chromium.linux/builders/Android%20Tests%20%28dbg%...

[----------] 1 test from ClientHintsTest
[ RUN      ] ClientHintsTest.HintsWellFormattedWithNonEnLocale
[ CRASHED      ]

petewil is reverting; try again.

[mmenke, 2013-09-26 21:30:17]

On 2013/09/26 21:17:53, Avi wrote:
> BTW, this also breaks android:
> 
>
http://build.chromium.org/p/chromium.linux/builders/Android%2520Tests%2520%25...
> 
> [----------] 1 test from ClientHintsTest
> [ RUN      ] ClientHintsTest.HintsWellFormattedWithNonEnLocale
> [ CRASHED      ]
> 
> petewil is reverting; try again.

Hmm...  Why did this work on the trybots, but not on the waterfall?  Curious
that it passed on the Android trybots without that crash as well.

[Avi (use Gerrit), 2013-09-26 21:34:31]

On 2013/09/26 21:30:17, mmenke wrote:
> On 2013/09/26 21:17:53, Avi wrote:
> > BTW, this also breaks android:
> > 
> >
>
http://build.chromium.org/p/chromium.linux/builders/Android%252520Tests%25252...
> > 
> > [----------] 1 test from ClientHintsTest
> > [ RUN      ] ClientHintsTest.HintsWellFormattedWithNonEnLocale
> > [ CRASHED      ]
> > 
> > petewil is reverting; try again.
> 
> Hmm...  Why did this work on the trybots, but not on the waterfall?  Curious
> that it passed on the Android trybots without that crash as well.

No idea. Then again, looking at the bots I don't see a second similar failure.
Weird that it would flake exactly there...

In any case, some interesting data for you to ponder. :/

[Lei Zhang, 2013-09-26 21:51:38]

On 2013/09/26 21:30:17, mmenke wrote:
> Hmm...  Why did this work on the trybots, but not on the waterfall?  Curious
> that it passed on the Android trybots without that crash as well.

A lot of time you get lucky because the tests got sharded in a particular way
and the tests in the shard run in a particular order. See my rant on
chromium-dev about how I tracked down some failing tests because some other bad
test flipped a global earlier in the test run.

[commit-bot: I haz the power, 2013-09-30 17:33:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bengr@chromium.org/24451003/75001

[Avi (use Gerrit), 2013-09-30 17:35:55]

https://codereview.chromium.org/24451003/diff/75001/chrome/browser/net/client...
File chrome/browser/net/client_hints_unittest.cc (right):

https://codereview.chromium.org/24451003/diff/75001/chrome/browser/net/client...
chrome/browser/net/client_hints_unittest.cc:33: // Android and iOS do not
support setLocal.
Without ScopedLocale, this test is meaningless (and is already covered by
HintsWellFormatted above). You should comment out the whole test rather than
just the ScopedLocale.

[Avi (use Gerrit), 2013-09-30 17:37:48]

On 2013/09/30 17:35:55, Avi wrote:
>
https://codereview.chromium.org/24451003/diff/75001/chrome/browser/net/client...
> File chrome/browser/net/client_hints_unittest.cc (right):
> 
>
https://codereview.chromium.org/24451003/diff/75001/chrome/browser/net/client...
> chrome/browser/net/client_hints_unittest.cc:33: // Android and iOS do not
> support setLocal.
> Without ScopedLocale, this test is meaningless (and is already covered by
> HintsWellFormatted above). You should comment out the whole test rather than
> just the ScopedLocale.

FYI, when I tried this fix earlier (https://codereview.chromium.org/24809002) I
failed on the Windows bots linking ScopedLocale. You might end up having to
figure out why.

[bengr, 2013-09-30 17:42:12]

On 2013/09/30 17:37:48, Avi wrote:
> On 2013/09/30 17:35:55, Avi wrote:
> >
>
https://codereview.chromium.org/24451003/diff/75001/chrome/browser/net/client...
> > File chrome/browser/net/client_hints_unittest.cc (right):
> > 
> >
>
https://codereview.chromium.org/24451003/diff/75001/chrome/browser/net/client...
> > chrome/browser/net/client_hints_unittest.cc:33: // Android and iOS do not
> > support setLocal.
> > Without ScopedLocale, this test is meaningless (and is already covered by
> > HintsWellFormatted above). You should comment out the whole test rather than
> > just the ScopedLocale.
> 
> FYI, when I tried this fix earlier (https://codereview.chromium.org/24809002)
I
> failed on the Windows bots linking ScopedLocale. You might end up having to
> figure out why.

Good to know. Thanks.

[bengr, 2013-09-30 17:42:22]

https://codereview.chromium.org/24451003/diff/75001/chrome/browser/net/client...
File chrome/browser/net/client_hints_unittest.cc (right):

https://codereview.chromium.org/24451003/diff/75001/chrome/browser/net/client...
chrome/browser/net/client_hints_unittest.cc:33: // Android and iOS do not
support setLocal.
On 2013/09/30 17:35:55, Avi wrote:
> Without ScopedLocale, this test is meaningless (and is already covered by
> HintsWellFormatted above). You should comment out the whole test rather than
> just the ScopedLocale.

Done.

[groby-ooo-7-16, 2013-09-30 17:54:24]

>FYI, when I tried this fix earlier (https://codereview.chromium.org/24809002) I
failed on the Windows bots linking ScopedLocale. You might end up having to
figure out why.

ScopedLocale seems to be currently Posix-only :( (I'm not sure why, IIRC Win
supports setlocale too)

[Avi (use Gerrit), 2013-09-30 18:15:29]

On 2013/09/30 17:54:24, groby wrote:
> >FYI, when I tried this fix earlier (https://codereview.chromium.org/24809002)
I
> failed on the Windows bots linking ScopedLocale. You might end up having to
> figure out why.
> 
> ScopedLocale seems to be currently Posix-only :( (I'm not sure why, IIRC Win
> supports setlocale too)

That's weird. Congrats, Ben, you get to fix that!

(LGTM for the test now.)

[Avi (use Gerrit), 2013-09-30 18:16:15]

https://codereview.chromium.org/24451003/diff/83001/chrome/browser/net/client...
File chrome/browser/net/client_hints_unittest.cc (right):

https://codereview.chromium.org/24451003/diff/83001/chrome/browser/net/client...
chrome/browser/net/client_hints_unittest.cc:33: // Android and iOS do not
support setLocal.
Move the comment above the #if.

And it's "setlocale", not "setLocal".

[bengr, 2013-10-04 21:27:23]

https://codereview.chromium.org/24451003/diff/83001/chrome/browser/net/client...
File chrome/browser/net/client_hints_unittest.cc (right):

https://codereview.chromium.org/24451003/diff/83001/chrome/browser/net/client...
chrome/browser/net/client_hints_unittest.cc:33: // Android and iOS do not
support setLocal.
On 2013/09/30 18:16:16, Avi wrote:
> Move the comment above the #if.
> 
> And it's "setlocale", not "setLocal".

Done.

[Avi (use Gerrit), 2013-10-04 21:59:01]

SLGTM; give it a try.

https://codereview.chromium.org/24451003/diff/93001/chrome/browser/net/client...
File chrome/browser/net/client_hints.cc (right):

https://codereview.chromium.org/24451003/diff/93001/chrome/browser/net/client...
chrome/browser/net/client_hints.cc:66: *it = '.';
:( I wish we could StringPrintf according to a provided locale rather than have
to do a decimal point switch afterwards.

[commit-bot: I haz the power, 2013-10-04 22:17:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bengr@chromium.org/24451003/93001

[commit-bot: I haz the power, 2013-10-05 05:07:25]

Change committed as 227178

[cbiesinger, 2013-10-24 00:02:30]

On 2013/10/05 05:07:25, I haz the power (commit-bot) wrote:
> Change committed as 227178

So out of curiosity, why is this implemented in browser/ instead of on the blink
side? Parts of this will have to be implemented on blink, and it's awkward to
have it split up like that :/

(I need to figure out how to access the command-line switch from blink, which
may not be possible, being a different process and all...)