Update chromeos system menu buttons for MD.

Update chromeos system menu buttons for MD.

Testing:
- for the VPN disconnect button, --shill-stub="vpn=1"
- for the screen tray item, use chrome remote desktop app to share your
  screen
- for the (chrome)cast item, apply this patch: http://hastebin.com/tuxutojiqo.php

BUG=647017
Committed: https://crrev.com/eafebe2c67be335ab6cf75d8794e46e67ae8879c
Cr-Commit-Position: refs/heads/master@{#429363}

[Evan Stade, 2016-10-31 19:32:20]

Description was changed from

==========
Update chromeos system menu buttons for MD.

Testing:
- for the VPN disconnect button, --shill-stub="vpn=1"
- for the screen tray item, use chrome remote desktop app to share your
  screen
- for the (chrome)cast item, ???

BUG=647017
==========

to

==========
Update chromeos system menu buttons for MD.

Testing:
- for the VPN disconnect button, --shill-stub="vpn=1"
- for the screen tray item, use chrome remote desktop app to share your
  screen
- for the (chrome)cast item, apply this patch:
http://hastebin.com/tuxutojiqo.php

BUG=647017
==========

[Evan Stade, 2016-10-31 19:41:09]

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

[tdanderson, 2016-11-01 22:38:50]

LGTM

https://codereview.chromium.org/2463163002/diff/60001/ash/common/system/chrom...
File ash/common/system/chromeos/network/network_state_list_detailed_view.cc
(right):

https://codereview.chromium.org/2463163002/diff/60001/ash/common/system/chrom...
ash/common/system/chromeos/network/network_state_list_detailed_view.cc:575:
DCHECK(!UseMd());
Good idea, thanks for adding this here and elsewhere

https://codereview.chromium.org/2463163002/diff/60001/ash/common/system/chrom...
File ash/common/system/chromeos/network/vpn_list_view.cc (right):

https://codereview.chromium.org/2463163002/diff/60001/ash/common/system/chrom...
ash/common/system/chromeos/network/vpn_list_view.cc:242: if (UseMd())
nit: if (UseMd()) {
  ...
}

https://codereview.chromium.org/2463163002/diff/60001/ash/common/system/chrom...
File ash/common/system/chromeos/screen_security/screen_tray_item.cc (right):

https://codereview.chromium.org/2463163002/diff/60001/ash/common/system/chrom...
ash/common/system/chromeos/screen_security/screen_tray_item.cc:78: auto layout =
auto* ?

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

https://codereview.chromium.org/2463163002/diff/60001/ash/common/system/tray/...
ash/common/system/tray/tray_popup_label_button.h:15: // TODO(estade): deprecated
for MD. Use CreateTrayPopupButton instead.
nit: can you please add "See crbug.com/614453" here to track the cleanup?

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

https://codereview.chromium.org/2463163002/diff/60001/ash/common/system/tray/...
ash/common/system/tray/tray_utils.h:26: // Creates a button for use in the
system menu that has no visible border when
nit: please add a comment that the caller assumes ownership of the returned
button (here and for CreateTrayPopupButton())

https://codereview.chromium.org/2463163002/diff/60001/ash/common/system/tray/...
ash/common/system/tray/tray_utils.h:27: // not being hovered/clicked.
nit: consider eliminating the double negative in the documentation such as "only
has a visible border when hovered/clicked"

https://codereview.chromium.org/2463163002/diff/60001/ash/common/system/tray/...
ash/common/system/tray/tray_utils.h:32: // Creates a button for use in the
system menu. For non-MD, this does the same
nit: please give a bit more description about what happens for MD (i.e.,
"creates a material design text button" and list any visual properties)

https://codereview.chromium.org/2463163002/diff/60001/ash/common/system/user/...
File ash/common/system/user/user_view.cc (right):

https://codereview.chromium.org/2463163002/diff/60001/ash/common/system/user/...
ash/common/system/user/user_view.cc:288: void UserView::Layout() {
With the way you've changed the logic here, I assume you're 100% certain that
|user_card_view_| will be non-null at the top of Layout()? If so, consider
adding a DCHECK to assert that.

[Evan Stade, 2016-11-01 23:54:09]

https://codereview.chromium.org/2463163002/diff/60001/ash/common/system/chrom...
File ash/common/system/chromeos/network/vpn_list_view.cc (right):

https://codereview.chromium.org/2463163002/diff/60001/ash/common/system/chrom...
ash/common/system/chromeos/network/vpn_list_view.cc:242: if (UseMd())
On 2016/11/01 22:38:50, tdanderson wrote:
> nit: if (UseMd()) {
>   ...
> }

Done.

https://codereview.chromium.org/2463163002/diff/60001/ash/common/system/chrom...
File ash/common/system/chromeos/screen_security/screen_tray_item.cc (right):

https://codereview.chromium.org/2463163002/diff/60001/ash/common/system/chrom...
ash/common/system/chromeos/screen_security/screen_tray_item.cc:78: auto layout =
On 2016/11/01 22:38:50, tdanderson wrote:
> auto* ?

why? isn't that the same thing in this case?

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

https://codereview.chromium.org/2463163002/diff/60001/ash/common/system/tray/...
ash/common/system/tray/tray_popup_label_button.h:15: // TODO(estade): deprecated
for MD. Use CreateTrayPopupButton instead.
On 2016/11/01 22:38:50, tdanderson wrote:
> nit: can you please add "See crbug.com/614453" here to track the cleanup?

Done.

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

https://codereview.chromium.org/2463163002/diff/60001/ash/common/system/tray/...
ash/common/system/tray/tray_utils.h:26: // Creates a button for use in the
system menu that has no visible border when
On 2016/11/01 22:38:50, tdanderson wrote:
> nit: please add a comment that the caller assumes ownership of the returned
> button (here and for CreateTrayPopupButton())

I have a hard time seeing how one would reasonably conclude otherwise... but
done.

https://codereview.chromium.org/2463163002/diff/60001/ash/common/system/tray/...
ash/common/system/tray/tray_utils.h:27: // not being hovered/clicked.
On 2016/11/01 22:38:50, tdanderson wrote:
> nit: consider eliminating the double negative in the documentation such as
"only
> has a visible border when hovered/clicked"

Done.

https://codereview.chromium.org/2463163002/diff/60001/ash/common/system/tray/...
ash/common/system/tray/tray_utils.h:32: // Creates a button for use in the
system menu. For non-MD, this does the same
On 2016/11/01 22:38:50, tdanderson wrote:
> nit: please give a bit more description about what happens for MD (i.e.,
> "creates a material design text button" and list any visual properties)

Done.

https://codereview.chromium.org/2463163002/diff/60001/ash/common/system/user/...
File ash/common/system/user/user_view.cc (right):

https://codereview.chromium.org/2463163002/diff/60001/ash/common/system/user/...
ash/common/system/user/user_view.cc:288: void UserView::Layout() {
On 2016/11/01 22:38:50, tdanderson wrote:
> With the way you've changed the logic here, I assume you're 100% certain that
> |user_card_view_| will be non-null at the top of Layout()? If so, consider
> adding a DCHECK to assert that.

AddUserCard is called in the constructor and it always creates user_card_view_.
Thus it should be non-null in pretty much every function, not just this one. The
code here unconditionally derefs user_card_view_ so in terms of documentation, I
think the DCHECK doesn't add much.

[Evan Stade, 2016-11-01 23:54:18]

The CQ bit was checked by estade@chromium.org

[Evan Stade, 2016-11-01 23:54:18]

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/2463163002/#ps80001
(title: "review feedback")

[commit-bot: I haz the power, 2016-11-01 23:55:05]

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, 2016-11-02 00:23:39]

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

[commit-bot: I haz the power, 2016-11-02 00:23:40]

Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[Evan Stade, 2016-11-02 01:01:29]

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

[commit-bot: I haz the power, 2016-11-02 01:02:26]

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, 2016-11-02 01:28:10]

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

[commit-bot: I haz the power, 2016-11-02 01:28:11]

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_...)

