Reuse the code within UpdateCheckMark()

Reuse the code within UpdateCheckMark()

Move UpdateCheckMark() from anonymous namespace of tray_accessibility.cc to tray_popup_utils.h
Then, it can be reused by AudioDetailedView and CommonPaletteTool.

BUG=707851
Review-Url: https://codereview.chromium.org/2803893002
Cr-Commit-Position: refs/heads/master@{#463020}
Committed: https://chromium.googlesource.com/chromium/src/+/c345ab7d5a956251b46048138972883c89337d8e

[minch1, 2017-04-05 23:37:52]

Description was changed from

==========
Reuse the logic of UpdateCheckMark(). This cl splits the update to check and
uncheck. Create two files ash/common/system/utils.h and                 
ash/common/system/utils.cc     to place the functions that be reused.

BUG=707851
==========

to

==========
Reuse the logic of UpdateCheckMark(). This cl splits the update to check and
uncheck. Create two files ash/common/system/utils.h and                 
ash/common/system/utils.cc     to place the functions that be reused.

BUG=707851
==========

[minch1, 2017-04-05 23:38:32]

Description was changed from

==========
Reuse the logic of UpdateCheckMark(). This cl splits the update to check and
uncheck. Create two files ash/common/system/utils.h and                 
ash/common/system/utils.cc     to place the functions that be reused.

BUG=707851
==========

to

==========
Reuse the logic of UpdateCheckMark(). 
This cl splits the update to check and uncheck. 
Create two files ash/common/system/utils.h and                  
                 ash/common/system/utils.cc    
     to place the functions that be reused.

BUG=707851
==========

[minch1, 2017-04-05 23:44:06]

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

[commit-bot: I haz the power, 2017-04-05 23:44:46]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-06 00:19:57]

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

[commit-bot: I haz the power, 2017-04-06 00:19:58]

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

[minch1, 2017-04-06 01:04:09]

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

[commit-bot: I haz the power, 2017-04-06 01:04:52]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[minch1, 2017-04-06 01:05:54]

Description was changed from

==========
Reuse the logic of UpdateCheckMark(). 
This cl splits the update to check and uncheck. 
Create two files ash/common/system/utils.h and                  
                 ash/common/system/utils.cc    
     to place the functions that be reused.

BUG=707851
==========

to

==========
Reuse the logic of UpdateCheckMark().
 
This cl splits the update to check and uncheck. 
Add two functions in ash/common/system/tray/tray_utils.h

void CheckMark(HoverHighlightView* container);
void UncheckMark(HoverHighlightView* container);
   
     to be reused by some other files.

BUG=707851
==========

[minch1, 2017-04-06 01:29:33]

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

[commit-bot: I haz the power, 2017-04-06 01:30:00]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[minch1, 2017-04-06 01:31:29]

minch@chromium.org changed reviewers:
+ tdanderson@chromium.org

[minch1, 2017-04-06 01:31:58]

Hi, tdanderson@, can you help review? Thanks.

[commit-bot: I haz the power, 2017-04-06 02:18:11]

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

[commit-bot: I haz the power, 2017-04-06 02:18:12]

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

[minch1, 2017-04-06 04:32:52]

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

[commit-bot: I haz the power, 2017-04-06 04:33:02]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[minch1, 2017-04-06 05:03:13]

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

[commit-bot: I haz the power, 2017-04-06 05:03:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-06 05:42:00]

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

[commit-bot: I haz the power, 2017-04-06 05:42:01]

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

[minch1, 2017-04-06 16:43:08]

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

[commit-bot: I haz the power, 2017-04-06 16:44:11]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[minch1, 2017-04-06 17:24:12]

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

[commit-bot: I haz the power, 2017-04-06 17:24:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tdanderson, 2017-04-06 17:52:09]

Hi, please see my comments below. I was going to suggest including
ash/common/system/chromeos/ime_menu/ime_list_view.cc in this refactoring since
it also has checkable rows, but because that class doesn't use a
HoverHighlightView then I think some more cleanup would need to take place
first. So don't worry about ime_list_view for now; I'll put a bit more thought
into that and track that work separately.

https://codereview.chromium.org/2803893002/diff/60001/ash/common/system/chrom...
File ash/common/system/chromeos/audio/audio_detailed_view.cc (right):

