Pass Device ID in the oauth2/token request. Keep Device ID in local state on Chrome OS.

chrome/browser/signin/chrome_signin_client.cc

Index: chrome/browser/signin/chrome_signin_client.cc
diff --git a/chrome/browser/signin/chrome_signin_client.cc b/chrome/browser/signin/chrome_signin_client.cc
index a98dd89930a065fbedb05edce03982d03b268889..a7a1d863f94f5d6337c42375ca55a8540965f070 100644
--- a/chrome/browser/signin/chrome_signin_client.cc
+++ b/chrome/browser/signin/chrome_signin_client.cc
@@ -33,6 +33,7 @@
 
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/chromeos/net/delay_network_call.h"
+#include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "components/user_manager/user_manager.h"
 #endif
 
@@ -47,6 +48,27 @@ ChromeSigninClient::ChromeSigninClient(
   signin_error_controller_->AddObserver(this);
 #if !defined(OS_CHROMEOS)
   net::NetworkChangeNotifier::AddNetworkChangeObserver(this);
+#else
+  // UserManager may not exist in unit_tests.
+  if (!user_manager::UserManager::IsInitialized())
+    return;
+
+  const user_manager::User* user =
+      chromeos::ProfileHelper::Get()->GetUserByProfile(profile_);
+  if (!user)
+    return;
+
+  // Need to move device ID from the old location to the new one, if it has not

[achuithb, 2015/05/12 23:25:01]

Should this code be retired after a few milestones?

[dzhioev (left Google), 2015/05/13 00:25:18]

I believe it shouldn't. We can't be sure that every existing user has signed in
at least once, no matter how much time passed.

+  // been done yet.
+  std::string legacy_device_id =

[achuithb, 2015/05/12 23:25:01]

nit: const

[dzhioev (left Google), 2015/05/13 00:25:18]

Done.

+      GetPrefs()->GetString(prefs::kGoogleServicesSigninScopedDeviceId);
+  user_manager::UserManager* user_manager = user_manager::UserManager::Get();

[achuithb, 2015/05/12 23:25:01]

nit: could potentially use auto here since the type is clear - up to you.

[dzhioev (left Google), 2015/05/13 00:25:18]

Sure

+  if (!legacy_device_id.empty() &&
+      user_manager->GetKnownUserDeviceId(user->GetUserID()).empty()) {
+    user_manager->SetKnownUserDeviceId(user->GetUserID(), legacy_device_id);
+  }

[pavely, 2015/05/13 00:05:52]

Could you add DCHECK that fires when both device_ids nonempty and don't match?

[dzhioev (left Google), 2015/05/13 20:24:22]

Unfortunately such thing can happen, and I don't know how to prevent it. It is
an edge case of migration process. This happens if user uses GAIA for the first
sign in after upgrade to M44:
1) User chooses "Add User" and inputs credentials of existing user.
2) User is forced to use GAIA because his token is invalid or user pods are not
displayed (there is a special preference for that).

As I said, this is an edge case. Will it break anything (expect the fact that
new device_id will be issued)? We can detect this situation after sign in and
react somehow (for example revoke old device id, if it possible).

[Nikita (slow), 2015/05/14 06:29:03]

This is an expected edge case of a migration flow with M44. See
go/cros-auth-design, flow 1, step 32 (specifically red line).

[Nikita (slow), 2015/05/14 06:34:14]

After such edge case happens and newly created device ID is passed to GAIA, same
device ID needs to used by sync: part of
https://code.google.com/p/chromium/issues/detail?id=486136

+  GetPrefs()->SetString(prefs::kGoogleServicesSigninScopedDeviceId,
+                        std::string());
 #endif
 }
 
@@ -113,6 +135,7 @@ std::string ChromeSigninClient::GetSigninScopedDeviceId() {
     return std::string();
   }
 
+#if !defined(OS_CHROMEOS)
   std::string signin_scoped_device_id =
       GetPrefs()->GetString(prefs::kGoogleServicesSigninScopedDeviceId);
   if (signin_scoped_device_id.empty()) {
@@ -123,6 +146,22 @@ std::string ChromeSigninClient::GetSigninScopedDeviceId() {
                           signin_scoped_device_id);
   }
   return signin_scoped_device_id;
+#else
+  // UserManager may not exist in unit_tests.
+  if (!user_manager::UserManager::IsInitialized())
+    return std::string();
+
+  const user_manager::User* user =
+      chromeos::ProfileHelper::Get()->GetUserByProfile(profile_);
+  if (!user)
+    return std::string();
+
+  std::string signin_scoped_device_id =

[achuithb, 2015/05/12 23:25:01]

nit: const

[dzhioev (left Google), 2015/05/13 00:25:18]

Done.

+      user_manager::UserManager::Get()->GetKnownUserDeviceId(user->GetUserID());
+  if (signin_scoped_device_id.empty())
+    LOG(ERROR) << "Device ID is not set for user.";

[pavely, 2015/05/13 00:05:52]

How device_id will be set for users that went straight from M36 to M44? They
wouldn't have device_id set in Preferences in cryptohome and so it will be empty
here as well.
Do we care about such case?
One solution would be to say that such users will have to sign out and sign in
again and by doing so they will go through codepath that generates new
device_id.

[dzhioev (left Google), 2015/05/13 20:24:22]

Oh, I missed this case.
We can catch this by checking if both KnownUserDeviceID and
kGoogleServicesSigninScopedDeviceId are empty in ChromeSingninClient
constructor. Done.

+  return signin_scoped_device_id;
+#endif
 }
 
 void ChromeSigninClient::OnSignedOut() {