Implementation of SigninClient::GetSigninScopedDeviceId

chrome/browser/ui/sync/one_click_signin_helper.cc

Index: chrome/browser/ui/sync/one_click_signin_helper.cc
diff --git a/chrome/browser/ui/sync/one_click_signin_helper.cc b/chrome/browser/ui/sync/one_click_signin_helper.cc
index 6962977fb3d9eefb1aa2013765182496b48857d7..66e20de3f2bc0556d3a17567ee3607d9d07c360e 100644
--- a/chrome/browser/ui/sync/one_click_signin_helper.cc
+++ b/chrome/browser/ui/sync/one_click_signin_helper.cc
@@ -414,13 +414,14 @@ bool AreWeShowingSignin(GURL url, signin::Source source, std::string email) {
        !email.empty());
 }
 
-// Gets signin scoped device id from signin client if profile is valid.
-// Otherwise returns empty string.
-std::string GetSigninScopedDeviceId(Profile* profile) {
+// If profile is valid then generates new signin scoped device id from signin
+// client. Otherwise returns empty string.
+std::string GenerateNewSigninScopedDeviceId(Profile* profile) {
   std::string signin_scoped_device_id;
   SigninClient* signin_client =
       profile ? ChromeSigninClientFactory::GetForProfile(profile) : NULL;
   if (signin_client) {
+    signin_client->ClearSigninScopedDeviceId();
     signin_scoped_device_id = signin_client->GetSigninScopedDeviceId();
   }
   return signin_scoped_device_id;
@@ -516,7 +517,6 @@ OneClickSigninHelper::StartSyncArgs::StartSyncArgs(
     const std::string& email,
     const std::string& password,
     const std::string& refresh_token,
-    const std::string& signin_scoped_device_id,
     content::WebContents* web_contents,
     bool untrusted_confirmation_required,
     signin::Source source,
@@ -528,7 +528,6 @@ OneClickSigninHelper::StartSyncArgs::StartSyncArgs(
       email(email),
       password(password),
       refresh_token(refresh_token),
-      signin_scoped_device_id(signin_scoped_device_id),
       web_contents(web_contents),
       source(source),
       callback(callback) {
@@ -661,10 +660,12 @@ void OneClickSigninHelper::SyncStarterWrapper::DisplayErrorBubble(
 }
 
 void OneClickSigninHelper::SyncStarterWrapper::StartSigninOAuthHelper() {
+  std::string signin_scoped_device_id =
+      GenerateNewSigninScopedDeviceId(args_.profile);

[pavely, 2014/06/30 23:31:09]

This is where new id generated before signing in.

[Roger Tawa OOO till Jul 10th, 2014/07/01 13:42:54]

For mirror, sign in occurs in inline_login_handler_impl.cc.  Should't we call
ClearSigninScopedDeviceId() there when the source !=
signin::SOURCE_AVATAR_BUBBLE_ADD_ACCOUNT && source != signin::SOURCE_REAUTH?

How about cros?  Will you handle that is a separate CL?

[pavely, 2014/07/01 21:41:47]

Ok, it seems like there are multiple codepaths that request refresh token and no
future-proof way to tell if particular call is new signin or reauth. I'll remove
call to clear device id from here and only leave one in signout codepath.

We'll see, maybe I'll find a better place to clear device_id on signin when i
know more details about how signin/signout works on chromeOS.

BTW, Where signin starts in case of mirror? Where should I start tracing code?

Yes, I'll handle CrOS in separate change.

On 2014/07/01 13:42:54, Roger Tawa wrote:

[Roger Tawa OOO till Jul 10th, 2014/07/03 15:07:41]

In mirror, start tracing from  InlineLoginHandlerImpl::CompleteLogin(),
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/....

   signin_oauth_helper_.reset(
       new SigninOAuthHelper(args_.profile->GetRequestContext(),
                             args_.session_index,
-                            args_.signin_scoped_device_id,
+                            signin_scoped_device_id,
                             this));
 }
 
@@ -1181,7 +1182,6 @@ bool OneClickSigninHelper::HandleCrossAccountError(
       Profile::FromBrowserContext(contents->GetBrowserContext());
   std::string last_email =
       profile->GetPrefs()->GetString(prefs::kGoogleServicesLastUsername);
-  std::string signin_scoped_device_id = GetSigninScopedDeviceId(profile);
 
   if (!last_email.empty() && !gaia::AreEmailsSame(last_email, email)) {
     // If the new email address is different from the email address that
@@ -1200,7 +1200,7 @@ bool OneClickSigninHelper::HandleCrossAccountError(
             &StartExplicitSync,
             StartSyncArgs(profile, browser, auto_accept,
                           session_index, email, password,
-                          refresh_token, signin_scoped_device_id,
+                          refresh_token,
                           contents, false /* confirmation_required */, source,
                           sync_callback),
             contents,
@@ -1487,7 +1487,6 @@ void OneClickSigninHelper::DidStopLoading(
           << " auto_accept=" << auto_accept_
           << " source=" << source_;
 
-  std::string signin_scoped_device_id = GetSigninScopedDeviceId(profile);
   switch (auto_accept_) {
     case AUTO_ACCEPT_NONE:
       if (showing_signin_)
@@ -1502,8 +1501,7 @@ void OneClickSigninHelper::DidStopLoading(
       if (!do_not_start_sync_for_testing_) {
         StartSync(
             StartSyncArgs(profile, browser, auto_accept_,
-                          session_index_, email_, password_,
-                          "", signin_scoped_device_id,
+                          session_index_, email_, password_, "",
                           NULL  /* don't force sync setup in same tab */,
                           true  /* confirmation_required */, source_,
                           CreateSyncStarterCallback()),
@@ -1519,8 +1517,7 @@ void OneClickSigninHelper::DidStopLoading(
       if (!do_not_start_sync_for_testing_) {
         StartSync(
             StartSyncArgs(profile, browser, auto_accept_,
-                          session_index_, email_, password_,
-                          "", signin_scoped_device_id,
+                          session_index_, email_, password_, "",
                           NULL  /* don't force sync setup in same tab */,
                           true  /* confirmation_required */, source_,
                           CreateSyncStarterCallback()),
@@ -1566,8 +1563,7 @@ void OneClickSigninHelper::DidStopLoading(
         if (!do_not_start_sync_for_testing_) {
           StartSync(
               StartSyncArgs(profile, browser, auto_accept_,
-                            session_index_, email_, password_,
-                            "", signin_scoped_device_id,
+                            session_index_, email_, password_, "",
                             contents,
                             untrusted_confirmation_required_, source_,
                             CreateSyncStarterCallback()),