cros: Show dircryto migration banner

cros: Show dircryto migration banner

- Add CryptohomeClient::NeedsDircryptoMigration wrapper;
- Check whether a user needs dircryto migration when the relevant
  user pod get focus;
- Show a banner message when the user needs dircryto migration;

BUG=708015
Review-Url: https://codereview.chromium.org/2808423002
Cr-Commit-Position: refs/heads/master@{#464332}
Committed: https://chromium.googlesource.com/chromium/src/+/e4b9a821a3e13d5bc49431173664dfd24ae3fb59

[xiyuan, 2017-04-11 16:11:44]

xiyuan@chromium.org changed reviewers:
+ achuith@chromium.org, hashimoto@chromium.org

[xiyuan, 2017-04-11 16:11:44]

Suggested split:

hashimoto@: CryptohomeClient changes under
  chromeos/dbus/*

achuith@: Login screen changes under
  chrome/browser/chromeos/login/screens/*
  ui/login/account_picker/*

I will run trybots when the system_api Cl lands and deps rolled.

[achuithb, 2017-04-11 21:46:52]

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

[commit-bot: I haz the power, 2017-04-11 21:47:46]

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

[achuithb, 2017-04-11 22:04:27]

lgtm. 

Since this is UI, do you think a browser test is worthwhile? I guess all the
interesting parts get mocked out.

https://codereview.chromium.org/2808423002/diff/20001/chrome/app/chromeos_str...
File chrome/app/chromeos_strings.grdp (right):

https://codereview.chromium.org/2808423002/diff/20001/chrome/app/chromeos_str...
chrome/app/chromeos_strings.grdp:2137: <message
name="IDS_LOGIN_NEEDS_DIRCRYPTO_MIGRATION_BANNER" desc="Banner text for users
that needs to migrate their data to use dircrypto.">
s/needs/need

https://codereview.chromium.org/2808423002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/screens/user_selection_screen.cc (right):

https://codereview.chromium.org/2808423002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/login/screens/user_selection_screen.cc:158: // Helper
class to call cryptohome to whether a user needs dircrypto migration.
to check whether

https://codereview.chromium.org/2808423002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/login/screens/user_selection_screen.cc:220: if
(needs_migration) {
ternary operator is worth considering here, but it's a matter of taste.

https://codereview.chromium.org/2808423002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/login/screens/user_selection_screen.cc:224:
owner_->ShowBannerMessage(base::string16());
So the only banner message about dircrypto migration?

[achuithb, 2017-04-11 22:04:54]

> I will run trybots when the system_api Cl lands and deps rolled.

Sorry, didn't see this.

[commit-bot: I haz the power, 2017-04-11 22:09:19]

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

[commit-bot: I haz the power, 2017-04-11 22:09:20]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[xiyuan, 2017-04-11 23:07:42]

All done and added a test.

https://codereview.chromium.org/2808423002/diff/20001/chrome/app/chromeos_str...
File chrome/app/chromeos_strings.grdp (right):

https://codereview.chromium.org/2808423002/diff/20001/chrome/app/chromeos_str...
chrome/app/chromeos_strings.grdp:2137: <message
name="IDS_LOGIN_NEEDS_DIRCRYPTO_MIGRATION_BANNER" desc="Banner text for users
that needs to migrate their data to use dircrypto.">
On 2017/04/11 22:04:26, achuithb wrote:
> s/needs/need

Done.

https://codereview.chromium.org/2808423002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/screens/user_selection_screen.cc (right):

https://codereview.chromium.org/2808423002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/login/screens/user_selection_screen.cc:158: // Helper
class to call cryptohome to whether a user needs dircrypto migration.
On 2017/04/11 22:04:26, achuithb wrote:
> to check whether

Done.

https://codereview.chromium.org/2808423002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/login/screens/user_selection_screen.cc:220: if
(needs_migration) {
On 2017/04/11 22:04:27, achuithb wrote:
> ternary operator is worth considering here, but it's a matter of taste.

Done.

https://codereview.chromium.org/2808423002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/login/screens/user_selection_screen.cc:224:
owner_->ShowBannerMessage(base::string16());
On 2017/04/11 22:04:26, achuithb wrote:
> So the only banner message about dircrypto migration?

IIUC, it is only used for user adding screen now (smart lock used it before but
seems no longer using it now) and we don't do migration chck on user adding
screen.

[achuithb, 2017-04-11 23:40:56]

lgtm!

[xiyuan, 2017-04-12 17:55:35]

hashimoto@, this is the chrome side change to use the added cryptohome api.

You can review only files under
  chromeos/dbus/*
.

[hashimoto, 2017-04-13 00:52:59]

Sorry for late response.
chromeos/dbus lgtm

[xiyuan, 2017-04-13 04:52:25]

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

[commit-bot: I haz the power, 2017-04-13 04:52:42]

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, 2017-04-13 06:35:18]

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

[commit-bot: I haz the power, 2017-04-13 06:35:19]

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

[xiyuan, 2017-04-13 06:56:41]

The CQ bit was checked by xiyuan@chromium.org

[commit-bot: I haz the power, 2017-04-13 06:57:04]

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-13 07:35:38]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1492066601659230,
"parent_rev": "4030a82031a1e9121b5dd6e724754ea6a8b0d80a", "commit_rev":
"e4b9a821a3e13d5bc49431173664dfd24ae3fb59"}

[commit-bot: I haz the power, 2017-04-13 07:36:28]

Description was changed from

==========
cros: Show dircryto migration banner

- Add CryptohomeClient::NeedsDircryptoMigration wrapper;
- Check whether a user needs dircryto migration when the relevant
  user pod get focus;
- Show a banner message when the user needs dircryto migration;

BUG=708015
==========

to

==========
cros: Show dircryto migration banner

- Add CryptohomeClient::NeedsDircryptoMigration wrapper;
- Check whether a user needs dircryto migration when the relevant
  user pod get focus;
- Show a banner message when the user needs dircryto migration;

BUG=708015

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

[commit-bot: I haz the power, 2017-04-13 07:36:30]

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