This CL replaces std::string user_id in ash/* with AccountId.

This CL replaces std::string user_id in ash/* with AccountId.

This CL replaces most of ASH temporary AccountId to/from email transformations
with AccountId references. It also replaces strings in multi_user_util
with AccountId objects.

This is part of transition to AccountId.

BUG=468875
TEST=manual

Committed: https://crrev.com/cdf46dd1677ae5a8974fb4b1677b39d89f973a36
Cr-Commit-Position: refs/heads/master@{#359225}

[Alexander Alekseev, 2015-11-05 09:48:52]

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

[commit-bot: I haz the power, 2015-11-05 09:49:21]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1428213004/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1428213004/1

[commit-bot: I haz the power, 2015-11-05 09:52:01]

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

[commit-bot: I haz the power, 2015-11-05 09:52:01]

Dry run: Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Alexander Alekseev, 2015-11-05 10:26:01]

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

[commit-bot: I haz the power, 2015-11-05 10:26:19]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1428213004/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1428213004/20001

[Alexander Alekseev, 2015-11-05 10:35:20]

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

[commit-bot: I haz the power, 2015-11-05 10:36:10]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1428213004/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1428213004/40001

[commit-bot: I haz the power, 2015-11-05 10:51:09]

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

[commit-bot: I haz the power, 2015-11-05 10:51:10]

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

[Alexander Alekseev, 2015-11-05 11:10:22]

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

[commit-bot: I haz the power, 2015-11-05 11:11:31]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1428213004/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1428213004/60001

[commit-bot: I haz the power, 2015-11-05 11:43:28]

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

[commit-bot: I haz the power, 2015-11-05 11:43:28]

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

[Alexander Alekseev, 2015-11-06 07:37:26]

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

[commit-bot: I haz the power, 2015-11-06 07:38:06]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1428213004/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1428213004/60001

[commit-bot: I haz the power, 2015-11-06 08:12:29]

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

[commit-bot: I haz the power, 2015-11-06 08:12:30]

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

[Alexander Alekseev, 2015-11-06 08:43:34]

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

[commit-bot: I haz the power, 2015-11-06 08:44:01]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1428213004/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1428213004/80001

[commit-bot: I haz the power, 2015-11-06 09:28:09]

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

[commit-bot: I haz the power, 2015-11-06 09:28:10]

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

[Alexander Alekseev, 2015-11-06 11:28:16]

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

[commit-bot: I haz the power, 2015-11-06 11:29:06]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1428213004/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1428213004/100001

[Alexander Alekseev, 2015-11-06 11:56:06]

alemate@chromium.org changed reviewers:
+ sky@chromium.org, stevenjb@chromium.org

[Alexander Alekseev, 2015-11-06 11:56:07]

This CL migrates code in ash/* and c/b/ui/ash/* to AccountId.

There is only one temporary AccountId object in ash/* , which depends on
user_manager::UserInfo API, which still needs to be updated.

Dear reviewers.

Both of you are owners of most of the files in this CL, so lets's divide the
review into two parts by the number of changed lines. So please review:

stevenjb@:
   c/b/ui/ash/multi_user/*

sky@: everything else, namely:
  ash/session/session_state_delegate.h
  ash/session/session_state_observer.h
  ash/shell/shell_delegate_impl.cc
  ash/system/tray/system_tray_delegate.cc
  ash/system/tray/system_tray_delegate.h
  ash/system/user/tray_user.cc
  ash/system/user/user_view.cc
  ash/test/test_session_state_delegate.cc
  ash/test/test_session_state_delegate.h
  c/b/c/accessibility/accessibility_manager.cc
  c/b/c/accessibility/accessibility_manager.h
  c/b/c/accessibility/magnification_manager.cc
  c/b/c/extensions/file_manager/private_api_misc.cc
  c/b/c/extensions/wallpaper_private_api_unittest.cc
  c/b/c/file_system_provider/notification_manager.cc
  c/b/feedback/show_feedback_page.cc
  c/b/notifications/message_center_notifications_unittest.cc
  c/b/notifications/profile_notification.cc
  c/b/signin/signin_error_notifier_ash.cc
  c/b/sync/sync_error_notifier_ash.cc
  c/b/ui/ash/chrome_shell_content_state_chromeos.cc
  c/b/ui/ash/launcher/chrome_launcher_controller.cc
  c/b/ui/ash/launcher/chrome_launcher_controller_unittest.cc
  c/b/ui/ash/launcher/multi_profile_app_window_launcher_controller.cc
  c/b/ui/ash/session_state_delegate_chromeos.cc
  c/b/ui/ash/session_state_delegate_chromeos.h
  c/b/ui/ash/session_state_delegate_views.cc
  c/b/ui/ash/session_state_delegate_views.h
  c/b/ui/ash/system_tray_delegate_chromeos.cc
  c/b/ui/ash/system_tray_delegate_chromeos.h
  c/b/ui/browser_finder.cc
  c/b/ui/browser_finder_chromeos_unittest.cc
  c/b/ui/browser_navigator.cc
  c/b/ui/browser_navigator_browsertest_chromeos.cc
  c/b/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc
  c/b/ui/views/frame/system_menu_model_builder.cc

[commit-bot: I haz the power, 2015-11-06 12:19:38]

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

[commit-bot: I haz the power, 2015-11-06 12:19:39]

Dry run: This issue passed the CQ dry run.

[stevenjb, 2015-11-06 17:06:56]

On 2015/11/06 12:19:39, commit-bot: I haz the power wrote:
> Dry run: This issue passed the CQ dry run.

I'm not actually a good reviewer for ash/multi_user/, you should see if skhune@
or mukai@ are available to review.

I will look at ash/system/ and c/b/ui/ash/ both of which I am familiar with.

[stevenjb, 2015-11-06 17:45:29]

I looked at all of ash/ and chrome/browser/ui/ash/, but I'm pretty fuzzy on the
multi_user/ stuff.

https://codereview.chromium.org/1428213004/diff/100001/chrome/browser/ui/ash/...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc (right):

https://codereview.chromium.org/1428213004/diff/100001/chrome/browser/ui/ash/...
chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc:1413:
.GetUserEmail()] = list;
Can we break this up to call out that we are extracting user_id here and using
that as a key? i.e.
std::string user_id =
  multi_user_util::GetAccountIdFromProfile(profile_).GetUserEmail();

https://codereview.chromium.org/1428213004/diff/100001/chrome/browser/ui/ash/...
File chrome/browser/ui/ash/multi_user/multi_user_context_menu_chromeos.cc
(right):

https://codereview.chromium.org/1428213004/diff/100001/chrome/browser/ui/ash/...
chrome/browser/ui/ash/multi_user/multi_user_context_menu_chromeos.cc:79: const
AccountId account_id = manager->GetWindowOwner(window);
const AccountId& ?

https://codereview.chromium.org/1428213004/diff/100001/chrome/browser/ui/ash/...
File chrome/browser/ui/ash/multi_user/multi_user_util.h (right):

https://codereview.chromium.org/1428213004/diff/100001/chrome/browser/ui/ash/...
chrome/browser/ui/ash/multi_user/multi_user_util.h:36: // Gets the current user.
Note that this will return real AccountId for
s/real/a valid/

https://codereview.chromium.org/1428213004/diff/100001/chrome/browser/ui/ash/...
File chrome/browser/ui/ash/session_state_delegate_chromeos.cc (right):

https://codereview.chromium.org/1428213004/diff/100001/chrome/browser/ui/ash/...
chrome/browser/ui/ash/session_state_delegate_chromeos.cc:166:
gaia::CanonicalizeEmail(gaia::SanitizeEmail(account_id.GetUserEmail())));
Is this DCHECK really necessary any more?

https://codereview.chromium.org/1428213004/diff/100001/chrome/browser/ui/ash/...
File chrome/browser/ui/ash/system_tray_delegate_chromeos.h (right):

https://codereview.chromium.org/1428213004/diff/100001/chrome/browser/ui/ash/...
chrome/browser/ui/ash/system_tray_delegate_chromeos.h:296:
base::ScopedPtrHashMap<AccountId, scoped_ptr<ash::tray::UserAccountsDelegate>>
Is AccointId going to hash well? I think that ScopedPtrMap might be better here.
(I guess it doesn't matter since there are not going to be many anyway).

[Alexander Alekseev, 2015-11-06 23:38:58]

alemate@chromium.org changed reviewers:
+ skuhne@chromium.org

[Alexander Alekseev, 2015-11-06 23:38:59]

+skuhne@ for c/b/ui/ash/multi_user .

https://codereview.chromium.org/1428213004/diff/100001/chrome/browser/ui/ash/...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc (right):

https://codereview.chromium.org/1428213004/diff/100001/chrome/browser/ui/ash/...
chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc:1413:
.GetUserEmail()] = list;
On 2015/11/06 17:45:29, stevenjb wrote:
> Can we break this up to call out that we are extracting user_id here and using
> that as a key? i.e.
> std::string user_id =
>   multi_user_util::GetAccountIdFromProfile(profile_).GetUserEmail();
> 

Done.

https://codereview.chromium.org/1428213004/diff/100001/chrome/browser/ui/ash/...
File chrome/browser/ui/ash/multi_user/multi_user_context_menu_chromeos.cc
(right):

https://codereview.chromium.org/1428213004/diff/100001/chrome/browser/ui/ash/...
chrome/browser/ui/ash/multi_user/multi_user_context_menu_chromeos.cc:79: const
AccountId account_id = manager->GetWindowOwner(window);
On 2015/11/06 17:45:29, stevenjb wrote:
> const AccountId& ?

Done.

https://codereview.chromium.org/1428213004/diff/100001/chrome/browser/ui/ash/...
File chrome/browser/ui/ash/multi_user/multi_user_util.h (right):

https://codereview.chromium.org/1428213004/diff/100001/chrome/browser/ui/ash/...
chrome/browser/ui/ash/multi_user/multi_user_util.h:36: // Gets the current user.
Note that this will return real AccountId for
On 2015/11/06 17:45:29, stevenjb wrote:
> s/real/a valid/

Done.

https://codereview.chromium.org/1428213004/diff/100001/chrome/browser/ui/ash/...
File chrome/browser/ui/ash/session_state_delegate_chromeos.cc (right):

https://codereview.chromium.org/1428213004/diff/100001/chrome/browser/ui/ash/...
chrome/browser/ui/ash/session_state_delegate_chromeos.cc:166:
gaia::CanonicalizeEmail(gaia::SanitizeEmail(account_id.GetUserEmail())));
On 2015/11/06 17:45:29, stevenjb wrote:
> Is this DCHECK really necessary any more?

It is still needed for the transition time. I'll drop it later.

https://codereview.chromium.org/1428213004/diff/100001/chrome/browser/ui/ash/...
File chrome/browser/ui/ash/system_tray_delegate_chromeos.h (right):

https://codereview.chromium.org/1428213004/diff/100001/chrome/browser/ui/ash/...
chrome/browser/ui/ash/system_tray_delegate_chromeos.h:296:
base::ScopedPtrHashMap<AccountId, scoped_ptr<ash::tray::UserAccountsDelegate>>
On 2015/11/06 17:45:29, stevenjb wrote:
> Is AccointId going to hash well? I think that ScopedPtrMap might be better
here.
> (I guess it doesn't matter since there are not going to be many anyway).

Done.

[Alexander Alekseev, 2015-11-06 23:39:09]

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

[commit-bot: I haz the power, 2015-11-06 23:40:21]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1428213004/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1428213004/120001

[stevenjb, 2015-11-06 23:52:31]

ash/ and chrome/browser/ui/ash/ lgtm

[commit-bot: I haz the power, 2015-11-07 00:21:39]

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

[commit-bot: I haz the power, 2015-11-07 00:21:40]

Dry run: This issue passed the CQ dry run.

[Alexander Alekseev, 2015-11-11 02:05:20]

skuhne@ . friendly ping.

[Mr4D (OOO till 08-26), 2015-11-11 05:19:02]

ui/ash/multi_user lgtm

[Alexander Alekseev, 2015-11-11 14:06:17]

sky@ : Please review all except c/b/ui/ash/* :

ash/* (as owner)
c/b/c/accessibility/accessibility_manager.cc
c/b/c/accessibility/accessibility_manager.h
c/b/c/accessibility/magnification_manager.cc
c/b/c/extensions/file_manager/private_api_misc.cc
c/b/c/extensions/wallpaper_private_api_unittest.cc
c/b/c/file_system_provider/notification_manager.cc
c/b/feedback/show_feedback_page.cc
c/b/notifications/message_center_notifications_unittest.cc
c/b/notifications/profile_notification.cc
c/b/signin/signin_error_notifier_ash.cc
c/b/sync/sync_error_notifier_ash.cc
c/b/ui/browser_finder.cc
c/b/ui/browser_finder_chromeos_unittest.cc
c/b/ui/browser_navigator.cc
c/b/ui/browser_navigator_browsertest_chromeos.cc
c/b/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc
c/b/ui/views/frame/system_menu_model_builder.cc

[sky, 2015-11-11 20:15:34]

LGTM

https://codereview.chromium.org/1428213004/diff/120001/ash/system/tray/system...
File ash/system/tray/system_tray_delegate.cc (right):

https://codereview.chromium.org/1428213004/diff/120001/ash/system/tray/system...
ash/system/tray/system_tray_delegate.cc:262: const AccountId& /* account_id */)
{
I prefer consistency. The rest of this class doesn't comment out the names, so
don't do it here either (this comment applies to a bunch of places in your
patch).