https://codereview.chromium.org/2803893002/diff/60001/ash/common/system/chrom...
ash/common/system/chromeos/audio/audio_detailed_view.cc:114: if (checked) {
nit: In general in the code base we avoid braces for conditionals with a single
line body (https://google.github.io/styleguide/cppguide.html#Conditionals). So
here (and elsewhere in this CL) you should format as:

if (checked)
  SetCheckMarkVisible(container);


We usually only include braces if the body is multiline:

if (condition) {
  x++;
  y--;
}

or if the condition itself is multiline:

if (some_long_condition_over_80_chars &&
    something_else) {
  foo();
}

or if it is part of an if / else if / else if where at least one of those parts
is multiline:

if (condition) {
  x++;
} else if (condition2) {
  y++;
  z--;
} else {
  x += y;
}

https://codereview.chromium.org/2803893002/diff/60001/ash/common/system/chrom...
File ash/common/system/chromeos/palette/common_palette_tool.cc (right):

https://codereview.chromium.org/2803893002/diff/60001/ash/common/system/chrom...
ash/common/system/chromeos/palette/common_palette_tool.cc:82: views::View*
CommonPaletteTool::CreateDefaultView(const base::string16& name) {
There is still some code in here that is common to what is done in the a11y and
audio classes (e.g., creating the vector icon, adding the right icon). Consider
if there any ways you can move that code someplace common, too.

https://codereview.chromium.org/2803893002/diff/60001/ash/common/system/tray/...
File ash/common/system/tray/tray_utils.cc (right):

https://codereview.chromium.org/2803893002/diff/60001/ash/common/system/tray/...
ash/common/system/tray/tray_utils.cc:36:
HoverHighlightView::AccessibilityState::UNCHECKED_CHECKBOX);
The two new functions you have introduced here are quite similar, and having two
functions instead of one requires more code at each call site. As one possible
idea, consider something such as:

UpdateCheckMarkVisibility(container, checked)

instead of:

if (checked)
  SetCheckMarkVisible(container);
else
  SetCheckMarkInvisible(container);

https://codereview.chromium.org/2803893002/diff/60001/ash/common/system/tray/...
File ash/common/system/tray/tray_utils.h (right):

https://codereview.chromium.org/2803893002/diff/60001/ash/common/system/tray/...
ash/common/system/tray/tray_utils.h:5: #ifndef
ASH_COMMON_SYSTEM_TRAY_TRAY_UTILS_H_
Ah, I didn't realize this file only had one thing in it. We should probably just
remove it and place SetupLabelForTray in tray_popup_utils.h instead (I can do
that in a separate CL). In the meantime can you please put any new functions in
tray_popup_utils.h instead?

[minch1, 2017-04-06 18:05:47]

Hi, tdanderson@, can you help check the comment?

https://codereview.chromium.org/2803893002/diff/60001/ash/common/system/tray/...
File ash/common/system/tray/tray_utils.cc (right):

https://codereview.chromium.org/2803893002/diff/60001/ash/common/system/tray/...
ash/common/system/tray/tray_utils.cc:36:
HoverHighlightView::AccessibilityState::UNCHECKED_CHECKBOX);
On 2017/04/06 17:52:09, tdanderson wrote:
> The two new functions you have introduced here are quite similar, and having
two
> functions instead of one requires more code at each call site. As one possible
> idea, consider something such as:
> 
> UpdateCheckMarkVisibility(container, checked)
> 
> instead of:
> 
> if (checked)
>   SetCheckMarkVisible(container);
> else
>   SetCheckMarkInvisible(container);

The logic of enable and disable check mark in CommonPaletteTool is separate.
That's the reason I introduced two functions here. And there is no
AddRightIcon() in CommonPaletteTool too. So actually I introduced three
functions here. Can  you help check PS#5 for this?

[minch1, 2017-04-06 18:09:41]

https://codereview.chromium.org/2803893002/diff/60001/ash/common/system/tray/...
File ash/common/system/tray/tray_utils.cc (right):

https://codereview.chromium.org/2803893002/diff/60001/ash/common/system/tray/...
ash/common/system/tray/tray_utils.cc:36:
HoverHighlightView::AccessibilityState::UNCHECKED_CHECKBOX);
On 2017/04/06 18:05:47, minch1 wrote:
> On 2017/04/06 17:52:09, tdanderson wrote:
> > The two new functions you have introduced here are quite similar, and having
> two
> > functions instead of one requires more code at each call site. As one
possible
> > idea, consider something such as:
> > 
> > UpdateCheckMarkVisibility(container, checked)
> > 
> > instead of:
> > 
> > if (checked)
> >   SetCheckMarkVisible(container);
> > else
> >   SetCheckMarkInvisible(container);
> 
> The logic of enable and disable check mark in CommonPaletteTool is separate.
> That's the reason I introduced two functions here. And there is no
> AddRightIcon() in CommonPaletteTool too. So actually I introduced three
> functions here. Can  you help check PS#5 for this?

