Some more fixes to cros system tray icons (esp. screen tray items)

Some more fixes to cros system tray icons (esp. screen tray items)

More things should use TrayItemView (and not override its layout manager)

BUG=653292
Committed: https://crrev.com/4b338bc886f384d287b2e11fc913016301ed6761
Cr-Commit-Position: refs/heads/master@{#429163}

[Evan Stade, 2016-11-01 19:04:10]

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

[Evan Stade, 2016-11-01 19:04:10]

I love being right and I love it when fixing bugs boils down to finding the
right lines of code to delete 8)

[Evan Stade, 2016-11-01 19:04:21]

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

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

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-01 19:33:30]

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

[commit-bot: I haz the power, 2016-11-01 19:33:31]

Dry run: This issue passed the CQ dry run.

[tdanderson, 2016-11-01 20:25:08]

LGTM

https://codereview.chromium.org/2465403002/diff/1/ash/common/system/cast/tray...
File ash/common/system/cast/tray_cast.h (right):

https://codereview.chromium.org/2465403002/diff/1/ash/common/system/cast/tray...
ash/common/system/cast/tray_cast.h:3: // found in the LICENSE file.
Can you please include a brief CL-level description for this issue? It will be
beneficial for someone who is searching through git logs in case of a
regression.

https://codereview.chromium.org/2465403002/diff/1/ash/common/system/chromeos/...
File ash/common/system/chromeos/screen_security/screen_tray_item.cc (right):

https://codereview.chromium.org/2465403002/diff/1/ash/common/system/chromeos/...
ash/common/system/chromeos/screen_security/screen_tray_item.cc:186: views::View*
ScreenTrayItem::CreateTrayView(LoginStatus status) {
Nice catch.

https://codereview.chromium.org/2465403002/diff/1/ash/common/system/tray/tray...
File ash/common/system/tray/tray_item_view.cc (right):

https://codereview.chromium.org/2465403002/diff/1/ash/common/system/tray/tray...
ash/common/system/tray/tray_item_view.cc:86: DCHECK_EQ(1, child_count());
Can you please add documentation somewhere in tray_item_view.h clarifying that
exactly one of |label_| and |image_view_| is non-null?

[Evan Stade, 2016-11-01 23:43:20]

Description was changed from

==========
Some more fixes to cros system tray icons (esp. screen tray items)

BUG=653292
==========

to

==========
Some more fixes to cros system tray icons (esp. screen tray items)

More things should use TrayItemView (and not override its layout manager)

BUG=653292
==========

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

https://codereview.chromium.org/2465403002/diff/1/ash/common/system/cast/tray...
File ash/common/system/cast/tray_cast.h (right):

https://codereview.chromium.org/2465403002/diff/1/ash/common/system/cast/tray...
ash/common/system/cast/tray_cast.h:3: // found in the LICENSE file.
On 2016/11/01 20:25:08, tdanderson wrote:
> Can you please include a brief CL-level description for this issue? It will be
> beneficial for someone who is searching through git logs in case of a
> regression.

done (I think)

https://codereview.chromium.org/2465403002/diff/1/ash/common/system/tray/tray...
File ash/common/system/tray/tray_item_view.cc (right):

https://codereview.chromium.org/2465403002/diff/1/ash/common/system/tray/tray...
ash/common/system/tray/tray_item_view.cc:86: DCHECK_EQ(1, child_count());
On 2016/11/01 20:25:08, tdanderson wrote:
> Can you please add documentation somewhere in tray_item_view.h clarifying that
> exactly one of |label_| and |image_view_| is non-null?

Done.

[Evan Stade, 2016-11-01 23:45:23]

The CQ bit was checked by estade@chromium.org

[Evan Stade, 2016-11-01 23:45:23]

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/2465403002/#ps20001
(title: "commentary")

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

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:37:31]

Description was changed from

==========
Some more fixes to cros system tray icons (esp. screen tray items)

More things should use TrayItemView (and not override its layout manager)

BUG=653292
==========

to

==========
Some more fixes to cros system tray icons (esp. screen tray items)

More things should use TrayItemView (and not override its layout manager)

BUG=653292
==========

[commit-bot: I haz the power, 2016-11-02 00:37:32]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-11-02 00:42:51]

Description was changed from

==========
Some more fixes to cros system tray icons (esp. screen tray items)

More things should use TrayItemView (and not override its layout manager)

BUG=653292
==========

to

==========
Some more fixes to cros system tray icons (esp. screen tray items)

More things should use TrayItemView (and not override its layout manager)

BUG=653292

Committed: https://crrev.com/4b338bc886f384d287b2e11fc913016301ed6761
Cr-Commit-Position: refs/heads/master@{#429163}
==========

[commit-bot: I haz the power, 2016-11-02 00:42:52]

Patchset 2 (id:??) landed as
https://crrev.com/4b338bc886f384d287b2e11fc913016301ed6761
Cr-Commit-Position: refs/heads/master@{#429163}