[MD] re-skin extension browser action badges

[MD] re-skin extension browser action badges

In addition to visual updates, badge_util is folded into
IconWithBadgeImageSource which in turn is moved into chrome/browser.

There should be no change unless you pass a --top-chrome-md value.

BUG=560345, 555031
TBR=thakis@chromium.org

Committed: https://crrev.com/6e3b7fab283467bdba8ca5636ab75e1affcf5114
Cr-Commit-Position: refs/heads/master@{#362601}

[Evan Stade, 2015-11-24 02:17:34]

estade@chromium.org changed reviewers:
+ rdevlin.cronin@chromium.org

[Evan Stade, 2015-11-24 02:17:41]

The CQ bit was checked by estade@chromium.org to run a CQ dry run

[Evan Stade, 2015-11-24 02:17:56]

Description was changed from

==========
[MD] re-skin extension browser action badges

In addition to visual updates, badge_util is folded into
IconWithBadgeImageSource.

There should be no change unless you pass a --top-chrome-md value.

BUG=560345
==========

to

==========
[MD] re-skin extension browser action badges

In addition to visual updates, badge_util is folded into
IconWithBadgeImageSource which in turn is moved into chrome/browser.

There should be no change unless you pass a --top-chrome-md value.

BUG=560345
==========

[commit-bot: I haz the power, 2015-11-24 02:21:31]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1472863004/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1472863004/1

[commit-bot: I haz the power, 2015-11-24 03:22:53]

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

[commit-bot: I haz the power, 2015-11-24 03:22:54]

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

[Evan Stade, 2015-11-24 03:29:00]

The CQ bit was checked by estade@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-11-24 03:29:57]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1472863004/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1472863004/20001

[commit-bot: I haz the power, 2015-11-24 03:41:35]

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

[commit-bot: I haz the power, 2015-11-24 03:41:35]

Dry run: Try jobs failed on following builders:
  android_chromium_gn_compile_dbg on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_chromium_gn_compile_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)

[Devlin, 2015-11-24 17:26:15]

https://codereview.chromium.org/1472863004/diff/20001/chrome/browser/ui/exten...
File chrome/browser/ui/extensions/icon_with_badge_image_source.cc (right):

https://codereview.chromium.org/1472863004/diff/20001/chrome/browser/ui/exten...
chrome/browser/ui/extensions/icon_with_badge_image_source.cc:1: // Copyright (c)
2012 The Chromium Authors. All rights reserved.
Can you reduce the similarity count so that Rietveld recognizes this as a move?

https://codereview.chromium.org/1472863004/diff/20001/chrome/browser/ui/toolb...
File chrome/browser/ui/toolbar/toolbar_actions_bar.cc (right):

https://codereview.chromium.org/1472863004/diff/20001/chrome/browser/ui/toolb...
chrome/browser/ui/toolbar/toolbar_actions_bar.cc:55: return 28;
I personally preferred using the real asset, since it's ultimately the source of
truth here.  Do we have an MD asset that we could use?

[Evan Stade, 2015-11-24 18:24:14]

https://codereview.chromium.org/1472863004/diff/20001/chrome/browser/ui/exten...
File chrome/browser/ui/extensions/icon_with_badge_image_source.cc (right):

https://codereview.chromium.org/1472863004/diff/20001/chrome/browser/ui/exten...
chrome/browser/ui/extensions/icon_with_badge_image_source.cc:1: // Copyright (c)
2012 The Chromium Authors. All rights reserved.
On 2015/11/24 17:26:15, Devlin wrote:
> Can you reduce the similarity count so that Rietveld recognizes this as a
move?

Done, but it decided it was a move of badge_util.cc

https://codereview.chromium.org/1472863004/diff/20001/chrome/browser/ui/toolb...
File chrome/browser/ui/toolbar/toolbar_actions_bar.cc (right):

