sync: change semantics (and name) of SigninManager::GetUsername

chrome/browser/sync/profile_sync_service.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/sync/profile_sync_service.h"
 
 #include <algorithm>
 #include <cstddef>
 #include <map>
 #include <set>
@@ skipping 128 matching lines @@
   sync_prefs_.RemoveSyncPrefObserver(this);
   Shutdown(false);
 }
 
 bool ProfileSyncService::AreCredentialsAvailable() {
   if (IsManaged()) {
     return false;
   }
 
   // CrOS user is always logged in. Chrome uses signin_ to check logged in.
-  if (!cros_user_.empty() || !signin_->GetUsername().empty()) {
+  if (!cros_user_.empty() || !signin_->GetAuthenticatedUsername().empty()) {
     // TODO(chron): Verify CrOS unit test behavior.
     return profile()->GetTokenService() &&
         profile()->GetTokenService()->HasTokenForService(
             browser_sync::SyncServiceName());
   }
   return false;
 }
 
 void ProfileSyncService::Initialize() {
   InitSettings();
 
   // We clear this here (vs Shutdown) because we want to remember that an error
   // happened on shutdown so we can display details (message, location) about it
   // in about:sync.
   ClearStaleErrors();
 
   sync_prefs_.AddSyncPrefObserver(this);
 
   // For now, the only thing we can do through policy is to turn sync off.
   if (IsManaged()) {
     DisableForUser();
     return;
   }
 
   RegisterAuthNotifications();
 
   if (!HasSyncSetupCompleted())
     DisableForUser();  // Clean up in case of previous crash / setup abort.
 
-  // In Chrome, we integrate a SigninManager which works with the sync
-  // setup wizard to kick off the TokenService. CrOS does its own plumbing
-  // for the TokenService in login and does not normally rely on signin_,
-  // so only initialize this if the token service has not been initialized
-  // (e.g. the browser crashed or is being debugged).
-  if (cros_user_.empty() ||
-      !profile_->GetTokenService()->Initialized()) {
-    // Will load tokens from DB and broadcast Token events after.
-    // Note: We rely on signin_ != NULL unless !cros_user_.empty().
-    signin_->Initialize(profile_);
-  }
+  signin_->Initialize(profile_);
 
   TryStart();
 }
 
 void ProfileSyncService::TryStart() {
   if (!sync_prefs_.IsStartSuppressed() && AreCredentialsAvailable()) {
     if (HasSyncSetupCompleted() || auto_start_enabled_) {
       // If sync setup has completed we always start the backend.
       // If autostart is enabled, but we haven't completed sync setup, we try to
       // start sync anyway, since it's possible we crashed/shutdown after
@@ skipping 72 matching lines @@
       } else {
         LOG(WARNING) << "The following sync URL specified at the command-line "
                      << "is invalid: " << value;
       }
     }
   }
 }
 
 SyncCredentials ProfileSyncService::GetCredentials() {
   SyncCredentials credentials;
-  credentials.email = cros_user_.empty() ? signin_->GetUsername() : cros_user_;
+  credentials.email = cros_user_.empty() ?

[Andrew T Wilson (Slow), 2011/12/06 21:09:15]

BTW, is the plan to eventually get rid of cros_user_ and instead move the
username for cros into SigninManager? I ask, because as we move the signin code
out of sync it would be nice for there to be one place that can authoritatively
say whether there's a signed-in user or not.

[tim (not reviewing), 2011/12/06 21:23:47]

As I mention in the CL description and in my first comment response (and there's
a blurb in the notes from yday's meeting too), I'm working towards removing
cros_user :) That (and having one single place store username info) is the sole
purpose for this CL.  I have a patch that I haven't sent up for review (because
it's based on this and my earlier removal of GetAuthenticatedUsername from PSS
so the diffs are messy) that removes cros_user, and distinguishes more
explicitly between auto-starting and manual-starting the service.