Maybe I can add one more function here to combine these two functions (so there
will have 4 funcs here), then there will be only one line in the call site?

[commit-bot: I haz the power, 2017-04-06 18:10:17]

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

[commit-bot: I haz the power, 2017-04-06 18:10:18]

Dry run: This issue passed the CQ dry run.

[tdanderson, 2017-04-06 18:16:16]

On 2017/04/06 18:05:47, minch1 wrote:
> Hi, tdanderson@, can you help check the comment?
> 
>
https://codereview.chromium.org/2803893002/diff/60001/ash/common/system/tray/...
> File ash/common/system/tray/tray_utils.cc (right):
> 
>
https://codereview.chromium.org/2803893002/diff/60001/ash/common/system/tray/...
> ash/common/system/tray/tray_utils.cc:36:
> HoverHighlightView::AccessibilityState::UNCHECKED_CHECKBOX);
> On 2017/04/06 17:52:09, tdanderson wrote:
> > The two new functions you have introduced here are quite similar, and having
> two
> > functions instead of one requires more code at each call site. As one
possible
> > idea, consider something such as:
> > 
> > UpdateCheckMarkVisibility(container, checked)
> > 
> > instead of:
> > 
> > if (checked)
> >   SetCheckMarkVisible(container);
> > else
> >   SetCheckMarkInvisible(container);
> 
> The logic of enable and disable check mark in CommonPaletteTool is separate.
> That's the reason I introduced two functions here. And there is no
> AddRightIcon() in CommonPaletteTool too. So actually I introduced three
> functions here. Can  you help check PS#5 for this?

I'm not sure what you mean by this comment. In patch set 5 you have two 
functions SetCheckMarkVisible() and SetCheckMarkInvisible(). These do
not need to be two separate functions; you can combine them into a single
function that takes in a bool indicating whether the checkmark is visible
or not.

And in CommonPaletteTool::CreateDefaultView() there is a call to
AddRightIcon() on line 91. Furthermore, all three classes (a11y,
audio, palette) also make a call to CreateVectorIcon(kCheckCircleIcon, ...),
which I believe can also be part of your refactoring.

[minch1, 2017-04-06 20:38:05]

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

[commit-bot: I haz the power, 2017-04-06 20:39:34]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[minch1, 2017-04-06 20:43:16]

Description was changed from

==========
Reuse the logic of UpdateCheckMark().
 
This cl splits the update to check and uncheck. 
Add two functions in ash/common/system/tray/tray_utils.h

void CheckMark(HoverHighlightView* container);
void UncheckMark(HoverHighlightView* container);
   
     to be reused by some other files.

BUG=707851
==========

to

==========
Move UpdateCheckMark() from anonymous namespace of tray_accessibility.cc to
tray_popup_utils.h
Then, it can be reused by AudioDetailedView and CommonPaletteTool.

BUG=707851
==========

[minch1, 2017-04-06 21:09:35]

Description was changed from

==========
Move UpdateCheckMark() from anonymous namespace of tray_accessibility.cc to
tray_popup_utils.h
Then, it can be reused by AudioDetailedView and CommonPaletteTool.

BUG=707851
==========

to

==========
Move UpdateCheckMark() from anonymous namespace of tray_accessibility.cc to
tray_popup_utils.h
Then, it can be reused by AudioDetailedView and CommonPaletteTool.

Note:
In CommonPaletteTool, originally it CreateVectorIcon with kMenuIconSize = 20. In
order to reuse the code, I make it use the default size. If this doesn't work, I
will try to rewrite the Update func.

BUG=707851
==========

[minch1, 2017-04-06 21:11:15]

Description was changed from