[Evan Stade, 2016-11-02 14:00:42]

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

[Evan Stade, 2016-11-02 14:00:43]

+derat for ash/test

[Evan Stade, 2016-11-02 14:01:09]

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

[commit-bot: I haz the power, 2016-11-02 14:01:53]

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

[Daniel Erat, 2016-11-02 14:10:24]

lgtm for ash/test

[commit-bot: I haz the power, 2016-11-02 14:47:34]

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

[commit-bot: I haz the power, 2016-11-02 14:47:35]

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_...)

[Evan Stade, 2016-11-02 17:59:37]

The CQ bit was checked by estade@chromium.org

[Evan Stade, 2016-11-02 17:59:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
tdanderson@chromium.org, derat@chromium.org
Link to the patchset: https://codereview.chromium.org/2463163002/#ps140001
(title: "boxlayout has some annoying dchecks")

[commit-bot: I haz the power, 2016-11-02 18:00:13]

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, 2016-11-02 19:28:02]

Description was changed from

==========
Update chromeos system menu buttons for MD.

Testing:
- for the VPN disconnect button, --shill-stub="vpn=1"
- for the screen tray item, use chrome remote desktop app to share your
  screen
- for the (chrome)cast item, apply this patch:
http://hastebin.com/tuxutojiqo.php

BUG=647017
==========

to

==========
Update chromeos system menu buttons for MD.

Testing:
- for the VPN disconnect button, --shill-stub="vpn=1"
- for the screen tray item, use chrome remote desktop app to share your
  screen
- for the (chrome)cast item, apply this patch:
http://hastebin.com/tuxutojiqo.php

BUG=647017
==========

[commit-bot: I haz the power, 2016-11-02 19:28:03]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-11-02 19:55:14]

Description was changed from

==========
Update chromeos system menu buttons for MD.

Testing:
- for the VPN disconnect button, --shill-stub="vpn=1"
- for the screen tray item, use chrome remote desktop app to share your
  screen
- for the (chrome)cast item, apply this patch:
http://hastebin.com/tuxutojiqo.php

BUG=647017
==========

to

==========
Update chromeos system menu buttons for MD.

Testing:
- for the VPN disconnect button, --shill-stub="vpn=1"
- for the screen tray item, use chrome remote desktop app to share your
  screen
- for the (chrome)cast item, apply this patch:
http://hastebin.com/tuxutojiqo.php

BUG=647017

Committed: https://crrev.com/eafebe2c67be335ab6cf75d8794e46e67ae8879c
Cr-Commit-Position: refs/heads/master@{#429363}
==========

[commit-bot: I haz the power, 2016-11-02 19:55:18]

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