Polish the result communication and display of the browsing data counters.

Polish the result communication and display of the browsing data counters.

So far, counters only supported a single int as a result. This was already insufficient, as the history counter needs to return a <int, bool> pair, which had to be encoded into a single int. For the upcoming autofill counter, which returns 3 ints, this would be even worse.

This CL restructures how the result is reported and interpreted. Counters now return a Result class with raw data instead of an int. The interpretation of that class is now the responsibility of the frontend.

Strings on the frontend are crafted according to https://folio.googleplex.com/clear-browsing-data/Data%20counters/V1 .

BUG=510028
Committed: https://crrev.com/6bb4dffab7e267d535162a2b17869db4b8662881
Cr-Commit-Position: refs/heads/master@{#357558}

[msramek, 2015-11-02 15:47:58]

Description was changed from

==========
Polish the result communication and display of the browsing data counters.

On the backend side, refactor the result reporting to return a complex result
instead of a single int. This is necessary, as history counter needs to return
an <int, bool> pair (until now squeezed into a single int), and the upcoming
autofill counter will need to return 3 ints.

On the frontend, display strings according to
https://folio.googleplex.com/clear-browsing-data/Data%20counters/V1 .

BUG=510028
==========

to

==========
Polish the result communication and display of the browsing data counters.

So far, counters only supported a single int as a result. This was already
insufficient, as the history counter needs to return a <int, bool> pair, which
had to be encoded into a single int. For the upcoming autofill counter, which
returns 3 ints, this would be even worse.

This CL restructures how the result is reported and interpreted. Counters now
return a Result class with raw data instead of an int. The interpretation of
that class is now the responsibility of the frontend.

Strings on the frontend are crafted according to
https://folio.googleplex.com/clear-browsing-data/Data%20counters/V1 .

BUG=510028
==========

[msramek, 2015-11-02 15:48:49]

Patchset #1 (id:1) has been deleted

[msramek, 2015-11-02 15:52:23]

Patchset #1 (id:20001) has been deleted

[msramek, 2015-11-02 15:56:30]

Patchset #1 (id:40001) has been deleted

[msramek, 2015-11-02 15:57:17]

msramek@chromium.org changed reviewers:
+ bauerb@chromium.org

[msramek, 2015-11-02 15:57:18]

Hi Bernhard,

could you please have a look at this?

Thanks,
Martin

[Bernhard Bauer, 2015-11-02 16:15:27]

https://codereview.chromium.org/1420013004/diff/60001/chrome/app/generated_re...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/1420013004/diff/60001/chrome/app/generated_re...
chrome/app/generated_resources.grd:7702: +      <message
name="IDS_DEL_CACHE_COUNTER_ALMOST_EMPTY" desc="A counter showing that the
user's cache has less than 1 MB of data.">
Use the previous message and pass in ui::FormatBytes(1024*1024)? That would also
allow you to tweak the threshold more easily.

https://codereview.chromium.org/1420013004/diff/60001/chrome/app/generated_re...
chrome/app/generated_resources.grd:7710: +      <message
name="IDS_DEL_PASSWORDS_COUNTER_EMPTY" desc="A counter showing that the user has
no passwords.">
This seems like it should go into the previous message, using the ICU
pluralization syntax.

https://codereview.chromium.org/1420013004/diff/60001/chrome/browser/browsing...
File chrome/browser/browsing_data/browsing_data_counter.h (right):

https://codereview.chromium.org/1420013004/diff/60001/chrome/browser/browsing...
chrome/browser/browsing_data/browsing_data_counter.h:22: Result(const
BrowsingDataCounter* source, bool finished, ResultInt value);
Do you want someone to construct object of this class? If not, make the
constructor public.

https://codereview.chromium.org/1420013004/diff/60001/chrome/browser/browsing...
chrome/browser/browsing_data/browsing_data_counter.h:31: bool finished_;
Hm... seeing as we have this class structure now, and it looks like |finished_|
and |value_| are going to be mutually exclusive, would it make sense to move
|value_| to FinishedResult and make both Value() and Finished() virtual?

https://codereview.chromium.org/1420013004/diff/60001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/options/clear_browser_data_handler.cc (right):

https://codereview.chromium.org/1420013004/diff/60001/chrome/browser/ui/webui...
chrome/browser/ui/webui/options/clear_browser_data_handler.cc:340:
BrowsingDataCounter::ResultInt value = result->value();
Name this variable so that it reflects what the value means, i.e. something like
|cacheSizeBytes|.

https://codereview.chromium.org/1420013004/diff/60001/chrome/browser/ui/webui...
chrome/browser/ui/webui/options/clear_browser_data_handler.cc:349: static const
int kBInMB = 1024 * 1024;
The name of this variable isn't really right. This is one megabyte in bytes, no?
Then, in keeping with the rule of naming quantities prefixed with their unit, it
would be kMegabyteBytes.

[msramek, 2015-11-02 20:54:06]

https://codereview.chromium.org/1420013004/diff/60001/chrome/app/generated_re...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/1420013004/diff/60001/chrome/app/generated_re...
chrome/app/generated_resources.grd:7702: +      <message
name="IDS_DEL_CACHE_COUNTER_ALMOST_EMPTY" desc="A counter showing that the
user's cache has less than 1 MB of data.">
On 2015/11/02 16:15:27, Bernhard Bauer wrote:
> Use the previous message and pass in ui::FormatBytes(1024*1024)? That would
also
> allow you to tweak the threshold more easily. 

Ah, didn't notice :-)

https://codereview.chromium.org/1420013004/diff/60001/chrome/app/generated_re...
chrome/app/generated_resources.grd:7710: +      <message
name="IDS_DEL_PASSWORDS_COUNTER_EMPTY" desc="A counter showing that the user has
no passwords.">
On 2015/11/02 16:15:27, Bernhard Bauer wrote:
> This seems like it should go into the previous message, using the ICU
> pluralization syntax.

Hm. I was actually hoping for it to be merged with
IDS_DEL_BROWSING_HISTORY_COUNTER_EMPTY, to assure a consistent translation of
"none".

But now that I think of it, that's actually wrong, because the word may differ
e.g. if "password" and "item" have different gender...

Thanks for catching :)

