Fix DCHECK when upgrading from an old profile.

components/signin/core/browser/signin_manager_base.cc

 // Copyright 2014 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 "components/signin/core/browser/signin_manager_base.h"
 
 #include <string>
 #include <vector>
 
 #include "base/command_line.h"
@@ skipping 44 matching lines @@
       client_->GetPrefs()->GetString(prefs::kGoogleServicesAccountId);
 
   // Handle backward compatibility: if kGoogleServicesAccountId is empty, but
   // kGoogleServicesUsername is not, then this is an old profile that needs to
   // be updated.  kGoogleServicesUserAccountId should not be empty, and contains
   // the gaia_id.  Use both properties to prime the account tracker before
   // proceeding.
   if (account_id.empty()) {
     std::string pref_account_username =
         client_->GetPrefs()->GetString(prefs::kGoogleServicesUsername);
-    std::string pref_gaia_id =
-        client_->GetPrefs()->GetString(prefs::kGoogleServicesUserAccountId);
+    if (!pref_account_username.empty()) {
+      // This is an old profile connected to a google account.  Migrate from
+      // kGoogleServicesUsername to kGoogleServicesAccountId.
+      std::string pref_gaia_id =
+          client_->GetPrefs()->GetString(prefs::kGoogleServicesUserAccountId);
 
-    // If kGoogleServicesUserAccountId is empty, then this is either a chromeos
-    // machine or a really old profile on one of the other platforms.  However
-    // in this case the account tracker should have the gaia_id so fetch it
-    // from there.
-    if (!pref_account_username.empty() && pref_gaia_id.empty()) {
-      AccountTrackerService::AccountInfo info =
-          account_tracker_service_->GetAccountInfo(pref_account_username);
-      DCHECK(!info.gaia.empty());
-      pref_gaia_id = info.gaia;
-    }
+      // If kGoogleServicesUserAccountId is empty, then this is either a cros
+      // machine or a really old profile on one of the other platforms.  However
+      // in this case the account tracker should have the gaia_id so fetch it
+      // from there.
+      if (pref_gaia_id.empty()) {
+        AccountTrackerService::AccountInfo info =
+            account_tracker_service_->GetAccountInfo(pref_account_username);
+        pref_gaia_id = info.gaia;
+      }
 
-    if (!pref_account_username.empty() && !pref_gaia_id.empty()) {
+      // If |pref_gaia_id| is still empty, this means the profile has been in
+      // an auth error state for some time (since M39).  It could also mean
+      // a profile that has not been used since M33.  Before migration to gaia
+      // id is complete, the returned value will be the normalized email, which
+      // is correct.  After the migration, the returned value will be empty,
+      // which means the user is essentially signed out.
+      // TODO(rogerta): may want to show a toast or something.
       account_id = account_tracker_service_->SeedAccountInfo(
           pref_gaia_id, pref_account_username);
 
       // Now remove obsolete preferences.
       client_->GetPrefs()->ClearPref(prefs::kGoogleServicesUsername);
+    }
 
-      // TODO(rogerta): once migration to gaia id is complete, remove
-      // kGoogleServicesUserAccountId and change all uses of that pref to
-      // kGoogleServicesAccountId.
-    }
+    // TODO(rogerta): once migration to gaia id is complete, remove
+    // kGoogleServicesUserAccountId and change all uses of that pref to
+    // kGoogleServicesAccountId.
   }
 
   if (!account_id.empty())
     SetAuthenticatedAccountId(account_id);
 }
 
 bool SigninManagerBase::IsInitialized() const { return initialized_; }
 
 bool SigninManagerBase::IsSigninAllowed() const {
   return client_->GetPrefs()->GetBoolean(prefs::kSigninAllowed);
 }
 
 std::string SigninManagerBase::GetAuthenticatedUsername() const {
   return account_tracker_service_->GetAccountInfo(
       GetAuthenticatedAccountId()).email;
 }
 
 const std::string& SigninManagerBase::GetAuthenticatedAccountId() const {
   return authenticated_account_id_;
 }
 
 void SigninManagerBase::SetAuthenticatedAccountInfo(const std::string& gaia_id,
                                                     const std::string& email) {
+  DCHECK(!gaia_id.empty());
+  DCHECK(!email.empty());
+
   std::string account_id =
       account_tracker_service_->SeedAccountInfo(gaia_id, email);

[Mike Lerman, 2015/04/24 12:43:09]

Since we're making the assumption now that the ATS always is seeded with an
email, can you add a DCHECK to SeedAccountInfo and perhaps to LoadFromPrefs that
validates we always have an email?

[Roger Tawa OOO till Jul 10th, 2015/04/24 13:00:33]

There is a dcheck in seed by virtue of the call to pick, see function.

I don't think a dcheck in load is necessary, since the existing dcheck in seed
already guarantees that we always seed with an email.

   SetAuthenticatedAccountId(account_id);
 }
 
 void SigninManagerBase::SetAuthenticatedAccountId(
     const std::string& account_id) {
   DCHECK(!account_id.empty());
   if (!authenticated_account_id_.empty()) {
     DLOG_IF(ERROR, account_id != authenticated_account_id_)
         << "Tried to change the authenticated id to something different: "
         << "Current: " << authenticated_account_id_ << ", New: " << account_id;
     return;
   }
 
   std::string pref_account_id =
       client_->GetPrefs()->GetString(prefs::kGoogleServicesAccountId);
 
   DCHECK(pref_account_id.empty() || pref_account_id == account_id)
       << "account_id=" << account_id
       << " pref_account_id=" << pref_account_id;
   authenticated_account_id_ = account_id;
   client_->GetPrefs()->SetString(prefs::kGoogleServicesAccountId, account_id);
 
   // This preference is set so that code on I/O thread has access to the
   // Gaia id of the signed in user.
   AccountTrackerService::AccountInfo info =
       account_tracker_service_->GetAccountInfo(account_id);
-  DCHECK(!info.gaia.empty());
-  client_->GetPrefs()->SetString(prefs::kGoogleServicesUserAccountId,
-                                 info.gaia);
+
+  // When this function is called from Initialize(), it's possible for
+  // |info.gaia| to be empty when migrating from a really old profile.
+  if (!info.gaia.empty()) {
+    client_->GetPrefs()->SetString(prefs::kGoogleServicesUserAccountId,
+                                   info.gaia);
+  }
 
   // Go ahead and update the last signed in account info here as well. Once a
   // user is signed in the two preferences should match. Doing it here as
   // opposed to on signin allows us to catch the upgrade scenario.
   client_->GetPrefs()->SetString(prefs::kGoogleServicesLastUsername,
                                  info.email);
 }
 
 bool SigninManagerBase::IsAuthenticated() const {
   return !authenticated_account_id_.empty();
@@ skipping 24 matching lines @@
   signin_diagnostics_observers_.RemoveObserver(observer);
 }
 
 void SigninManagerBase::NotifyDiagnosticsObservers(
     const TimedSigninStatusField& field,
     const std::string& value) {
   FOR_EACH_OBSERVER(SigninDiagnosticsObserver,
                     signin_diagnostics_observers_,
                     NotifySigninValueChanged(field, value));
 }