https://codereview.chromium.org/1472863004/diff/20001/chrome/browser/ui/toolb...
chrome/browser/ui/toolbar/toolbar_actions_bar.cc:55: return 28;
On 2015/11/24 17:26:15, Devlin wrote:
> I personally preferred using the real asset, since it's ultimately the source
of
> truth here.  Do we have an MD asset that we could use?

no, there's no asset, which is a good thing, and a general goal of our MD work.

[Devlin, 2015-11-24 23:08:51]

https://codereview.chromium.org/1472863004/diff/60001/chrome/browser/ui/exten...
File chrome/browser/ui/extensions/icon_with_badge_image_source.cc (right):

https://codereview.chromium.org/1472863004/diff/60001/chrome/browser/ui/exten...
chrome/browser/ui/extensions/icon_with_badge_image_source.cc:151:
ResourceBundle* rb = &ResourceBundle::GetSharedInstance();
nitty nit: any reason to not use a reference?

https://codereview.chromium.org/1472863004/diff/60001/chrome/browser/ui/exten...
chrome/browser/ui/extensions/icon_with_badge_image_source.cc:152: gfx::FontList
base_font = rb->GetFontList(ResourceBundle::BaseFont)
nit: const&?

https://codereview.chromium.org/1472863004/diff/60001/chrome/browser/ui/exten...
chrome/browser/ui/extensions/icon_with_badge_image_source.cc:215: // Overlay the
gradient. It is stretchy, so we do this in three parts.
There's enough if (md)/else blocks here that I think this would be more readable
if we had two methods for doing the actual painting, passing in the few shared
params.

Also, a high-level comment or two on the differences between MD and non-MD would
be helpful.

[Evan Stade, 2015-11-24 23:32:47]

https://codereview.chromium.org/1472863004/diff/60001/chrome/browser/ui/exten...
File chrome/browser/ui/extensions/icon_with_badge_image_source.cc (right):

https://codereview.chromium.org/1472863004/diff/60001/chrome/browser/ui/exten...
chrome/browser/ui/extensions/icon_with_badge_image_source.cc:151:
ResourceBundle* rb = &ResourceBundle::GetSharedInstance();
On 2015/11/24 23:08:51, Devlin wrote:
> nitty nit: any reason to not use a reference?

We typically don't use non-const refs for anything. I've been instructed to
avoid the (very common) pattern of

  ResourceBundle& rb = ResourceBundle::GetSharedInstance();

https://codereview.chromium.org/1472863004/diff/60001/chrome/browser/ui/exten...
chrome/browser/ui/extensions/icon_with_badge_image_source.cc:152: gfx::FontList
base_font = rb->GetFontList(ResourceBundle::BaseFont)
On 2015/11/24 23:08:51, Devlin wrote:
> nit: const&?

DeriveWithHeightUpperBound doesn't return const ref

https://codereview.chromium.org/1472863004/diff/60001/chrome/browser/ui/exten...
chrome/browser/ui/extensions/icon_with_badge_image_source.cc:215: // Overlay the
gradient. It is stretchy, so we do this in three parts.
On 2015/11/24 23:08:51, Devlin wrote:
> There's enough if (md)/else blocks here that I think this would be more
readable
> if we had two methods for doing the actual painting, passing in the few shared
> params.


I've reduced the number of IsModeMaterial conditionals but I think the shared
code is too interleaved to factor out cleanly.

It's also relevant that MD is going to be default soon (TM). Hence all this
pre-MD code will soon be removed. It will be much easier to just delete some
conditional blocks than to re-integrate two separate functions.

> 
> Also, a high-level comment or two on the differences between MD and non-MD
would
> be helpful.

again, MD is going to replace non-MD very shortly. In particular, for this UI,
the only reason for them to coexist is to give us a chance to come to an
agreeable MD version before making the switch. We don't have to wait for the
rest of MD to be enabled. Thus I don't think a comment about the difference
would last very long or be very useful in the meantime (although such
information can be found in the CL description or bug report).