https://codereview.chromium.org/1420013004/diff/60001/chrome/browser/browsing...
File chrome/browser/browsing_data/browsing_data_counter.h (right):

https://codereview.chromium.org/1420013004/diff/60001/chrome/browser/browsing...
chrome/browser/browsing_data/browsing_data_counter.h:22: Result(const
BrowsingDataCounter* source, bool finished, ResultInt value);
On 2015/11/02 16:15:27, Bernhard Bauer wrote:
> Do you want someone to construct object of this class? If not, make the
> constructor public.

That is what I also did originally, I just didn't like to have to make
FinishedResult and PendingResult its friends.

Anyway, according to your other comment, when we make finished virtual and move
value to FinishedResult, there is no difference between Result and
PendingResult. So I removed PendingResult and the constructor can stay public
here.

https://codereview.chromium.org/1420013004/diff/60001/chrome/browser/browsing...
chrome/browser/browsing_data/browsing_data_counter.h:31: bool finished_;
On 2015/11/02 16:15:27, Bernhard Bauer wrote:
> Hm... seeing as we have this class structure now, and it looks like
|finished_|
> and |value_| are going to be mutually exclusive, would it make sense to move
> |value_| to FinishedResult and make both Value() and Finished() virtual?

Yes! Done.

https://codereview.chromium.org/1420013004/diff/60001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/options/clear_browser_data_handler.cc (right):

https://codereview.chromium.org/1420013004/diff/60001/chrome/browser/ui/webui...
chrome/browser/ui/webui/options/clear_browser_data_handler.cc:340:
BrowsingDataCounter::ResultInt value = result->value();
On 2015/11/02 16:15:27, Bernhard Bauer wrote:
> Name this variable so that it reflects what the value means, i.e. something
like
> |cacheSizeBytes|.

Done. Here and elsewhere.

https://codereview.chromium.org/1420013004/diff/60001/chrome/browser/ui/webui...
chrome/browser/ui/webui/options/clear_browser_data_handler.cc:349: static const
int kBInMB = 1024 * 1024;
On 2015/11/02 16:15:27, Bernhard Bauer wrote:
> The name of this variable isn't really right. This is one megabyte in bytes,
no?
> Then, in keeping with the rule of naming quantities prefixed with their unit,
it
> would be kMegabyteBytes.

Hmm, haven't heard of that. In fact,
https://google.github.io/styleguide/cppguide.html#Constant_Names states
kDaysInAWeek, so I would keep kBytesInMegabyte, perhaps just expanding the units
to words.