==========
Move UpdateCheckMark() from anonymous namespace of tray_accessibility.cc to
tray_popup_utils.h
Then, it can be reused by AudioDetailedView and CommonPaletteTool.

Note:
In CommonPaletteTool, originally it CreateVectorIcon with kMenuIconSize = 20. In
order to reuse the code, I make it use the default size. If this doesn't work, I
will try to rewrite the Update func.

BUG=707851
==========

to

==========
Move UpdateCheckMark() from anonymous namespace of tray_accessibility.cc to
tray_popup_utils.h
Then, it can be reused by AudioDetailedView and CommonPaletteTool.

Note:
CommonPaletteTool originally CreateVectorIcon with kMenuIconSize = 20. In order
to reuse the code, I made it use the default size. (If this doesn't work, I will
try to rewrite the Update func.)

BUG=707851
==========

[commit-bot: I haz the power, 2017-04-06 21:28:16]

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

[commit-bot: I haz the power, 2017-04-06 21:28:17]

Dry run: This issue passed the CQ dry run.

[minch1, 2017-04-06 21:30:03]

tdanderson@, can you help take a look again?

[tdanderson, 2017-04-07 17:54:31]

Looking good! I have one more suggestion though, let me know what you think.

https://codereview.chromium.org/2803893002/diff/100001/ash/common/system/tray...
File ash/common/system/tray/tray_popup_utils.cc (right):