[Evan Stade, 2015-11-24 23:32:51]

The CQ bit was checked by estade@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-11-24 23:38:03]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1472863004/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1472863004/80001

[commit-bot: I haz the power, 2015-11-24 23:53:50]

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

[commit-bot: I haz the power, 2015-11-24 23:53:51]

Dry run: Try jobs failed on following builders:
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)

[Devlin, 2015-11-24 23:54:32]

https://codereview.chromium.org/1472863004/diff/60001/chrome/browser/ui/exten...
File chrome/browser/ui/extensions/icon_with_badge_image_source.cc (right):

https://codereview.chromium.org/1472863004/diff/60001/chrome/browser/ui/exten...
chrome/browser/ui/extensions/icon_with_badge_image_source.cc:215: // Overlay the
gradient. It is stretchy, so we do this in three parts.
On 2015/11/24 23:32:47, Evan Stade wrote:
> It's also relevant that MD is going to be default soon (TM).
How soon?

> It will be much easier to just delete some
> conditional blocks than to re-integrate two separate functions.
I'm not entirely sure I agree with this.  It seems like it's pretty easy to just
inline a function, since it's only called in one place.
if (MD)
  DoMDFoo();
else
  DoLegacyFoo();

DoMdFoo() {
  <foo stuff>
}

becomes
<foo stuff>

Also, there's relatively little here that's actually shared between MD and
non-md, and quite a few things that are instantiated for both because they're
used in separate if-else blocks.  E.g., text_paint (only non-md), utf16_text
(only md), base_font (only md)...

Are you sure it's messier to separate these out than keep them together?  (If
you are, then we can keep it this way - just asking you to think about it
again.)

<update> See also latest patch set comment for an orthogonal suggestion that
would also work.

https://codereview.chromium.org/1472863004/diff/80001/chrome/browser/ui/exten...
File chrome/browser/ui/extensions/icon_with_badge_image_source.cc (right):

https://codereview.chromium.org/1472863004/diff/80001/chrome/browser/ui/exten...
chrome/browser/ui/extensions/icon_with_badge_image_source.cc:190: gfx::Rect
rect(badge_width >= kCenterAlignThreshold
Alternatively, instead of making two functions, we can pull this (including
calculating the badge width) into a "get_badge_rect(int text_width)" lambda
above, e.g. under canvas->Save().  If we then also set up rect_paint above, we
could reduce this down to one big if-else - which, IMO, helps with readability
over this, and also gets us better scoping for text_paint, base_font,
utf16_text.

[Evan Stade, 2015-11-25 00:18:20]

https://codereview.chromium.org/1472863004/diff/60001/chrome/browser/ui/exten...
File chrome/browser/ui/extensions/icon_with_badge_image_source.cc (right):

https://codereview.chromium.org/1472863004/diff/60001/chrome/browser/ui/exten...
chrome/browser/ui/extensions/icon_with_badge_image_source.cc:215: // Overlay the
gradient. It is stretchy, so we do this in three parts.
On 2015/11/24 23:54:32, Devlin wrote:
> On 2015/11/24 23:32:47, Evan Stade wrote:
> > It's also relevant that MD is going to be default soon (TM).
> How soon?

Overall, hopefully M50. For this smallish change (and others before it), sooner
than that (ideally the current milestone, m49).

> 
> > It will be much easier to just delete some
> > conditional blocks than to re-integrate two separate functions.
> I'm not entirely sure I agree with this.  It seems like it's pretty easy to
just
> inline a function, since it's only called in one place.
> if (MD)
>   DoMDFoo();
> else
>   DoLegacyFoo();
> 
> DoMdFoo() {
>   <foo stuff>
> }
> 
> becomes
> <foo stuff>

this example assumes no parameters but the original suggestion involved
parameters

> 
> Also, there's relatively little here that's actually shared between MD and
> non-md, and quite a few things that are instantiated for both because they're
> used in separate if-else blocks.  E.g., text_paint (only non-md), utf16_text
> (only md), base_font (only md)...

only base font is instantiated for both, and that's fixable, but doesn't seem
super worth it.

> 
> Are you sure it's messier to separate these out than keep them together?  (If
> you are, then we can keep it this way - just asking you to think about it
> again.)