https://codereview.chromium.org/1428213004/diff/120001/chrome/browser/ui/ash/...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc (right):

https://codereview.chromium.org/1428213004/diff/120001/chrome/browser/ui/ash/...
chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc:1411: const
std::string user_id =
user_email?

https://codereview.chromium.org/1428213004/diff/120001/chrome/browser/ui/ash/...
File chrome/browser/ui/ash/multi_user/multi_user_window_manager.h (right):

https://codereview.chromium.org/1428213004/diff/120001/chrome/browser/ui/ash/...
chrome/browser/ui/ash/multi_user/multi_user_window_manager.h:104: // string if
not assigned yet.
nit: not string anymore.

[Alexander Alekseev, 2015-11-11 23:32:06]

https://codereview.chromium.org/1428213004/diff/120001/ash/system/tray/system...
File ash/system/tray/system_tray_delegate.cc (right):

https://codereview.chromium.org/1428213004/diff/120001/ash/system/tray/system...
ash/system/tray/system_tray_delegate.cc:262: const AccountId& /* account_id */)
{
On 2015/11/11 20:15:34, sky wrote:
> I prefer consistency. The rest of this class doesn't comment out the names, so
> don't do it here either (this comment applies to a bunch of places in your
> patch).

Done.

https://codereview.chromium.org/1428213004/diff/120001/chrome/browser/ui/ash/...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc (right):

