chromeos: Refactor ash SystemTrayDelegate enterprise methods to mojo

chromeos: Refactor ash SystemTrayDelegate enterprise methods to mojo

This is needed to support mustash.

* Migrate code from SystemTrayDelegateChromeOS to SystemTrayClient
* Migrate strings into ash_strings.grd

BUG=715335
TEST=added to ash_unittests and chrome browser_tests

Review-Url: https://codereview.chromium.org/2839043004
Cr-Commit-Position: refs/heads/master@{#467495}
Committed: https://chromium.googlesource.com/chromium/src/+/1b67ae3a76893ee92586a28fd4b11640e28e12de

[James Cook, 2017-04-26 18:47:12]

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

[commit-bot: I haz the power, 2017-04-26 18:47:50]

Dry run: CQ is trying da patch.

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

[James Cook, 2017-04-26 18:49:36]

jamescook@chromium.org changed reviewers:
+ tsepez@chromium.org, xiyuan@chromium.org

[James Cook, 2017-04-26 18:49:36]

xiyuan, please take a look at everything.

tsepez, please take a look at ash mojom.

https://codereview.chromium.org/2839043004/diff/1/ash/system/tray/system_tray...
File ash/system/tray/system_tray_delegate.h (left):

https://codereview.chromium.org/2839043004/diff/1/ash/system/tray/system_tray...
ash/system/tray/system_tray_delegate.h:59: virtual std::string
GetEnterpriseRealm() const;
This wasn't used.

https://codereview.chromium.org/2839043004/diff/1/chrome/browser/ui/ash/syste...
File chrome/browser/ui/ash/system_tray_client.cc (right):

https://codereview.chromium.org/2839043004/diff/1/chrome/browser/ui/ash/syste...
chrome/browser/ui/ash/system_tray_client.cc:329: if
(session_manager::SessionManager::Get()->IsUserSessionBlocked()) {
This is slightly different, but I think closer to what we want.

[commit-bot: I haz the power, 2017-04-26 18:57:00]

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

[commit-bot: I haz the power, 2017-04-26 18:57:01]

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

[Tom Sepez, 2017-04-26 19:29:53]

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

https://codereview.chromium.org/2839043004/diff/1/ash/public/interfaces/syste...
ash/public/interfaces/system_tray.mojom:29: SetEnterpriseDomain(string
enterprise_domain, bool active_directory_managed);
This just updates the display, right?  No change of status actually occurs?

[xiyuan, 2017-04-26 19:55:13]

Just nits

https://codereview.chromium.org/2839043004/diff/1/ash/BUILD.gn
File ash/BUILD.gn (right):

https://codereview.chromium.org/2839043004/diff/1/ash/BUILD.gn#newcode1185
ash/BUILD.gn:1185: "system/enterprise/tray_enterprise_unittest.cc",
Seems the file is missed from the CL.

https://codereview.chromium.org/2839043004/diff/1/ash/system/enterprise/tray_...
File ash/system/enterprise/tray_enterprise.h (right):

https://codereview.chromium.org/2839043004/diff/1/ash/system/enterprise/tray_...
ash/system/enterprise/tray_enterprise.h:26: LabelTrayView* tray_view() { return
tray_view_; }
nit: Prevailing style is to put inline accessors after all other methods.

https://codereview.chromium.org/2839043004/diff/1/chrome/browser/ui/ash/syste...
File chrome/browser/ui/ash/system_tray_client.cc (right):

https://codereview.chromium.org/2839043004/diff/1/chrome/browser/ui/ash/syste...
chrome/browser/ui/ash/system_tray_client.cc:329: if
(session_manager::SessionManager::Get()->IsUserSessionBlocked()) {
On 2017/04/26 18:49:36, James Cook wrote:
> This is slightly different, but I think closer to what we want.

Acknowledged.

[James Cook, 2017-04-26 21:03:58]

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

[James Cook, 2017-04-26 21:04:15]

xiyuan / tsepez, please take another look.

https://codereview.chromium.org/2839043004/diff/1/ash/BUILD.gn
File ash/BUILD.gn (right):

https://codereview.chromium.org/2839043004/diff/1/ash/BUILD.gn#newcode1185
ash/BUILD.gn:1185: "system/enterprise/tray_enterprise_unittest.cc",
On 2017/04/26 19:55:12, xiyuan wrote:
> Seems the file is missed from the CL.

Whoops. Silly me. Done.

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

https://codereview.chromium.org/2839043004/diff/1/ash/public/interfaces/syste...
ash/public/interfaces/system_tray.mojom:29: SetEnterpriseDomain(string
enterprise_domain, bool active_directory_managed);
On 2017/04/26 19:29:53, Tom Sepez wrote:
> This just updates the display, right?  No change of status actually occurs?

Correct. I updated the comment to try to clarify.

https://codereview.chromium.org/2839043004/diff/1/ash/system/enterprise/tray_...
File ash/system/enterprise/tray_enterprise.h (right):

https://codereview.chromium.org/2839043004/diff/1/ash/system/enterprise/tray_...
ash/system/enterprise/tray_enterprise.h:26: LabelTrayView* tray_view() { return
tray_view_; }
On 2017/04/26 19:55:12, xiyuan wrote:
> nit: Prevailing style is to put inline accessors after all other methods.

Done.

[commit-bot: I haz the power, 2017-04-26 21:04:38]

Dry run: CQ is trying da patch.

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

[xiyuan, 2017-04-26 21:12:09]

lgtm

[Tom Sepez, 2017-04-26 21:13:27]

lgtm

[James Cook, 2017-04-26 21:21:43]

The CQ bit was unchecked by jamescook@chromium.org

[James Cook, 2017-04-26 21:21:48]

The CQ bit was checked by jamescook@chromium.org

[commit-bot: I haz the power, 2017-04-26 21:22:49]

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-26 22:56:17]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1493241708241640,
"parent_rev": "a4fc2b93398945ed1953483b6df3ccf433a216bf", "commit_rev":
"1b67ae3a76893ee92586a28fd4b11640e28e12de"}

[commit-bot: I haz the power, 2017-04-26 22:57:19]

Description was changed from

==========
chromeos: Refactor ash SystemTrayDelegate enterprise methods to mojo

This is needed to support mustash.

* Migrate code from SystemTrayDelegateChromeOS to SystemTrayClient
* Migrate strings into ash_strings.grd

BUG=715335
TEST=added to ash_unittests and chrome browser_tests
==========

to

==========
chromeos: Refactor ash SystemTrayDelegate enterprise methods to mojo

This is needed to support mustash.

* Migrate code from SystemTrayDelegateChromeOS to SystemTrayClient
* Migrate strings into ash_strings.grd

BUG=715335
TEST=added to ash_unittests and chrome browser_tests

Review-Url: https://codereview.chromium.org/2839043004
Cr-Commit-Position: refs/heads/master@{#467495}
Committed:
https://chromium.googlesource.com/chromium/src/+/1b67ae3a76893ee92586a28fd4b1...
==========

[commit-bot: I haz the power, 2017-04-26 22:57:20]

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