I'm sure that's what I believe, yes :) It may or may not be what others believe.
When reading this function, whether you're trying to understand MD or pre-MD,
you can read from top to bottom and just skip the parts that aren't relevant. If
you start slicing and dicing, you can't read from top to bottom any more; you
have to skip around.

> 
> <update> See also latest patch set comment for an orthogonal suggestion that
> would also work.

I would rather just copy paste the function and have two copies of the same code
(temporarily of course) than put effort into disentangling and then
re-entangling.

[Devlin, 2015-12-01 21:06:42]

lgtm

https://codereview.chromium.org/1472863004/diff/60001/chrome/browser/ui/exten...
File chrome/browser/ui/extensions/icon_with_badge_image_source.cc (right):

https://codereview.chromium.org/1472863004/diff/60001/chrome/browser/ui/exten...
chrome/browser/ui/extensions/icon_with_badge_image_source.cc:215: // Overlay the
gradient. It is stretchy, so we do this in three parts.
On 2015/11/25 00:18:20, Evan Stade wrote:
> On 2015/11/24 23:54:32, Devlin wrote:
> > On 2015/11/24 23:32:47, Evan Stade wrote:
> > > It's also relevant that MD is going to be default soon (TM).
> > How soon?
> 
> Overall, hopefully M50. For this smallish change (and others before it),
sooner
> than that (ideally the current milestone, m49).
> 
> > 
> > > It will be much easier to just delete some
> > > conditional blocks than to re-integrate two separate functions.
> > I'm not entirely sure I agree with this.  It seems like it's pretty easy to
> just
> > inline a function, since it's only called in one place.
> > if (MD)
> >   DoMDFoo();
> > else
> >   DoLegacyFoo();
> > 
> > DoMdFoo() {
> >   <foo stuff>
> > }
> > 
> > becomes
> > <foo stuff>
> 
> this example assumes no parameters but the original suggestion involved
> parameters
> 
> > 
> > Also, there's relatively little here that's actually shared between MD and
> > non-md, and quite a few things that are instantiated for both because
they're
> > used in separate if-else blocks.  E.g., text_paint (only non-md), utf16_text
> > (only md), base_font (only md)...
> 
> only base font is instantiated for both, and that's fixable, but doesn't seem
> super worth it.
> 
> > 
> > Are you sure it's messier to separate these out than keep them together? 
(If
> > you are, then we can keep it this way - just asking you to think about it
> > again.)
> 
> I'm sure that's what I believe, yes :) It may or may not be what others
believe.
> When reading this function, whether you're trying to understand MD or pre-MD,
> you can read from top to bottom and just skip the parts that aren't relevant.
If
> you start slicing and dicing, you can't read from top to bottom any more; you
> have to skip around.
> 
> > 
> > <update> See also latest patch set comment for an orthogonal suggestion that
> > would also work.
> 
> I would rather just copy paste the function and have two copies of the same
code
> (temporarily of course) than put effort into disentangling and then
> re-entangling.

I'm not sure I agree, but it's not worth arguing about for code that's hopefully
fairly transient. :)

https://codereview.chromium.org/1472863004/diff/100001/chrome/browser/ui/tool...
File chrome/browser/ui/toolbar/toolbar_actions_bar.cc (right):

https://codereview.chromium.org/1472863004/diff/100001/chrome/browser/ui/tool...
chrome/browser/ui/toolbar/toolbar_actions_bar.cc:55: return 28;
Is this guaranteed to be the same across all platforms?