[Bernhard Bauer, 2015-11-03 10:11:20]

https://codereview.chromium.org/1420013004/diff/60001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/options/clear_browser_data_handler.cc (right):

https://codereview.chromium.org/1420013004/diff/60001/chrome/browser/ui/webui...
chrome/browser/ui/webui/options/clear_browser_data_handler.cc:349: static const
int kBInMB = 1024 * 1024;
On 2015/11/02 20:54:06, msramek wrote:
> On 2015/11/02 16:15:27, Bernhard Bauer wrote:
> > The name of this variable isn't really right. This is one megabyte in bytes,
> no?
> > Then, in keeping with the rule of naming quantities prefixed with their
unit,
> it
> > would be kMegabyteBytes.
> 
> Hmm, haven't heard of that. In fact,
> https://google.github.io/styleguide/cppguide.html#Constant_Names states
> kDaysInAWeek, so I would keep kBytesInMegabyte, perhaps just expanding the
units
> to words.

Hm, I can't find any explicit mention in the style guide, but we definitely do
that (for all variables, not just constants). But writing things out is
definitely descriptive enough too. (Technically it should be kBytesInAMegabyte
though, no? 😃)

https://codereview.chromium.org/1420013004/diff/100001/chrome/browser/browsin...
File chrome/browser/browsing_data/browsing_data_counter.cc (right):

https://codereview.chromium.org/1420013004/diff/100001/chrome/browser/browsin...
chrome/browser/browsing_data/browsing_data_counter.cc:86: return 0;
You could add a NOTREACHED(), or -- depending on how you feel about static casts
-- only implement this method in the subclass.

https://codereview.chromium.org/1420013004/diff/100001/chrome/browser/browsin...
File chrome/browser/browsing_data/browsing_data_counter.h (right):

