Remove the extra separator appears beside overview tray

Remove the extra separator appears beside overview tray

In the cl https://codereview.chromium.org/2618383003/, 
"set_separator_visibility to false" is called for all trays 
that the separator is missing on the right side. I forget to 
set it for the overview_tray.
 
TEST=MANUAL

BUG=692374
Review-Url: https://codereview.chromium.org/2702583002
Cr-Commit-Position: refs/heads/master@{#451514}
Committed: https://chromium.googlesource.com/chromium/src/+/c8cc142e41b1c23af56fb3f3649441bed8c25bb5

[yiyix, 2017-02-16 19:52:31]

Description was changed from

==========
fix visibility

BUG=692374
==========

to

==========
Remove the extra separator appears beside overview tray

In the cl https://codereview.chromium.org/2618383003/, 
"set_separator_visibility to false" is called for all trays 
that the separator is missing on the right side. I forget to 
set it for the overview_tray.
 
TEST=MANUAL

BUG=692374
==========

[yiyix, 2017-02-16 19:52:32]

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

[yiyix, 2017-02-16 19:52:59]

@tdanderson, could you please review this change? Thank you.

[tdanderson, 2017-02-16 20:25:03]

LGTM with one comment addressed.

https://codereview.chromium.org/2702583002/diff/1/ash/common/system/overview/...
File ash/common/system/overview/overview_button_tray.cc (right):

https://codereview.chromium.org/2702583002/diff/1/ash/common/system/overview/...
ash/common/system/overview/overview_button_tray.cc:60: // separator is required
on its right side.
How about something like "Since OverviewButtonTray is located on the rightmost
position of a horizontal shelf, no separator is required." to make it more
general? Note that using the class name 'OverviewButtonTray' is preferred over
'|overview_button_tray|' since the |...| notation should only be used for
identifier names or |this|.

Also, in the SystemTray constructor above the set_separator_visibility() call
you say "Since |system_tray| locates on the right most position..." but this
isn't true if overview mode is enabled. The reason we don't need a separator for
the system tray is because the user avatar is on the right hand side and that is
sufficient to indicate separation. Can you also please change that documentation
accordingly?

[yiyix, 2017-02-16 20:43:43]

https://codereview.chromium.org/2702583002/diff/1/ash/common/system/overview/...
File ash/common/system/overview/overview_button_tray.cc (right):

https://codereview.chromium.org/2702583002/diff/1/ash/common/system/overview/...
ash/common/system/overview/overview_button_tray.cc:60: // separator is required
on its right side.
On 2017/02/16 20:25:03, tdanderson wrote:
> How about something like "Since OverviewButtonTray is located on the rightmost
> position of a horizontal shelf, no separator is required." to make it more
> general? Note that using the class name 'OverviewButtonTray' is preferred over
> '|overview_button_tray|' since the |...| notation should only be used for
> identifier names or |this|.
> 
> Also, in the SystemTray constructor above the set_separator_visibility() call
> you say "Since |system_tray| locates on the right most position..." but this
> isn't true if overview mode is enabled. The reason we don't need a separator
for
> the system tray is because the user avatar is on the right hand side and that
is
> sufficient to indicate separation. Can you also please change that
documentation
> accordingly?

I have update all comments. Could you check it again? Thank you

[tdanderson, 2017-02-16 22:05:19]

Thanks, patch set 2 LGTM

[yiyix, 2017-02-16 22:19:22]

yiyix@chromium.org changed reviewers:
+ jamescook@chromium.org

[yiyix, 2017-02-16 22:19:40]

@jamescook, Could you please take a look? Thank you.

[James Cook, 2017-02-17 01:41:10]

LGTM

FYI tdanderson is an owner for ash/common/system (via ash/system). If presubmits
are complaining let me know and I'll fix the owners files.

[yiyix, 2017-02-17 01:44:25]

The CQ bit was checked by yiyix@chromium.org

[commit-bot: I haz the power, 2017-02-17 01:45:46]

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-02-17 03:48:04]

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

[commit-bot: I haz the power, 2017-02-17 03:48:05]

Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)

[yiyix, 2017-02-19 03:53:58]

The CQ bit was checked by yiyix@chromium.org

[commit-bot: I haz the power, 2017-02-19 03:54:11]

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-02-19 05:00:05]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1487476438937600,
"parent_rev": "64975bc58e6b041ade1cfe2edb431896548845f3", "commit_rev":
"c8cc142e41b1c23af56fb3f3649441bed8c25bb5"}

[commit-bot: I haz the power, 2017-02-19 05:00:38]

Description was changed from

==========
Remove the extra separator appears beside overview tray

In the cl https://codereview.chromium.org/2618383003/, 
"set_separator_visibility to false" is called for all trays 
that the separator is missing on the right side. I forget to 
set it for the overview_tray.
 
TEST=MANUAL

BUG=692374
==========

to

==========
Remove the extra separator appears beside overview tray

In the cl https://codereview.chromium.org/2618383003/, 
"set_separator_visibility to false" is called for all trays 
that the separator is missing on the right side. I forget to 
set it for the overview_tray.
 
TEST=MANUAL

BUG=692374

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

[commit-bot: I haz the power, 2017-02-19 05:00:39]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/c8cc142e41b1c23af56fb3f36494...