[Evan Stade, 2015-12-01 23:15:57]

estade@chromium.org changed reviewers:
+ pkasting@chromium.org, tdanderson@chromium.org

[Evan Stade, 2015-12-01 23:15:58]

https://codereview.chromium.org/1472863004/diff/100001/chrome/browser/ui/tool...
File chrome/browser/ui/toolbar/toolbar_actions_bar.cc (right):

https://codereview.chromium.org/1472863004/diff/100001/chrome/browser/ui/tool...
chrome/browser/ui/toolbar/toolbar_actions_bar.cc:55: return 28;
On 2015/12/01 21:06:42, Devlin wrote:
> Is this guaranteed to be the same across all platforms?

if you mean OSX vs. Windows, yes. I think it might need to change for hybrid
material mode. It would probably be best to use
GetLayoutConstant(LOCATION_BAR_HEIGHT), and to use that I've had to move the
file out of views/

+pkasting, +tdanderson

[Evan Stade, 2015-12-01 23:18:39]

estade@chromium.org changed reviewers:
+ thakis@chromium.org

[Evan Stade, 2015-12-01 23:18:40]

+thakis as well for chrome/common stamp (deletions only)

[Evan Stade, 2015-12-01 23:18:52]

The CQ bit was checked by estade@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-12-01 23:21:31]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1472863004/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1472863004/120001

[commit-bot: I haz the power, 2015-12-01 23:43:55]

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

[commit-bot: I haz the power, 2015-12-01 23:43:55]

Dry run: Try jobs failed on following builders:
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Peter Kasting, 2015-12-02 00:03:12]

RS LGTM on moving layout constants.  Go ahead and file a bug (and maybe add a
TODO to the header?) about deciding whether it makes sense to hide some of these
values if they're e.g. views-only, and how to do that.

[Evan Stade, 2015-12-02 00:38:47]

The CQ bit was checked by estade@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-12-02 00:41:26]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1472863004/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1472863004/120001

[tdanderson, 2015-12-02 00:57:10]

lgtm with one comment below.

https://codereview.chromium.org/1472863004/diff/100001/chrome/browser/ui/tool...
File chrome/browser/ui/toolbar/toolbar_actions_bar.cc (right):

https://codereview.chromium.org/1472863004/diff/100001/chrome/browser/ui/tool...
chrome/browser/ui/toolbar/toolbar_actions_bar.cc:55: return 28;
On 2015/12/01 23:15:58, Evan Stade wrote:
> On 2015/12/01 21:06:42, Devlin wrote:
> > Is this guaranteed to be the same across all platforms?
> 
> if you mean OSX vs. Windows, yes. I think it might need to change for hybrid
> material mode. It would probably be best to use
> GetLayoutConstant(LOCATION_BAR_HEIGHT), and to use that I've had to move the
> file out of views/
> 
> +pkasting, +tdanderson

Actually there is no difference between material and hybrid in this case. Both
should be 28 so using LOCATION_BAR_HEIGHT here is incorrect.

[Evan Stade, 2015-12-02 01:02:38]

Description was changed from

==========
[MD] re-skin extension browser action badges

In addition to visual updates, badge_util is folded into
IconWithBadgeImageSource which in turn is moved into chrome/browser.

There should be no change unless you pass a --top-chrome-md value.

BUG=560345
==========

to

==========
[MD] re-skin extension browser action badges

In addition to visual updates, badge_util is folded into
IconWithBadgeImageSource which in turn is moved into chrome/browser.

There should be no change unless you pass a --top-chrome-md value.

BUG=560345,555031
==========

[Evan Stade, 2015-12-02 01:42:29]

