ChromeOS: fix user signin by alias.

This CL fixes the issue with login using alternative e-mails.

If new user is added, we lookup if it already exists by e-mail.
This CL also adds lookup by gaia_id, and adds gaia_id support
for telemetry test API.

BUG=535369
TEST=manual

Committed: https://crrev.com/0f37b696b7a814d4157401c304643594a4ae9731
Cr-Commit-Position: refs/heads/master@{#351482}

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

[Alexander Alekseev, 2015-09-26 09:49:56]

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

[commit-bot: I haz the power, 2015-09-26 09:50:05]

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

[commit-bot: I haz the power, 2015-09-26 10:25:53]

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

[commit-bot: I haz the power, 2015-09-26 10:25:54]

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

[Alexander Alekseev, 2015-09-29 12:25:34]

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

[Alexander Alekseev, 2015-09-29 12:26:02]

Please review.

[Alexander Alekseev, 2015-09-29 12:26:07]

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

[commit-bot: I haz the power, 2015-09-29 12:26:19]

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

[commit-bot: I haz the power, 2015-09-29 13:30:29]

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

[commit-bot: I haz the power, 2015-09-29 13:30:30]

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

[achuithb, 2015-09-29 18:18:42]

We need browser tests for this. Do you want to do it in a separate CL or can you
add them to this one?

https://codereview.chromium.org/1370913002/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc (right):

https://codereview.chromium.org/1370913002/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:530: LOG(WARNING)
<< "Existing user '" << old_email
Should we make this LOG(INFO)?

Also, should we add UMA stats for this event?

https://codereview.chromium.org/1370913002/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:537: return
sanitized_email;
The names are a bit confusing.

This method is called GetCanonicalEmail, you have variables, sanitized_email,
canonicalized_email, old_email, authenticated_email, gaia_id. What is the
difference between all of these? Could we add a comment somewhere?

[Alexander Alekseev, 2015-09-29 19:02:08]

> We need browser tests for this. Do you want to do it in
> a separate CL or can you add them to this one?

I think it is better to have tests in a separate CL to
speed up testing.

Also, display e-mail is a bit more tricky, so I'll fix it in a separate CL.

https://codereview.chromium.org/1370913002/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc (right):

https://codereview.chromium.org/1370913002/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:530: LOG(WARNING)
<< "Existing user '" << old_email
On 2015/09/29 18:18:42, achuithb wrote:
> Should we make this LOG(INFO)?
> 
> Also, should we add UMA stats for this event?

LOG(INFO) is prohibited by precommit hooks.
So you think we should still use INFO?

https://codereview.chromium.org/1370913002/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:537: return
sanitized_email;
On 2015/09/29 18:18:42, achuithb wrote:
> The names are a bit confusing.
> 
> This method is called GetCanonicalEmail, you have variables, sanitized_email,
> canonicalized_email, old_email, authenticated_email, gaia_id. What is the
> difference between all of these? Could we add a comment somewhere?

It actually is returning sanitized email for compatibility.
We should probably call Canonicalize() here, but currently we rely on GAIA to
return canonical e-mail.

On the other hand, UserContext returns canonicalized e-mail, so that KnownUsers
data is stored using canonicalized e-mails as ID.

This way, it all works because Canonicalize() is done on GAIA side ;)

I've added comment here.

[achuithb, 2015-09-29 19:07:48]

https://codereview.chromium.org/1370913002/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc (right):

https://codereview.chromium.org/1370913002/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:530: LOG(WARNING)
<< "Existing user '" << old_email
On 2015/09/29 19:02:07, Alexander Alekseev wrote:
> On 2015/09/29 18:18:42, achuithb wrote:
> > Should we make this LOG(INFO)?
> > 
> > Also, should we add UMA stats for this event?
> 
> LOG(INFO) is prohibited by precommit hooks.
> So you think we should still use INFO?

Doesn't feel like a WARNING, but I guess it's ok.

What about the UMA stat?

[Alexander Alekseev, 2015-09-29 19:12:43]

On 2015/09/29 19:07:48, achuithb wrote:
>
https://codereview.chromium.org/1370913002/diff/20001/chrome/browser/ui/webui...
> File chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc (right):
> 
>
https://codereview.chromium.org/1370913002/diff/20001/chrome/browser/ui/webui...
> chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:530:
LOG(WARNING)
> << "Existing user '" << old_email
> On 2015/09/29 19:02:07, Alexander Alekseev wrote:
> > On 2015/09/29 18:18:42, achuithb wrote:
> > > Should we make this LOG(INFO)?
> > > 
> > > Also, should we add UMA stats for this event?
> > 
> > LOG(INFO) is prohibited by precommit hooks.
> > So you think we should still use INFO?
> 
> Doesn't feel like a WARNING, but I guess it's ok.
> 
> What about the UMA stat?

Let it be the separate CL, as we probably need to measure the number of users
using aliases, not the number of password change attempts.

[achuithb, 2015-09-29 19:28:40]