+      signin_->GetAuthenticatedUsername() : cros_user_;
   DCHECK(!credentials.email.empty());
   TokenService* service = profile_->GetTokenService();
   credentials.sync_token = service->GetTokenForService(
       browser_sync::SyncServiceName());
   return credentials;
 }
 
 void ProfileSyncService::InitializeBackend(bool delete_stale_data) {
   if (!backend_.get()) {
     NOTREACHED();
@@ skipping 773 matching lines @@
 
 void ProfileSyncService::OnUserSubmittedAuth(
     const std::string& username, const std::string& password,
     const std::string& captcha, const std::string& access_code) {
   last_attempted_user_email_ = username;
   is_auth_in_progress_ = true;
   NotifyObservers();
 
   auth_start_time_ = base::TimeTicks::Now();
 
-  if (!signin_->IsInitialized()) {
-    // In ChromeOS we sign in during login, so we do not initialize signin_.
-    // If this function gets called, we need to re-authenticate (e.g. for
-    // two factor signin), so initialize signin_ here.
-    signin_->Initialize(profile_);
-  }
-
   if (!access_code.empty()) {
     signin_->ProvideSecondFactorAccessCode(access_code);
     return;
   }
 
-  if (!signin_->GetUsername().empty()) {
-    signin_->ClearInMemoryData();
-  }
-
   // The user has submitted credentials, which indicates they don't
   // want to suppress start up anymore.
   sync_prefs_.SetStartSuppressed(false);
 
   signin_->StartSignIn(username,
                        password,
                        last_auth_error_.captcha().token,
                        captcha);
 }
 
@@ skipping 349 matching lines @@
       if (AreCredentialsAvailable()) {
         if (backend_initialized_) {
           backend_->UpdateCredentials(GetCredentials());
         }
         if (!sync_prefs_.IsStartSuppressed())
           StartUp();
       }
       break;
     }
     case chrome::NOTIFICATION_TOKEN_LOADING_FINISHED: {
-      // If not in Chrome OS, and we have a username without tokens,
-      // the user will need to signin again, so sign out.
+      // If not in auto-start / Chrome OS mode, and we have a username
+      // without tokens, the user will need to signin again. NotifyObservers to
+      // trigger errors in the UI that will allow the user to re-login.
       if (cros_user_.empty() &&
-          !signin_->GetUsername().empty() &&
+          !signin_->GetAuthenticatedUsername().empty() &&
           !AreCredentialsAvailable()) {
-        DisableForUser();
+        NotifyObservers();

[Andrew T Wilson (Slow), 2011/12/06 21:09:15]

Agreed with Lingesh - can you explain why the UI is going to display an error in
this case? Does the UI someplace check for AreCredentialsAvailable() rather than
just last_auth_error?

[tim (not reviewing), 2011/12/06 21:23:47]

Woops, I musn't have uploaded the right patchset. I addressed this, will
reupload shortly.

       }
       break;
     }
     default: {
       NOTREACHED();
     }
   }
 }
 
 void ProfileSyncService::AddObserver(Observer* observer) {
@@ skipping 43 matching lines @@
 void ProfileSyncService::StopAndSuppress() {
   sync_prefs_.SetStartSuppressed(true);
   Shutdown(false);
 }
 
 void ProfileSyncService::UnsuppressAndStart() {
   DCHECK(profile_);
   sync_prefs_.SetStartSuppressed(false);
   // Set username in SigninManager, as SigninManager::OnGetUserInfoSuccess
   // is never called for some clients.
-  if (signin_->GetUsername().empty()) {
-    signin_->SetUsername(sync_prefs_.GetGoogleServicesUsername());
+  if (signin_->GetAuthenticatedUsername().empty()) {
+    signin_->SetAuthenticatedUsername(
+        sync_prefs_.GetGoogleServicesUsername());
   }
   TryStart();
 }
 
 void ProfileSyncService::AcknowledgeSyncedTypes() {
   syncable::ModelTypeSet registered_types;
   GetRegisteredDataTypes(&registered_types);
   sync_prefs_.AcknowledgeSyncedTypes(registered_types);
 }
 
@@ skipping 15 matching lines @@
              << "Unrecoverable error.";
   } else {
     DVLOG(0) << "ConfigureDataTypeManager not invoked because backend is not "
              << "initialized";
   }
 }
 
 const FailedDatatypesHandler& ProfileSyncService::failed_datatypes_handler() {
   return failed_datatypes_handler_;
 }