On 2015/12/02 00:57:10, tdanderson wrote:
> lgtm with one comment below.
> 
>
https://codereview.chromium.org/1472863004/diff/100001/chrome/browser/ui/tool...
> File chrome/browser/ui/toolbar/toolbar_actions_bar.cc (right):
> 
>
https://codereview.chromium.org/1472863004/diff/100001/chrome/browser/ui/tool...
> chrome/browser/ui/toolbar/toolbar_actions_bar.cc:55: return 28;
> On 2015/12/01 23:15:58, Evan Stade wrote:
> > On 2015/12/01 21:06:42, Devlin wrote:
> > > Is this guaranteed to be the same across all platforms?
> > 
> > if you mean OSX vs. Windows, yes. I think it might need to change for hybrid
> > material mode. It would probably be best to use
> > GetLayoutConstant(LOCATION_BAR_HEIGHT), and to use that I've had to move the
> > file out of views/
> > 
> > +pkasting, +tdanderson
> 
> Actually there is no difference between material and hybrid in this case. Both
> should be 28 so using LOCATION_BAR_HEIGHT here is incorrect.

ah, ok. I've undone the file move. Going to TBR thakis for chrome/common

[Evan Stade, 2015-12-02 01:42:42]

Description was changed from

==========
[MD] re-skin extension browser action badges

In addition to visual updates, badge_util is folded into
IconWithBadgeImageSource which in turn is moved into chrome/browser.

There should be no change unless you pass a --top-chrome-md value.

BUG=560345,555031
==========

to

==========
[MD] re-skin extension browser action badges

In addition to visual updates, badge_util is folded into
IconWithBadgeImageSource which in turn is moved into chrome/browser.

There should be no change unless you pass a --top-chrome-md value.

BUG=560345,555031 
TBR=thakis@chromium.org
==========

[Evan Stade, 2015-12-02 01:42:47]

The CQ bit was checked by estade@chromium.org

[Evan Stade, 2015-12-02 01:42:47]

The patchset sent to the CQ was uploaded after l-g-t-m from
rdevlin.cronin@chromium.org, tdanderson@chromium.org, pkasting@chromium.org
Link to the patchset: https://codereview.chromium.org/1472863004/#ps140001
(title: "undo file move")

[commit-bot: I haz the power, 2015-12-02 01:43:20]

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

[Nico, 2015-12-02 02:45:52]

chrome common rs-lgt

[Nico, 2015-12-02 02:46:01]

rs-lgtm too

[commit-bot: I haz the power, 2015-12-02 02:46:55]

Description was changed from

==========
[MD] re-skin extension browser action badges

In addition to visual updates, badge_util is folded into
IconWithBadgeImageSource which in turn is moved into chrome/browser.

There should be no change unless you pass a --top-chrome-md value.

BUG=560345,555031 
TBR=thakis@chromium.org
==========

to

==========
[MD] re-skin extension browser action badges

In addition to visual updates, badge_util is folded into
IconWithBadgeImageSource which in turn is moved into chrome/browser.

There should be no change unless you pass a --top-chrome-md value.

BUG=560345,555031 
TBR=thakis@chromium.org
==========

[commit-bot: I haz the power, 2015-12-02 02:46:56]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2015-12-02 02:47:54]

Description was changed from

==========
[MD] re-skin extension browser action badges

In addition to visual updates, badge_util is folded into
IconWithBadgeImageSource which in turn is moved into chrome/browser.

There should be no change unless you pass a --top-chrome-md value.

BUG=560345,555031 
TBR=thakis@chromium.org
==========

to

==========
[MD] re-skin extension browser action badges

In addition to visual updates, badge_util is folded into
IconWithBadgeImageSource which in turn is moved into chrome/browser.

There should be no change unless you pass a --top-chrome-md value.

BUG=560345,555031
TBR=thakis@chromium.org

Committed: https://crrev.com/6e3b7fab283467bdba8ca5636ab75e1affcf5114
Cr-Commit-Position: refs/heads/master@{#362601}
==========

[commit-bot: I haz the power, 2015-12-02 02:47:55]

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