https://codereview.chromium.org/1420013004/diff/100001/chrome/browser/browsin...
chrome/browser/browsing_data/browsing_data_counter.h:18: typedef uint64_t
ResultInt;
Hm... I guess you need 64 bits? But unless you actually need the value to be
unsigned, prefer a signed type (see
https://google.github.io/styleguide/cppguide.html#Integer_Types).

https://codereview.chromium.org/1420013004/diff/100001/chrome/browser/browsin...
chrome/browser/browsing_data/browsing_data_counter.h:20: class Result {
Add a couple of comments what these classes are for? In particular, that Result
can be used for a pending result.

https://codereview.chromium.org/1420013004/diff/100001/chrome/browser/browsin...
chrome/browser/browsing_data/browsing_data_counter.h:26: virtual bool finished()
const;
Use CamelCase for non-trivial methods (which includes anything virtual).

https://codereview.chromium.org/1420013004/diff/100001/chrome/browser/browsin...
chrome/browser/browsing_data/browsing_data_counter.h:27: virtual const ResultInt
value() const;
The const isn't really necessary if you return a straight value.

https://codereview.chromium.org/1420013004/diff/100001/chrome/browser/browsin...
chrome/browser/browsing_data/browsing_data_counter.h:30: const
BrowsingDataCounter* source_;
DISALLOW_COPY_AND_ASSIGN (you don't want this assignable, right?)

[msramek, 2015-11-03 10:55:46]

https://codereview.chromium.org/1420013004/diff/60001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/options/clear_browser_data_handler.cc (right):

https://codereview.chromium.org/1420013004/diff/60001/chrome/browser/ui/webui...
chrome/browser/ui/webui/options/clear_browser_data_handler.cc:349: static const
int kBInMB = 1024 * 1024;
On 2015/11/03 10:11:19, Bernhard Bauer wrote:
> On 2015/11/02 20:54:06, msramek wrote:
> > On 2015/11/02 16:15:27, Bernhard Bauer wrote:
> > > The name of this variable isn't really right. This is one megabyte in
bytes,
> > no?
> > > Then, in keeping with the rule of naming quantities prefixed with their
> unit,
> > it
> > > would be kMegabyteBytes.
> > 
> > Hmm, haven't heard of that. In fact,
> > https://google.github.io/styleguide/cppguide.html#Constant_Names states
> > kDaysInAWeek, so I would keep kBytesInMegabyte, perhaps just expanding the
> units
> > to words.
> 
> Hm, I can't find any explicit mention in the style guide, but we definitely do
> that (for all variables, not just constants). But writing things out is
> definitely descriptive enough too. (Technically it should be kBytesInAMegabyte
> though, no? 😃)

Yes :-) I wanted to be concise (although it's just one letter), but looking at
other constants throughout Chrome, they're actually quite verbose. So let's make
it at least sound properly and add "a".

https://codereview.chromium.org/1420013004/diff/100001/chrome/browser/browsin...
File chrome/browser/browsing_data/browsing_data_counter.cc (right):

https://codereview.chromium.org/1420013004/diff/100001/chrome/browser/browsin...
chrome/browser/browsing_data/browsing_data_counter.cc:86: return 0;
On 2015/11/03 10:11:19, Bernhard Bauer wrote:
> You could add a NOTREACHED(), or -- depending on how you feel about static
casts
> -- only implement this method in the subclass.

Yes, I wanted to move it to the subclass (if you look at the diff between 2 and
3 you can see a trace of it), but there would be too many static casts. In this
case I'd go with more readable code at callsites rather than semantic
perfection.

But I added a NOTREACHED().

https://codereview.chromium.org/1420013004/diff/100001/chrome/browser/browsin...
File chrome/browser/browsing_data/browsing_data_counter.h (right):

https://codereview.chromium.org/1420013004/diff/100001/chrome/browser/browsin...
chrome/browser/browsing_data/browsing_data_counter.h:18: typedef uint64_t
ResultInt;
On 2015/11/03 10:11:19, Bernhard Bauer wrote:
> Hm... I guess you need 64 bits? But unless you actually need the value to be
> unsigned, prefer a signed type (see
> https://google.github.io/styleguide/cppguide.html#Integer_Types).

I definitely need more than 32 bits, as I'm summing the sizes of two caches,
which are both 32-bits numbers. And I also heard that we might get rid of this
limit for caches (and max size will be 64 bit).

But I guess that nothing I'm counting will actually reach the size where I would
be worried about that last bit, so I guess signed is ok.

https://codereview.chromium.org/1420013004/diff/100001/chrome/browser/browsin...
chrome/browser/browsing_data/browsing_data_counter.h:20: class Result {
On 2015/11/03 10:11:20, Bernhard Bauer wrote:
> Add a couple of comments what these classes are for? In particular, that
Result
> can be used for a pending result.

Done.

https://codereview.chromium.org/1420013004/diff/100001/chrome/browser/browsin...
chrome/browser/browsing_data/browsing_data_counter.h:26: virtual bool finished()
const;
On 2015/11/03 10:11:20, Bernhard Bauer wrote:
> Use CamelCase for non-trivial methods (which includes anything virtual).

Done.

https://codereview.chromium.org/1420013004/diff/100001/chrome/browser/browsin...
chrome/browser/browsing_data/browsing_data_counter.h:27: virtual const ResultInt
value() const;
On 2015/11/03 10:11:20, Bernhard Bauer wrote:
> The const isn't really necessary if you return a straight value.

Done. It was originally a reference, I had to remove it otherwise I wouldn't be
able to "return 0" in Result (without defining additional member). But it's just
an int, so that's not a problem.

...also it's apparently a compilation error on Mac.

https://codereview.chromium.org/1420013004/diff/100001/chrome/browser/browsin...
chrome/browser/browsing_data/browsing_data_counter.h:30: const
BrowsingDataCounter* source_;
On 2015/11/03 10:11:19, Bernhard Bauer wrote:
> DISALLOW_COPY_AND_ASSIGN (you don't want this assignable, right?)

Done. I need to get used to this.

[Bernhard Bauer, 2015-11-03 11:37:02]

https://codereview.chromium.org/1420013004/diff/140001/chrome/browser/browsin...
File chrome/browser/browsing_data/browsing_data_counter.h (right):

https://codereview.chromium.org/1420013004/diff/140001/chrome/browser/browsin...
chrome/browser/browsing_data/browsing_data_counter.h:30: virtual ResultInt
Value() const;
Add a comment that this method should only be called for finished results?

https://codereview.chromium.org/1420013004/diff/140001/chrome/browser/browsin...
File chrome/browser/browsing_data/cache_counter_browsertest.cc (right):

https://codereview.chromium.org/1420013004/diff/140001/chrome/browser/browsin...
chrome/browser/browsing_data/cache_counter_browsertest.cc:136: result_ =
result->Value();
...aand this would crash now, no?

[msramek, 2015-11-03 11:57:07]

...ok, this time I actually ran the tests before uploading :)

https://codereview.chromium.org/1420013004/diff/140001/chrome/browser/browsin...
File chrome/browser/browsing_data/browsing_data_counter.h (right):

https://codereview.chromium.org/1420013004/diff/140001/chrome/browser/browsin...
chrome/browser/browsing_data/browsing_data_counter.h:30: virtual ResultInt
Value() const;
On 2015/11/03 11:37:02, Bernhard Bauer wrote:
> Add a comment that this method should only be called for finished results?

Done.

https://codereview.chromium.org/1420013004/diff/140001/chrome/browser/browsin...
File chrome/browser/browsing_data/cache_counter_browsertest.cc (right):

https://codereview.chromium.org/1420013004/diff/140001/chrome/browser/browsin...
chrome/browser/browsing_data/cache_counter_browsertest.cc:136: result_ =
result->Value();
On 2015/11/03 11:37:02, Bernhard Bauer wrote:
> ...aand this would crash now, no?

Hm :/ If I need to guard access to Value() with Finished(), it's not that much
different than as if I had to also cast to FinishedResult.

My preference would be to keep it simple and and just remove NOTREACHED, since
unfinished computation implies that the result value cannot be relied on.
Currently, it's up to the UI layer to understand and interpret that.

The other option is to actually move Value() to FinishedResult. It will be
cleaner, but more code on the callsites.

[Bernhard Bauer, 2015-11-03 12:07:09]

https://codereview.chromium.org/1420013004/diff/140001/chrome/browser/browsin...
File chrome/browser/browsing_data/cache_counter_browsertest.cc (right):

https://codereview.chromium.org/1420013004/diff/140001/chrome/browser/browsin...
chrome/browser/browsing_data/cache_counter_browsertest.cc:136: result_ =
result->Value();
On 2015/11/03 11:57:06, msramek wrote:
> On 2015/11/03 11:37:02, Bernhard Bauer wrote:
> > ...aand this would crash now, no?
> 
> Hm :/ If I need to guard access to Value() with Finished(), it's not that much
> different than as if I had to also cast to FinishedResult.
> 
> My preference would be to keep it simple and and just remove NOTREACHED, since
> unfinished computation implies that the result value cannot be relied on.
> Currently, it's up to the UI layer to understand and interpret that.
> 
> The other option is to actually move Value() to FinishedResult. It will be
> cleaner, but more code on the callsites.

Or store the result instead of the two values.

[msramek, 2015-11-03 13:06:29]

Patchset #7 (id:180001) has been deleted

[msramek, 2015-11-03 13:15:40]

https://codereview.chromium.org/1420013004/diff/140001/chrome/browser/browsin...
File chrome/browser/browsing_data/cache_counter_browsertest.cc (right):

https://codereview.chromium.org/1420013004/diff/140001/chrome/browser/browsin...
chrome/browser/browsing_data/cache_counter_browsertest.cc:136: result_ =
result->Value();
On 2015/11/03 12:07:08, Bernhard Bauer wrote:
> On 2015/11/03 11:57:06, msramek wrote:
> > On 2015/11/03 11:37:02, Bernhard Bauer wrote:
> > > ...aand this would crash now, no?
> > 
> > Hm :/ If I need to guard access to Value() with Finished(), it's not that
much
> > different than as if I had to also cast to FinishedResult.
> > 
> > My preference would be to keep it simple and and just remove NOTREACHED,
since
> > unfinished computation implies that the result value cannot be relied on.
> > Currently, it's up to the UI layer to understand and interpret that.
> > 
> > The other option is to actually move Value() to FinishedResult. It will be
> > cleaner, but more code on the callsites.
> 
> Or store the result instead of the two values.

If I store it as a Result, I'll still have to do the conversion in GetResult()
and DCHECKs. So it should be converted here when we get the callback anyway. So
I did that, and I left the members stored separately.

In clear_browser_data_handler.cc, I just inlined the two casts, as I find that
more readable than nesting the whole "if (Finished())" branch one level deeper.

https://codereview.chromium.org/1420013004/diff/200001/chrome/app/generated_r...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/1420013004/diff/200001/chrome/app/generated_r...
chrome/app/generated_resources.grd:7700: +         =0 {none}
I just realized that we also removed the word "password(s)" in the new mocks.
Fixed this here.

[Bernhard Bauer, 2015-11-03 14:13:46]

LGTM with nits, and an optional suggestion:

https://codereview.chromium.org/1420013004/diff/200001/chrome/browser/browsin...
File chrome/browser/browsing_data/browsing_data_counter.h (right):

https://codereview.chromium.org/1420013004/diff/200001/chrome/browser/browsin...
chrome/browser/browsing_data/browsing_data_counter.h:23: // |Value()| is not
valid until the counting has |Finished()|.
This is a bit confusing on its own, as there is no Value() method.

https://codereview.chromium.org/1420013004/diff/200001/chrome/browser/browsin...
chrome/browser/browsing_data/browsing_data_counter.h:50: virtual ResultInt
Value() const;
This doesn't need to be virtual now.

https://codereview.chromium.org/1420013004/diff/200001/chrome/browser/browsin...
File chrome/browser/browsing_data/cache_counter_browsertest.cc (right):

https://codereview.chromium.org/1420013004/diff/200001/chrome/browser/browsin...
chrome/browser/browsing_data/cache_counter_browsertest.cc:137: result_ =
static_cast<BrowsingDataCounter::FinishedResult*>(
Basically, now anytime you call Value(), the call is guarded by Finished(), so
you could in principle add Value() back on the base class instead. I'll leave
that up to you though.

[msramek, 2015-11-03 14:54:09]

Thanks, Bernhard!

Sorry for having to iterate several times :/ I guess I'm just doing too many
things at once right now...

https://codereview.chromium.org/1420013004/diff/200001/chrome/browser/browsin...
File chrome/browser/browsing_data/browsing_data_counter.h (right):

https://codereview.chromium.org/1420013004/diff/200001/chrome/browser/browsin...
chrome/browser/browsing_data/browsing_data_counter.h:23: // |Value()| is not
valid until the counting has |Finished()|.
On 2015/11/03 14:13:46, Bernhard Bauer wrote:
> This is a bit confusing on its own, as there is no Value() method.

Done. Weird, I thought I removed the comment already.

https://codereview.chromium.org/1420013004/diff/200001/chrome/browser/browsin...
chrome/browser/browsing_data/browsing_data_counter.h:50: virtual ResultInt
Value() const;
On 2015/11/03 14:13:46, Bernhard Bauer wrote:
> This doesn't need to be virtual now.

Done.

https://codereview.chromium.org/1420013004/diff/200001/chrome/browser/browsin...
File chrome/browser/browsing_data/cache_counter_browsertest.cc (right):

https://codereview.chromium.org/1420013004/diff/200001/chrome/browser/browsin...
chrome/browser/browsing_data/cache_counter_browsertest.cc:137: result_ =
static_cast<BrowsingDataCounter::FinishedResult*>(
On 2015/11/03 14:13:46, Bernhard Bauer wrote:
> Basically, now anytime you call Value(), the call is guarded by Finished(), so
> you could in principle add Value() back on the base class instead. I'll leave
> that up to you though.

I'll leave it as it is now. We'd get rid of the casts, but not of the additional
Finished() checks anyway, so at least the Result semantics are now clear :)

[msramek, 2015-11-03 14:54:53]

The CQ bit was checked by msramek@chromium.org

[msramek, 2015-11-03 14:54:53]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org
Link to the patchset: https://codereview.chromium.org/1420013004/#ps220001
(title: "Nits.")

[commit-bot: I haz the power, 2015-11-03 14:55:26]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1420013004/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1420013004/220001

[commit-bot: I haz the power, 2015-11-03 15:51:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-11-03 15:51:50]

Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[msramek, 2015-11-03 15:59:48]

The CQ bit was checked by msramek@chromium.org

[commit-bot: I haz the power, 2015-11-03 16:00:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1420013004/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1420013004/220001

[commit-bot: I haz the power, 2015-11-03 17:14:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-11-03 17:14:04]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[msramek, 2015-11-03 17:23:59]

The CQ bit was checked by msramek@chromium.org

[commit-bot: I haz the power, 2015-11-03 17:25:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1420013004/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1420013004/220001

[commit-bot: I haz the power, 2015-11-03 18:21:01]

Committed patchset #8 (id:220001)

[commit-bot: I haz the power, 2015-11-03 18:22:42]

Patchset 8 (id:??) landed as
https://crrev.com/6bb4dffab7e267d535162a2b17869db4b8662881
Cr-Commit-Position: refs/heads/master@{#357558}