https://codereview.chromium.org/1428213004/diff/120001/chrome/browser/ui/ash/...
chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc:1411: const
std::string user_id =
On 2015/11/11 20:15:34, sky wrote:
> user_email?

Done.

https://codereview.chromium.org/1428213004/diff/120001/chrome/browser/ui/ash/...
File chrome/browser/ui/ash/multi_user/multi_user_window_manager.h (right):

https://codereview.chromium.org/1428213004/diff/120001/chrome/browser/ui/ash/...
chrome/browser/ui/ash/multi_user/multi_user_window_manager.h:104: // string if
not assigned yet.
On 2015/11/11 20:15:34, sky wrote:
> nit: not string anymore.

Done.

[Alexander Alekseev, 2015-11-11 23:32:18]

The CQ bit was checked by alemate@chromium.org

[Alexander Alekseev, 2015-11-11 23:32:19]

The patchset sent to the CQ was uploaded after l-g-t-m from
stevenjb@chromium.org, sky@chromium.org, skuhne@chromium.org
Link to the patchset: https://codereview.chromium.org/1428213004/#ps140001
(title: "Update after review.")

[commit-bot: I haz the power, 2015-11-11 23:34:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1428213004/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1428213004/140001

[commit-bot: I haz the power, 2015-11-12 03:15:24]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2015-11-12 20:03:32]

Patchset 8 (id:??) landed as
https://crrev.com/cdf46dd1677ae5a8974fb4b1677b39d89f973a36
Cr-Commit-Position: refs/heads/master@{#359225}