Connect Login UI to SystemTray

Connect Login UI to SystemTray

WebuiLoginView toggles the visibility and enabled state of the SystemTray.
However when running in mash it cannot directly access the tray.

This change updates the system_tray.mojom to add the needed calls, and updates
WebuiLoginView to use the mojom for both normal and mash modes.

BUG=648758
TEST=ran associated browser tests, manual testing of login in normal and mash

Review-Url: https://codereview.chromium.org/2625473002
Cr-Commit-Position: refs/heads/master@{#442430}
Committed: https://chromium.googlesource.com/chromium/src/+/328a830cd41918893878cad3a5e33299b1727150

[jonross, 2017-01-09 18:37:01]

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

[jonross, 2017-01-09 18:37:01]

Hey James,

Could you review this change? I'm adding a few items to the SystemTray mojom, as
login also needs to modify it.

Thanks,
Jon

[James Cook, 2017-01-09 19:30:05]

LGTM with nits

https://codereview.chromium.org/2625473002/diff/1/ash/common/system/tray/syst...
File ash/common/system/tray/system_tray_controller.cc (right):

https://codereview.chromium.org/2625473002/diff/1/ash/common/system/tray/syst...
ash/common/system/tray/system_tray_controller.cc:131: ash::SystemTray* tray =
nit: no ash::, here and below

https://codereview.chromium.org/2625473002/diff/1/ash/common/system/tray/syst...
ash/common/system/tray/system_tray_controller.cc:136: // We disable the UI to
prevent user from interracting with UI elements,
nit: not your fault, but "interacting" here and "particularly" below

https://codereview.chromium.org/2625473002/diff/1/ash/common/system/tray/syst...
ash/common/system/tray/system_tray_controller.cc:156:
tray->GetWidget()->SetOpacity(visible ? 1.0 : 0.0);
nit: 1.f and 0.f since the call takes floats

https://codereview.chromium.org/2625473002/diff/1/ash/public/interfaces/syste...
File ash/public/interfaces/system_tray.mojom (right):

https://codereview.chromium.org/2625473002/diff/1/ash/public/interfaces/syste...
ash/public/interfaces/system_tray.mojom:14: // Sets the enabled state on the
primary tray. If not |enabled| any open
nit: on -> of. Also, I might say "tray on the primary display" here and below.

[jonross, 2017-01-09 22:01:35]

https://codereview.chromium.org/2625473002/diff/1/ash/common/system/tray/syst...
File ash/common/system/tray/system_tray_controller.cc (right):

https://codereview.chromium.org/2625473002/diff/1/ash/common/system/tray/syst...
ash/common/system/tray/system_tray_controller.cc:131: ash::SystemTray* tray =
On 2017/01/09 19:30:05, James Cook wrote:
> nit: no ash::, here and below

Sadly they are, compiler incorrectly tries to link vs mojom::SystemTray, so the
specificity is needed here.

https://codereview.chromium.org/2625473002/diff/1/ash/common/system/tray/syst...
ash/common/system/tray/system_tray_controller.cc:136: // We disable the UI to
prevent user from interracting with UI elements,
On 2017/01/09 19:30:05, James Cook wrote:
> nit: not your fault, but "interacting" here and "particularly" below

I'm used to making these comment typos myself!

WTB spell check for comments....

https://codereview.chromium.org/2625473002/diff/1/ash/common/system/tray/syst...
ash/common/system/tray/system_tray_controller.cc:156:
tray->GetWidget()->SetOpacity(visible ? 1.0 : 0.0);
On 2017/01/09 19:30:05, James Cook wrote:
> nit: 1.f and 0.f since the call takes floats

Done.

https://codereview.chromium.org/2625473002/diff/1/ash/public/interfaces/syste...
File ash/public/interfaces/system_tray.mojom (right):

https://codereview.chromium.org/2625473002/diff/1/ash/public/interfaces/syste...
ash/public/interfaces/system_tray.mojom:14: // Sets the enabled state on the
primary tray. If not |enabled| any open
On 2017/01/09 19:30:05, James Cook wrote:
> nit: on -> of. Also, I might say "tray on the primary display" here and below.

Done.

[jonross, 2017-01-09 22:01:53]

jonross@chromium.org changed reviewers:
+ xiyuan@chromium.org

[jonross, 2017-01-09 22:01:53]

xiyuan@chromium.org: Please review changes in 

webui_login_view.cc

Thanks,
Jon

[xiyuan, 2017-01-09 22:08:50]

webui_login_view.cc lgtm

[jonross, 2017-01-09 22:14:05]

jonross@chromium.org changed reviewers:
+ tsepez@chromium.org

[jonross, 2017-01-09 22:14:05]

Hello tsepez@,

Could you provide a security owners review for the changes to:
ash/public/interfaces/system_tray.mojom

Thanks,
Jonathan

[Tom Sepez, 2017-01-09 23:27:46]

Mojom LGTM

[jonross, 2017-01-09 23:30:16]

The CQ bit was checked by jonross@chromium.org

[jonross, 2017-01-09 23:30:16]

The patchset sent to the CQ was uploaded after l-g-t-m from
jamescook@chromium.org
Link to the patchset: https://codereview.chromium.org/2625473002/#ps40001
(title: "nits")

[commit-bot: I haz the power, 2017-01-09 23:31:22]

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-01-10 00:57:16]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1484004616001670,
"parent_rev": "c281383a4a0459271ea2a258c4e1a5262fa30f78", "commit_rev":
"328a830cd41918893878cad3a5e33299b1727150"}

[commit-bot: I haz the power, 2017-01-10 00:57:45]

Description was changed from

==========
Connect Login UI to SystemTray

WebuiLoginView toggles the visibility and enabled state of the SystemTray.
However when running in mash it cannot directly access the tray.

This change updates the system_tray.mojom to add the needed calls, and updates
WebuiLoginView to use the mojom for both normal and mash modes.

BUG=648758
TEST=ran associated browser tests, manual testing of login in normal and mash
==========

to

==========
Connect Login UI to SystemTray

WebuiLoginView toggles the visibility and enabled state of the SystemTray.
However when running in mash it cannot directly access the tray.

This change updates the system_tray.mojom to add the needed calls, and updates
WebuiLoginView to use the mojom for both normal and mash modes.

BUG=648758
TEST=ran associated browser tests, manual testing of login in normal and mash

Review-Url: https://codereview.chromium.org/2625473002
Cr-Commit-Position: refs/heads/master@{#442430}
Committed:
https://chromium.googlesource.com/chromium/src/+/328a830cd41918893878cad3a5e3...
==========

[commit-bot: I haz the power, 2017-01-10 00:57:46]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/328a830cd41918893878cad3a5e3...