ChromeOS: Fix removeUser button on signin screen.

ChromeOS: Fix removeUser button on signin screen.

BUG=462823
TEST=manual

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

[Alexander Alekseev, 2015-11-21 02:37:09]

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

[Alexander Alekseev, 2015-11-21 02:37:26]

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

[Alexander Alekseev, 2015-11-21 02:37:26]

Please review.

[commit-bot: I haz the power, 2015-11-21 02:38:40]

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

[Alexander Alekseev, 2015-11-21 02:40:03]

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

[commit-bot: I haz the power, 2015-11-21 02:41:17]

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

[commit-bot: I haz the power, 2015-11-21 03:40:52]

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

[commit-bot: I haz the power, 2015-11-21 03:40:52]

Dry run: This issue passed the CQ dry run.

[achuithb, 2015-11-23 21:17:39]

achuith@chromium.org changed reviewers:
+ achuith@chromium.org

[achuithb, 2015-11-23 21:17:39]

lgtm

https://codereview.chromium.org/1470633002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/signin_screen_controller.h (right):

https://codereview.chromium.org/1470633002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/login/signin_screen_controller.h:54: //
user_manager::RemoveUserDelegate implementation:
could you make everything below here private while you're here?

https://codereview.chromium.org/1470633002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc (right):

https://codereview.chromium.org/1470633002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:1163:
account_id.GetUserEmail(), connector->GetEnterpriseDomain());
I assume this will take account_id in the future?

https://codereview.chromium.org/1470633002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:1192:
base::StringValue(FullyCanonicalize(account_id.GetUserEmail())), NULL);
Is FullyCanonicalize necessary here?

[dzhioev (left Google), 2015-11-24 00:05:44]

On 2015/11/21 02:37:26, Alexander Alekseev wrote:
> Please review.

LGTM

[Alexander Alekseev, 2015-11-24 06:04:11]

https://codereview.chromium.org/1470633002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/signin_screen_controller.h (right):

https://codereview.chromium.org/1470633002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/login/signin_screen_controller.h:54: //
user_manager::RemoveUserDelegate implementation:
On 2015/11/23 21:17:39, achuithb wrote:
> could you make everything below here private while you're here?

Hm. Do you think we should make overridden public methods private?

https://codereview.chromium.org/1470633002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc (right):

https://codereview.chromium.org/1470633002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:1163:
account_id.GetUserEmail(), connector->GetEnterpriseDomain());
On 2015/11/23 21:17:39, achuithb wrote:
> I assume this will take account_id in the future?

Yes.

https://codereview.chromium.org/1470633002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc:1192:
base::StringValue(FullyCanonicalize(account_id.GetUserEmail())), NULL);
On 2015/11/23 21:17:39, achuithb wrote:
> Is FullyCanonicalize necessary here?

I left it here in order not to miss this fact later, when kReportingUsers will
be migrated.

[Alexander Alekseev, 2015-11-24 06:58:01]

https://codereview.chromium.org/1470633002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/signin_screen_controller.h (right):

https://codereview.chromium.org/1470633002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/login/signin_screen_controller.h:54: //
user_manager::RemoveUserDelegate implementation:
On 2015/11/23 21:17:39, achuithb wrote:
> could you make everything below here private while you're here?

Done.

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

The CQ bit was checked by alemate@chromium.org

[Alexander Alekseev, 2015-11-24 06:58:39]

The patchset sent to the CQ was uploaded after l-g-t-m from
dzhioev@chromium.org, achuith@chromium.org
Link to the patchset: https://codereview.chromium.org/1470633002/#ps40001
(title: "Update after review.")

[commit-bot: I haz the power, 2015-11-24 06:58:57]

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

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

This CL has an open dependency (Issue 1463753002 Patch 70001). Please resolve
the dependency and try again.

[Alexander Alekseev, 2015-11-24 08:32:18]

The CQ bit was checked by alemate@chromium.org

[Alexander Alekseev, 2015-11-24 08:32:18]

The patchset sent to the CQ was uploaded after l-g-t-m from
dzhioev@chromium.org, achuith@chromium.org
Link to the patchset: https://codereview.chromium.org/1470633002/#ps60001
(title: "Rebased.")

[commit-bot: I haz the power, 2015-11-24 08:34:29]

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

[commit-bot: I haz the power, 2015-11-24 09:04:15]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-11-24 09:04:57]

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