On 2015/09/29 19:12:43, Alexander Alekseev wrote:
> On 2015/09/29 19:07:48, achuithb wrote:
> >
>
https://codereview.chromium.org/1370913002/diff/20001/chrome/browser/ui/webui...
> > File chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc (right):
> > 
> >
>
https://codereview.chromium.org/1370913002/diff/20001/chrome/browser/ui/webui...
> > chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:530:
> LOG(WARNING)
> > << "Existing user '" << old_email
> > On 2015/09/29 19:02:07, Alexander Alekseev wrote:
> > > On 2015/09/29 18:18:42, achuithb wrote:
> > > > Should we make this LOG(INFO)?
> > > > 
> > > > Also, should we add UMA stats for this event?
> > > 
> > > LOG(INFO) is prohibited by precommit hooks.
> > > So you think we should still use INFO?
> > 
> > Doesn't feel like a WARNING, but I guess it's ok.
> > 
> > What about the UMA stat?
> 
> Let it be the separate CL, as we probably need to measure the number of users
> using aliases, not the number of password change attempts.

could you please file bugs for these 2 issues? UMA and browser tests.

[Alexander Alekseev, 2015-09-29 20:22:29]

On 2015/09/29 19:28:40, achuithb wrote:
> On 2015/09/29 19:12:43, Alexander Alekseev wrote:
> > On 2015/09/29 19:07:48, achuithb wrote:
> > >
> >
>
https://codereview.chromium.org/1370913002/diff/20001/chrome/browser/ui/webui...
> > > File chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc
(right):
> > > 
> > >
> >
>
https://codereview.chromium.org/1370913002/diff/20001/chrome/browser/ui/webui...
> > > chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:530:
> > LOG(WARNING)
> > > << "Existing user '" << old_email
> > > On 2015/09/29 19:02:07, Alexander Alekseev wrote:
> > > > On 2015/09/29 18:18:42, achuithb wrote:
> > > > > Should we make this LOG(INFO)?
> > > > > 
> > > > > Also, should we add UMA stats for this event?
> > > > 
> > > > LOG(INFO) is prohibited by precommit hooks.
> > > > So you think we should still use INFO?
> > > 
> > > Doesn't feel like a WARNING, but I guess it's ok.
> > > 
> > > What about the UMA stat?
> > 
> > Let it be the separate CL, as we probably need to measure the number of
users
> > using aliases, not the number of password change attempts.
> 
> could you please file bugs for these 2 issues? UMA and browser tests.

Done.

[achuithb, 2015-09-29 20:35:49]

lgtm

[Alexander Alekseev, 2015-09-29 20:36:07]

The CQ bit was checked by alemate@chromium.org

[commit-bot: I haz the power, 2015-09-29 20:36:20]

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

[commit-bot: I haz the power, 2015-09-29 21:13:05]

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

[commit-bot: I haz the power, 2015-09-29 21:13:06]

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

[Alexander Alekseev, 2015-09-30 02:51:35]

The CQ bit was checked by alemate@chromium.org

[Alexander Alekseev, 2015-09-30 02:51:35]

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

[commit-bot: I haz the power, 2015-09-30 02:51:46]

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

[commit-bot: I haz the power, 2015-09-30 03:31:26]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-09-30 03:32:04]

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

[afakhry, 2015-10-01 21:14:25]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/1384533003/ by afakhry@chromium.org.

The reason for reverting is: This Cl is causing the test
login_OwnershipNotRetaken to fail on ChromePFQ.
https://code.google.com/p/chromium/issues/detail?id=537986..

[Alexander Alekseev, 2015-10-03 02:57:18]

I've added fix for telemetry tests.

PTAL.

[Alexander Alekseev, 2015-10-03 02:57:39]

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

[commit-bot: I haz the power, 2015-10-03 02:58:00]

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

[commit-bot: I haz the power, 2015-10-03 07:12:28]

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

[commit-bot: I haz the power, 2015-10-03 07:12:28]

Dry run: This issue passed the CQ dry run.

[achuithb, 2015-10-05 19:38:08]

https://codereview.chromium.org/1370913002/diff/100001/chrome/browser/resourc...
File chrome/browser/resources/chromeos/login/login_common.js (right):

https://codereview.chromium.org/1370913002/diff/100001/chrome/browser/resourc...
chrome/browser/resources/chromeos/login/login_common.js:291: if (!gaia_id) {
Should we add a TODO here to remove this?

gaia_id should always be set, right?

[achuithb, 2015-10-05 19:39:18]

On 2015/10/05 19:38:08, achuithb wrote:
>
https://codereview.chromium.org/1370913002/diff/100001/chrome/browser/resourc...
> File chrome/browser/resources/chromeos/login/login_common.js (right):
> 
>
https://codereview.chromium.org/1370913002/diff/100001/chrome/browser/resourc...
> chrome/browser/resources/chromeos/login/login_common.js:291: if (!gaia_id) {
> Should we add a TODO here to remove this?
> 
> gaia_id should always be set, right?

Please expand the description of this CL with bullet points of what has changed.

[Alexander Alekseev, 2015-10-05 21:21:28]

https://codereview.chromium.org/1370913002/diff/100001/chrome/browser/resourc...
File chrome/browser/resources/chromeos/login/login_common.js (right):

https://codereview.chromium.org/1370913002/diff/100001/chrome/browser/resourc...
chrome/browser/resources/chromeos/login/login_common.js:291: if (!gaia_id) {
On 2015/10/05 19:38:08, achuithb wrote:
> Should we add a TODO here to remove this?
> 
> gaia_id should always be set, right?

Done.

[achuithb, 2015-10-06 07:36:51]

lgtm

[Alexander Alekseev, 2015-10-06 08:19:44]

The CQ bit was checked by alemate@chromium.org

[commit-bot: I haz the power, 2015-10-06 08:20:09]

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

[commit-bot: I haz the power, 2015-10-06 09:26:49]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-10-06 09:27:59]

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