https://codereview.chromium.org/2803893002/diff/100001/ash/common/system/tray...
ash/common/system/tray/tray_popup_utils.cc:439: void
TrayPopupUtils::UpdateCheckMarkVisibility(HoverHighlightView* container,
What do you think about splitting this into two helpers like follows:

void TrayPopupUtils::InitializeAsCheckableRow(HoverHighlightView* container,
bool checked) {
  gfxImageSkia check_mark = CreateVectorIcon(...);
  container->AddRightIcon(...);
  UpdateCheckMarkVisibility(container, checked);
}

void TrayPopupUtils::UpdateCheckMarkVisibility(HoverHighlightView* container,
bool visible) {
  container->SetRightViewVisible(checked);
  container->SetAccessibilityState(checked ? CHECKED_CHECKBOX :
UNCHECKED_CHECKBOX);
}

Then everywhere you currently call UpdateCheckMarkVisibility(), you would call
InitializeAsCheckableRow() instead. The advantage of having
UpdateCheckMarkVisibility() as a separate function is that now you can call it
from common_palette_tool (lines 55 and 66) to eliminate the duplicated code
there.

[minch1, 2017-04-07 19:44:39]

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

[commit-bot: I haz the power, 2017-04-07 19:45:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[minch1, 2017-04-07 19:47:48]

Thanks for your suggestion, Terry. Please check the PS#7.

[commit-bot: I haz the power, 2017-04-07 20:29:06]

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

[commit-bot: I haz the power, 2017-04-07 20:29:08]

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

[minch1, 2017-04-07 20:39:30]

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

[commit-bot: I haz the power, 2017-04-07 20:40:02]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tdanderson, 2017-04-07 20:57:32]

PS8 LGTM with a few comments below. Also a nit regarding the CL description:
it's nice to have the first line of the description be the CL title. So:

Reuse the code within UpdateCheckMark()
<blank line>
<the CL description>

The reason is that it makes it easier when people are skimming through git logs.

https://codereview.chromium.org/2803893002/diff/120001/ash/common/system/tray...
File ash/common/system/tray/tray_popup_utils.cc (right):

https://codereview.chromium.org/2803893002/diff/120001/ash/common/system/tray...
ash/common/system/tray/tray_popup_utils.cc:442:
CreateVectorIcon(kCheckCircleIcon, gfx::kGoogleGreen700);
Regarding the "Note" you made in the CL description, it's actually ok not to
pass in a size parameter here (so, what you have here is correct). You can
remove the note from the CL description.

(Side note: When calling CreateVectorIcon() and no size parameter is passed in,
then we try to pull the size information from the corresponding .icon file. In
ash/resources/vector_icons, check_circle.1x.icon specifies a size of 20, so we
do not need to pass it in.)

https://codereview.chromium.org/2803893002/diff/120001/ash/common/system/tray...
File ash/common/system/tray/tray_popup_utils.h (right):

https://codereview.chromium.org/2803893002/diff/120001/ash/common/system/tray...
ash/common/system/tray/tray_popup_utils.h:212: // Update the status of check
mark when click a row in the system menu.
nit: How about something like "Updates the visibility and a11y state of the
checkable row |container|." here?

https://codereview.chromium.org/2803893002/diff/120001/ash/common/system/tray...
File ash/common/system/tray_accessibility.cc (left):

https://codereview.chromium.org/2803893002/diff/120001/ash/common/system/tray...
ash/common/system/tray_accessibility.cc:28: #include "ui/gfx/color_palette.h"
Thanks for keeping the #includes up to date! Most people forget about these
unfortunately.

[commit-bot: I haz the power, 2017-04-07 21:07:41]

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

[commit-bot: I haz the power, 2017-04-07 21:07:42]

Dry run: This issue passed the CQ dry run.

[minch1, 2017-04-07 21:20:24]

Description was changed from

==========
Move UpdateCheckMark() from anonymous namespace of tray_accessibility.cc to
tray_popup_utils.h
Then, it can be reused by AudioDetailedView and CommonPaletteTool.

Note:
CommonPaletteTool originally CreateVectorIcon with kMenuIconSize = 20. In order
to reuse the code, I made it use the default size. (If this doesn't work, I will
try to rewrite the Update func.)

BUG=707851
==========

to

==========
Reuse the code within UpdateCheckMark()

Move UpdateCheckMark() from anonymous namespace of tray_accessibility.cc to
tray_popup_utils.h
Then, it can be reused by AudioDetailedView and CommonPaletteTool.


BUG=707851
==========

[minch1, 2017-04-07 21:20:48]

Description was changed from

==========
Reuse the code within UpdateCheckMark()

Move UpdateCheckMark() from anonymous namespace of tray_accessibility.cc to
tray_popup_utils.h
Then, it can be reused by AudioDetailedView and CommonPaletteTool.


BUG=707851
==========

to

==========
Reuse the code within UpdateCheckMark()

Move UpdateCheckMark() from anonymous namespace of tray_accessibility.cc to
tray_popup_utils.h
Then, it can be reused by AudioDetailedView and CommonPaletteTool.

BUG=707851
==========

[minch1, 2017-04-07 21:23:38]

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

[commit-bot: I haz the power, 2017-04-07 21:24:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-07 21:53:39]

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

[commit-bot: I haz the power, 2017-04-07 21:53:40]

Dry run: This issue passed the CQ dry run.

[minch1, 2017-04-07 21:55:17]

The CQ bit was checked by minch@chromium.org

[minch1, 2017-04-07 21:55:17]

The patchset sent to the CQ was uploaded after l-g-t-m from
tdanderson@chromium.org
Link to the patchset: https://codereview.chromium.org/2803893002/#ps160001
(title: "Update comments.")

[commit-bot: I haz the power, 2017-04-07 21:55:47]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-07 22:17:29]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1491602117236770,
"parent_rev": "3de411ec62537cdf662bf2d2ade0ebc54c3e3450", "commit_rev":
"2f5e3259b67a10f436106149da7a67fcd3021420"}

[commit-bot: I haz the power, 2017-04-07 22:23:42]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1491602117236770,
"parent_rev": "437b4baef6dce3ea2e2da872068f21c2be784306", "commit_rev":
"c345ab7d5a956251b46048138972883c89337d8e"}

[commit-bot: I haz the power, 2017-04-07 22:25:20]

Description was changed from

==========
Reuse the code within UpdateCheckMark()

Move UpdateCheckMark() from anonymous namespace of tray_accessibility.cc to
tray_popup_utils.h
Then, it can be reused by AudioDetailedView and CommonPaletteTool.

BUG=707851
==========

to

==========
Reuse the code within UpdateCheckMark()

Move UpdateCheckMark() from anonymous namespace of tray_accessibility.cc to
tray_popup_utils.h
Then, it can be reused by AudioDetailedView and CommonPaletteTool.

BUG=707851

Review-Url: https://codereview.chromium.org/2803893002
Cr-Commit-Position: refs/heads/master@{#463020}
Committed:
https://chromium.googlesource.com/chromium/src/+/c345ab7d5a956251b46048138972...
==========

[commit-bot: I haz the power, 2017-04-07 22:25:21]

Committed patchset #9 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/c345ab7d5a956251b46048138972...