Add account id/gaia id to sign-in internals page.

BUG=405038

Review URL: https://codereview.chromium.org/956363002

Cr-Commit-Position: refs/heads/master@{#320907}

chrome/browser/signin/about_signin_internals_factory.cc

@@ -6,6 +6,7 @@
 
 #include "base/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/signin/account_tracker_service_factory.h"
 #include "chrome/browser/signin/chrome_signin_client_factory.h"
 #include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
@@ -23,9 +24,10 @@ AboutSigninInternalsFactory::AboutSigninInternalsFactory()
     : BrowserContextKeyedServiceFactory(
         "AboutSigninInternals",
         BrowserContextDependencyManager::GetInstance()) {
-  DependsOn(SigninManagerFactory::GetInstance());
-  DependsOn(ProfileOAuth2TokenServiceFactory::GetInstance());
+  DependsOn(AccountTrackerServiceFactory::GetInstance());
   DependsOn(ChromeSigninClientFactory::GetInstance());
+  DependsOn(ProfileOAuth2TokenServiceFactory::GetInstance());
+  DependsOn(SigninManagerFactory::GetInstance());
 }
 
 AboutSigninInternalsFactory::~AboutSigninInternalsFactory() {}
@@ -45,6 +47,10 @@ AboutSigninInternalsFactory* AboutSigninInternalsFactory::GetInstance() {
 void AboutSigninInternalsFactory::RegisterProfilePrefs(
     user_prefs::PrefRegistrySyncable* user_prefs) {
   // SigninManager information for about:signin-internals.
+
+  // TODO(rogerta): leaving untimed fields here for now because legacy
+  // profiles still have these prefs.  In three or four version from M43
+  // we can probably remove them.
   for (int i = UNTIMED_FIELDS_BEGIN; i < UNTIMED_FIELDS_END; ++i) {
     const std::string pref_path = SigninStatusFieldToString(
         static_cast<UntimedSigninStatusField>(i));
@@ -53,6 +59,7 @@ void AboutSigninInternalsFactory::RegisterProfilePrefs(
         std::string(),
         user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
   }
+
   for (int i = TIMED_FIELDS_BEGIN; i < TIMED_FIELDS_END; ++i) {
     const std::string value = SigninStatusFieldToString(
         static_cast<TimedSigninStatusField>(i)) + ".value";
@@ -74,6 +81,7 @@ KeyedService* AboutSigninInternalsFactory::BuildServiceInstanceFor(
   Profile* profile = Profile::FromBrowserContext(context);
   AboutSigninInternals* service = new AboutSigninInternals(
       ProfileOAuth2TokenServiceFactory::GetForProfile(profile),
+      AccountTrackerServiceFactory::GetForProfile(profile),
       SigninManagerFactory::GetForProfile(profile));
   service->Initialize(ChromeSigninClientFactory::GetForProfile(profile));
   return service;

chrome/browser/ui/webui/signin/inline_login_handler_impl.cc

@@ -694,8 +694,7 @@ void InlineLoginHandlerImpl::CompleteLogin(const base::ListValue* args) {
 
   AboutSigninInternals* about_signin_internals =
       AboutSigninInternalsFactory::GetForProfile(Profile::FromWebUI(web_ui()));
-  about_signin_internals->OnAuthenticationResultReceived(
-      "GAIA Auth Successful");
+  about_signin_internals->OnAuthenticationResultReceived("Successful");
 
   content::StoragePartition* partition =
       content::BrowserContext::GetStoragePartitionForSite(

components/signin/core/browser/about_signin_internals.cc

@@ -13,6 +13,7 @@
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/trace_event/trace_event.h"
+#include "components/signin/core/browser/account_tracker_service.h"
 #include "components/signin/core/browser/profile_oauth2_token_service.h"
 #include "components/signin/core/browser/signin_client.h"
 #include "components/signin/core/browser/signin_internals_util.h"
@@ -67,8 +68,12 @@ void AddCookieEntry(base::ListValue* accounts_list,
 
 std::string SigninStatusFieldToLabel(UntimedSigninStatusField field) {
   switch (field) {
+    case ACCOUNT_ID:
+      return "Account Id";
+    case GAIA_ID:
+      return "Gaia Id";
     case USERNAME:
-      return "User Id";
+      return "Username";
     case UNTIMED_FIELDS_END:
       NOTREACHED();
       return std::string();
@@ -80,18 +85,14 @@ std::string SigninStatusFieldToLabel(UntimedSigninStatusField field) {
 #if !defined (OS_CHROMEOS)
 std::string SigninStatusFieldToLabel(TimedSigninStatusField field) {
   switch (field) {
-    case SIGNIN_TYPE:
-      return "Type";
     case AUTHENTICATION_RESULT_RECEIVED:
-      return "Last Authentication Result Received";
+      return "Gaia Authentication Result";
     case REFRESH_TOKEN_RECEIVED:
-      return "Last RefreshToken Received";
-    case GET_USER_INFO_STATUS:
-      return "Last OnGetUserInfo Received";
-    case UBER_TOKEN_STATUS:
-      return "Last OnUberToken Received";
-    case MERGE_SESSION_STATUS:
-      return "Last OnMergeSession Received";
+      return "RefreshToken Received";
+    case SIGNIN_STARTED:
+      return "SigninManager Started";
+    case SIGNIN_COMPLETED:
+      return "SigninManager Completed";
     case TIMED_FIELDS_END:
       NOTREACHED();
       return "Error";
@@ -101,12 +102,41 @@ std::string SigninStatusFieldToLabel(TimedSigninStatusField field) {
 }
 #endif // !defined (OS_CHROMEOS)
 
+void SetPref(PrefService* prefs,
+             TimedSigninStatusField field,
+             const std::string& time,
+             const std::string& value) {
+  std::string value_pref = SigninStatusFieldToString(field) + ".value";
+  std::string time_pref = SigninStatusFieldToString(field) + ".time";
+  prefs->SetString(value_pref, value);
+  prefs->SetString(time_pref, time);
+}
+
+void GetPref(PrefService* prefs,
+             TimedSigninStatusField field,
+             std::string* time,
+             std::string* value) {
+  std::string value_pref = SigninStatusFieldToString(field) + ".value";
+  std::string time_pref = SigninStatusFieldToString(field) + ".time";
+  *value = prefs->GetString(value_pref);
+  *time = prefs->GetString(time_pref);
+}
+
+void ClearPref(PrefService* prefs, TimedSigninStatusField field) {
+  std::string value_pref = SigninStatusFieldToString(field) + ".value";
+  std::string time_pref = SigninStatusFieldToString(field) + ".time";
+  prefs->ClearPref(value_pref);
+  prefs->ClearPref(time_pref);
+}
+
 }  // anonymous namespace
 
 AboutSigninInternals::AboutSigninInternals(
     ProfileOAuth2TokenService* token_service,
+    AccountTrackerService* account_tracker,
     SigninManagerBase* signin_manager)
     : token_service_(token_service),
+      account_tracker_(account_tracker),
       signin_manager_(signin_manager),
       client_(NULL) {}
 
@@ -122,22 +152,6 @@ void AboutSigninInternals::RemoveSigninObserver(
   signin_observers_.RemoveObserver(observer);
 }
 
-void AboutSigninInternals::NotifySigninValueChanged(
-    const UntimedSigninStatusField& field,
-    const std::string& value) {
-  unsigned int field_index = field - UNTIMED_FIELDS_BEGIN;
-  DCHECK(field_index >= 0 &&
-         field_index < signin_status_.untimed_signin_fields.size());
-
-  signin_status_.untimed_signin_fields[field_index] = value;
-
-  // Also persist these values in the prefs.
-  const std::string pref_path = SigninStatusFieldToString(field);
-  client_->GetPrefs()->SetString(pref_path.c_str(), value);
-
-  NotifyObservers();
-}
-
 void AboutSigninInternals::NotifySigninValueChanged(
     const TimedSigninStatusField& field,
     const std::string& value) {
@@ -153,46 +167,31 @@ void AboutSigninInternals::NotifySigninValueChanged(
   signin_status_.timed_signin_fields[field_index] = timed_value;
 
   // Also persist these values in the prefs.
-  const std::string value_pref = SigninStatusFieldToString(field) + ".value";
-  const std::string time_pref = SigninStatusFieldToString(field) + ".time";
-  client_->GetPrefs()->SetString(value_pref.c_str(), value);
-  client_->GetPrefs()->SetString(time_pref.c_str(), time_as_str);
+  SetPref(client_->GetPrefs(), field, value, time_as_str);
+
+  // If the user is restarting a sign in process, clear the fields that are
+  // to come.
+  if (field == AUTHENTICATION_RESULT_RECEIVED) {
+    ClearPref(client_->GetPrefs(), REFRESH_TOKEN_RECEIVED);
+    ClearPref(client_->GetPrefs(), SIGNIN_STARTED);
+    ClearPref(client_->GetPrefs(), SIGNIN_COMPLETED);
+  }
 
   NotifyObservers();
 }
 
 void AboutSigninInternals::RefreshSigninPrefs() {
-  // Since the AboutSigninInternals has a dependency on the SigninManager
-  // (as seen in the AboutSigninInternalsFactory) the SigninManager can have
-  // the AuthenticatedUsername set before AboutSigninInternals can observe it.
-  // For that scenario, read the AuthenticatedUsername if it exists.
-  if (signin_manager_->IsAuthenticated()) {
-    signin_status_.untimed_signin_fields[USERNAME] =
-        signin_manager_->GetAuthenticatedUsername();
-  }
-
   // Return if no client exists. Can occur in unit tests.
   if (!client_)
     return;
 
   PrefService* pref_service = client_->GetPrefs();
-  for (int i = UNTIMED_FIELDS_BEGIN; i < UNTIMED_FIELDS_END; ++i) {
-    const std::string pref_path =
-        SigninStatusFieldToString(static_cast<UntimedSigninStatusField>(i));
-
-    signin_status_.untimed_signin_fields[i - UNTIMED_FIELDS_BEGIN] =
-        pref_service->GetString(pref_path.c_str());
-  }
   for (int i = TIMED_FIELDS_BEGIN; i < TIMED_FIELDS_END; ++i) {
-    const std::string value_pref =
-        SigninStatusFieldToString(static_cast<TimedSigninStatusField>(i)) +
-        ".value";
-    const std::string time_pref =
-        SigninStatusFieldToString(static_cast<TimedSigninStatusField>(i)) +
-        ".time";
-
-    TimedSigninStatusValue value(pref_service->GetString(value_pref.c_str()),
-                                 pref_service->GetString(time_pref.c_str()));
+    std::string time_str;
+    std::string value_str;
+    GetPref(pref_service, static_cast<TimedSigninStatusField>(i),
+            &time_str, &value_str);
+    TimedSigninStatusValue value(value_str, time_str);
     signin_status_.timed_signin_fields[i - TIMED_FIELDS_BEGIN] = value;
   }
 
@@ -241,7 +240,8 @@ void AboutSigninInternals::NotifyObservers() {
           "422460 AboutSigninInternals::NotifyObservers 0.5"));
 
   scoped_ptr<base::DictionaryValue> signin_status_value =
-      signin_status_.ToValue(product_version);
+      signin_status_.ToValue(account_tracker_, signin_manager_,
+                             product_version);
 
   // TODO(robliao): Remove ScopedTracker below once https://crbug.com/422460 is
   // fixed.
@@ -255,7 +255,8 @@ void AboutSigninInternals::NotifyObservers() {
 }
 
 scoped_ptr<base::DictionaryValue> AboutSigninInternals::GetSigninStatus() {
-  return signin_status_.ToValue(client_->GetProductVersion()).Pass();
+  return signin_status_.ToValue(account_tracker_, signin_manager_,
+                                client_->GetProductVersion()).Pass();
 }
 
 void AboutSigninInternals::OnAccessTokenRequested(
@@ -356,11 +357,27 @@ void AboutSigninInternals::OnListAccountsFailure(
   VLOG(1) << "AboutSigninInternals::OnListAccountsFailure:" << error.ToString();
 }
 
+void AboutSigninInternals::GoogleSigninFailed(
+    const GoogleServiceAuthError& error) {
+  NotifyObservers();
+}
+
+void AboutSigninInternals::GoogleSigninSucceeded(const std::string& account_id,
+                                                 const std::string& username,
+                                                 const std::string& password) {
+  NotifyObservers();
+}
+
+void AboutSigninInternals::GoogleSignedOut(const std::string& account_id,
+                                           const std::string& username) {
+  NotifyObservers();
+}
+
 void AboutSigninInternals::OnListAccountsComplete(
     std::vector<std::pair<std::string, bool> >& gaia_accounts) {
-  base::DictionaryValue signin_status;
+  base::DictionaryValue cookie_status;
   base::ListValue* cookie_info = new base::ListValue();
-  signin_status.Set("cookie_info", cookie_info);
+  cookie_status.Set("cookie_info", cookie_info);
 
   for (size_t i = 0; i < gaia_accounts.size(); ++i) {
     AddCookieEntry(cookie_info,
@@ -374,7 +391,7 @@ void AboutSigninInternals::OnListAccountsComplete(
   // Update the observers that the cookie's accounts are updated.
   FOR_EACH_OBSERVER(AboutSigninInternals::Observer,
                     signin_observers_,
-                    OnCookieAccountsFetched(&signin_status));
+                    OnCookieAccountsFetched(&cookie_status));
 }
 
 AboutSigninInternals::TokenInfo::TokenInfo(
@@ -437,8 +454,7 @@ base::DictionaryValue* AboutSigninInternals::TokenInfo::ToValue() const {
 }
 
 AboutSigninInternals::SigninStatus::SigninStatus()
-    : untimed_signin_fields(UNTIMED_FIELDS_COUNT),
-      timed_signin_fields(TIMED_FIELDS_COUNT) {}
+    : timed_signin_fields(TIMED_FIELDS_COUNT) {}
 
 AboutSigninInternals::SigninStatus::~SigninStatus() {
   for (TokenInfoMap::iterator it = token_info_map.begin();
@@ -461,7 +477,9 @@ AboutSigninInternals::TokenInfo* AboutSigninInternals::SigninStatus::FindToken(
 }
 
 scoped_ptr<base::DictionaryValue> AboutSigninInternals::SigninStatus::ToValue(
-    std::string product_version) {
+    AccountTrackerService* account_tracker,
+    SigninManagerBase* signin_manager,
+    const std::string& product_version) {
   // TODO(robliao): Remove ScopedTracker below once https://crbug.com/422460 is
   // fixed.
   tracked_objects::ScopedTracker tracking_profile1(
@@ -474,20 +492,17 @@ scoped_ptr<base::DictionaryValue> AboutSigninInternals::SigninStatus::ToValue(
 
   // A summary of signin related info first.
   base::ListValue* basic_info = AddSection(signin_info, "Basic Information");
-  const std::string signin_status_string =
-      untimed_signin_fields[USERNAME - UNTIMED_FIELDS_BEGIN].empty()
-          ? "Not Signed In"
-          : "Signed In";
   AddSectionEntry(basic_info, "Chrome Version", product_version);
-  AddSectionEntry(basic_info, "Signin Status", signin_status_string);
-  AddSectionEntry(basic_info, "Webview Based Signin Enabled?",
-      switches::IsEnableWebviewBasedSignin() == true ? "True" : "False");
-  AddSectionEntry(basic_info, "New Avatar Menu Enabled?",
-      switches::IsNewAvatarMenu() == true ? "True" : "False");
-  AddSectionEntry(basic_info, "New Profile Management Enabled?",
-      switches::IsNewProfileManagement() == true ? "True" : "False");
-  AddSectionEntry(basic_info, "Account Consistency Enabled?",
-      switches::IsEnableAccountConsistency() == true ? "True" : "False");
+  AddSectionEntry(basic_info, "Webview Based Signin?",
+      switches::IsEnableWebviewBasedSignin() == true ? "On" : "Off");
+  AddSectionEntry(basic_info, "New Avatar Menu?",
+      switches::IsNewAvatarMenu() == true ? "On" : "Off");
+  AddSectionEntry(basic_info, "New Profile Management?",
+      switches::IsNewProfileManagement() == true ? "On" : "Off");
+  AddSectionEntry(basic_info, "Account Consistency?",
+      switches::IsEnableAccountConsistency() == true ? "On" : "Off");
+  AddSectionEntry(basic_info, "Signin Status",
+      signin_manager->IsAuthenticated() ? "Signed In" : "Not Signed In");
 
   // TODO(robliao): Remove ScopedTracker below once https://crbug.com/422460 is
   // fixed.
@@ -495,12 +510,21 @@ scoped_ptr<base::DictionaryValue> AboutSigninInternals::SigninStatus::ToValue(
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "422460 AboutSigninInternals::SigninStatus::ToValue2"));
 
-  // Only add username.  SID and LSID have moved to tokens section.
-  const std::string field =
-      SigninStatusFieldToLabel(static_cast<UntimedSigninStatusField>(USERNAME));
-  AddSectionEntry(basic_info,
-                  field,
-                  untimed_signin_fields[USERNAME - UNTIMED_FIELDS_BEGIN]);
+  if (signin_manager->IsAuthenticated()) {
+    std::string account_id = signin_manager->GetAuthenticatedAccountId();
+    AddSectionEntry(basic_info,
+                    SigninStatusFieldToLabel(
+                        static_cast<UntimedSigninStatusField>(ACCOUNT_ID)),
+                    account_id);
+    AddSectionEntry(basic_info,
+                    SigninStatusFieldToLabel(
+                        static_cast<UntimedSigninStatusField>(GAIA_ID)),
+                    account_tracker->GetAccountInfo(account_id).gaia);
+    AddSectionEntry(basic_info,
+                    SigninStatusFieldToLabel(
+                        static_cast<UntimedSigninStatusField>(USERNAME)),
+                    signin_manager->GetAuthenticatedUsername());
+  }
 
 #if !defined(OS_CHROMEOS)
   // TODO(robliao): Remove ScopedTracker below once https://crbug.com/422460 is

components/signin/core/browser/about_signin_internals.h

@@ -19,10 +19,10 @@
 #include "google_apis/gaia/gaia_auth_consumer.h"
 #include "google_apis/gaia/oauth2_token_service.h"
 
+class AccountTrackerService;
 class GaiaAuthFetcher;
 class ProfileOAuth2TokenService;
 class SigninClient;
-class SigninManagerBase;
 
 // Many values in SigninStatus are also associated with a timestamp.
 // This makes it easier to keep values and their associated times together.
@@ -34,7 +34,8 @@ class AboutSigninInternals
     : public KeyedService,
       public signin_internals_util::SigninDiagnosticsObserver,
       public OAuth2TokenService::DiagnosticsObserver,
-      public GaiaAuthConsumer {
+      public GaiaAuthConsumer,
+      SigninManagerBase::Observer {
  public:
   class Observer {
    public:
@@ -47,6 +48,7 @@ class AboutSigninInternals
   };
 
   AboutSigninInternals(ProfileOAuth2TokenService* token_service,
+                       AccountTrackerService* account_tracker,
                        SigninManagerBase* signin_manager);
   ~AboutSigninInternals() override;
 
@@ -58,17 +60,11 @@ class AboutSigninInternals
   // Pulls all signin values that have been persisted in the user prefs.
   void RefreshSigninPrefs();
 
-  // SigninManager::SigninDiagnosticsObserver implementation.
-  void NotifySigninValueChanged(
-      const signin_internals_util::UntimedSigninStatusField& field,
-      const std::string& value) override;
-
-  void NotifySigninValueChanged(
-      const signin_internals_util::TimedSigninStatusField& field,
-      const std::string& value) override;
-
   void Initialize(SigninClient* client);
 
+  void OnRefreshTokenReceived(std::string status);
+  void OnAuthenticationResultReceived(std::string status);
+
   // KeyedService implementation.
   void Shutdown() override;
 
@@ -92,22 +88,6 @@ class AboutSigninInternals
   // corresponding to the cookies residing on the current cookie jar.
   void GetCookieAccountsAsync();
 
-  // OAuth2TokenService::DiagnosticsObserver implementations.
-  void OnAccessTokenRequested(
-      const std::string& account_id,
-      const std::string& consumer_id,
-      const OAuth2TokenService::ScopeSet& scopes) override;
-  void OnFetchAccessTokenComplete(const std::string& account_id,
-                                  const std::string& consumer_id,
-                                  const OAuth2TokenService::ScopeSet& scopes,
-                                  GoogleServiceAuthError error,
-                                  base::Time expiration_time) override;
-  void OnTokenRemoved(const std::string& account_id,
-                      const OAuth2TokenService::ScopeSet& scopes) override;
-
-    void OnRefreshTokenReceived(std::string status);
-    void OnAuthenticationResultReceived(std::string status);
-
  private:
   // Encapsulates diagnostic information about tokens for different services.
   struct TokenInfo {
@@ -137,7 +117,6 @@ class AboutSigninInternals
   // by SigninInternals to maintain information that needs to be shown in
   // the about:signin-internals page.
   struct SigninStatus {
-    std::vector<std::string> untimed_signin_fields;
     std::vector<TimedSigninStatusValue> timed_signin_fields;
     TokenInfoMap token_info_map;
 
@@ -165,16 +144,44 @@ class AboutSigninInternals
     //                           "status" : request status} elems]
     //       }],
     //  }
-    scoped_ptr<base::DictionaryValue> ToValue(std::string product_version);
+    scoped_ptr<base::DictionaryValue> ToValue(
+        AccountTrackerService* account_tracker,
+        SigninManagerBase* signin_manager,
+        const std::string& product_version);
   };
 
-  void NotifyObservers();
+  // SigninManager::SigninDiagnosticsObserver implementation.
+  void NotifySigninValueChanged(
+      const signin_internals_util::TimedSigninStatusField& field,
+      const std::string& value) override;
 
+  // OAuth2TokenService::DiagnosticsObserver implementations.
+  void OnAccessTokenRequested(
+      const std::string& account_id,
+      const std::string& consumer_id,
+      const OAuth2TokenService::ScopeSet& scopes) override;
+  void OnFetchAccessTokenComplete(const std::string& account_id,
+                                  const std::string& consumer_id,
+                                  const OAuth2TokenService::ScopeSet& scopes,
+                                  GoogleServiceAuthError error,
+                                  base::Time expiration_time) override;
+  void OnTokenRemoved(const std::string& account_id,
+                      const OAuth2TokenService::ScopeSet& scopes) override;
 
-  // Overriden from GaiaAuthConsumer.
+  // GaiaAuthConsumer implementations.
   void OnListAccountsSuccess(const std::string& data) override;
   void OnListAccountsFailure(const GoogleServiceAuthError& error) override;
 
+  // SigninManagerBase::Observer implementations.
+  void GoogleSigninFailed(const GoogleServiceAuthError& error) override;
+  void GoogleSigninSucceeded(const std::string& account_id,
+                                     const std::string& username,
+                                     const std::string& password) override;
+  void GoogleSignedOut(const std::string& account_id,
+                               const std::string& username) override;
+
+  void NotifyObservers();
+
   // Callback for ListAccounts. Once the email addresses are fetched from GAIA,
   // they are pushed to the signin_internals_ui.
   void OnListAccountsComplete(
@@ -187,6 +194,9 @@ class AboutSigninInternals
   // Weak pointer to the token service.
   ProfileOAuth2TokenService* token_service_;
 
+  // Weak pointer to the account tracker.
+  AccountTrackerService* account_tracker_;
+
   // Weak pointer to the signin manager.
   SigninManagerBase* signin_manager_;
 

components/signin/core/browser/signin_internals_util.cc

@@ -21,6 +21,8 @@ const char kTokenPrefPrefix[] = "google.services.signin.tokens.";
 #define ENUM_CASE(x) case x: return (std::string(kSigninPrefPrefix) + #x)
 std::string SigninStatusFieldToString(UntimedSigninStatusField field) {
   switch (field) {
+    ENUM_CASE(ACCOUNT_ID);
+    ENUM_CASE(GAIA_ID);
     ENUM_CASE(USERNAME);
     case UNTIMED_FIELDS_END:
       NOTREACHED();
@@ -33,12 +35,10 @@ std::string SigninStatusFieldToString(UntimedSigninStatusField field) {
 
 std::string SigninStatusFieldToString(TimedSigninStatusField field) {
   switch (field) {
-    ENUM_CASE(SIGNIN_TYPE);
     ENUM_CASE(AUTHENTICATION_RESULT_RECEIVED);
     ENUM_CASE(REFRESH_TOKEN_RECEIVED);
-    ENUM_CASE(GET_USER_INFO_STATUS);
-    ENUM_CASE(UBER_TOKEN_STATUS);
-    ENUM_CASE(MERGE_SESSION_STATUS);
+    ENUM_CASE(SIGNIN_STARTED);
+    ENUM_CASE(SIGNIN_COMPLETED);
     case TIMED_FIELDS_END:
       NOTREACHED();
       return std::string();

components/signin/core/browser/signin_internals_util.h

@@ -27,7 +27,9 @@ enum {
 };
 
 enum UntimedSigninStatusField {
-  USERNAME = UNTIMED_FIELDS_BEGIN,
+  ACCOUNT_ID = UNTIMED_FIELDS_BEGIN,
+  GAIA_ID,
+  USERNAME,
   UNTIMED_FIELDS_END
 };
 
@@ -37,12 +39,10 @@ enum {
 };
 
 enum TimedSigninStatusField {
-  SIGNIN_TYPE = TIMED_FIELDS_BEGIN,
-  AUTHENTICATION_RESULT_RECEIVED,
+  AUTHENTICATION_RESULT_RECEIVED = TIMED_FIELDS_BEGIN,
   REFRESH_TOKEN_RECEIVED,
-  GET_USER_INFO_STATUS,
-  UBER_TOKEN_STATUS,
-  MERGE_SESSION_STATUS,
+  SIGNIN_STARTED,
+  SIGNIN_COMPLETED,
   TIMED_FIELDS_END
 };
 
@@ -65,8 +65,6 @@ std::string SigninStatusFieldToString(TimedSigninStatusField field);
 class SigninDiagnosticsObserver {
  public:
   // Credentials and signin related changes.
-  virtual void NotifySigninValueChanged(const UntimedSigninStatusField& field,
-                                        const std::string& value) {}
   virtual void NotifySigninValueChanged(const TimedSigninStatusField& field,
                                         const std::string& value) {}
   // OAuth tokens related changes.

components/signin/core/browser/signin_manager.cc

@@ -105,7 +105,7 @@ std::string SigninManager::SigninTypeToString(SigninManager::SigninType type) {
     case SIGNIN_TYPE_NONE:
       return "No Signin";
     case SIGNIN_TYPE_WITH_REFRESH_TOKEN:
-      return "Signin with refresh token";
+      return "With refresh token";
   }
 
   NOTREACHED();
@@ -137,7 +137,7 @@ bool SigninManager::PrepareForSignin(SigninType type,
   password_.assign(password);
   signin_manager_signed_in_ = false;
   user_info_fetched_by_account_tracker_ = false;
-  NotifyDiagnosticsObservers(SIGNIN_TYPE, SigninTypeToString(type));
+  NotifyDiagnosticsObservers(SIGNIN_STARTED, SigninTypeToString(type));
   return true;
 }
 
@@ -156,8 +156,6 @@ void SigninManager::StartSignInWithRefreshToken(
   temp_refresh_token_ = refresh_token;
   possibly_invalid_username_ = username;
 
-  NotifyDiagnosticsObservers(GET_USER_INFO_STATUS, "Successful");
-
   if (!callback.is_null() && !temp_refresh_token_.empty()) {
     callback.Run(temp_refresh_token_);
   } else {
@@ -229,9 +227,6 @@ void SigninManager::SignOut(
   client_->GetPrefs()->ClearPref(prefs::kSignedInTime);
   client_->OnSignedOut();
 
-  // Erase (now) stale information from AboutSigninInternals.
-  NotifyDiagnosticsObservers(USERNAME, "");
-
   // Determine the duration the user was logged in and log that to UMA.
   if (!signin_time.is_null()) {
     base::TimeDelta signed_in_duration = base::Time::Now() - signin_time;
@@ -389,6 +384,8 @@ void SigninManager::MergeSigninCredentialIntoCookieJar() {
 }
 
 void SigninManager::CompletePendingSignin() {
+  NotifyDiagnosticsObservers(SIGNIN_COMPLETED, "Successful");
+
   DCHECK(!possibly_invalid_username_.empty());
   OnSignedIn(possibly_invalid_username_);
 

components/signin/core/browser/signin_manager_base.cc

@@ -94,7 +94,6 @@ void SigninManagerBase::SetAuthenticatedUsername(const std::string& username) {
                                                      username,
                                                      username);
   client_->GetPrefs()->SetString(prefs::kGoogleServicesUsername, username);
-  NotifyDiagnosticsObservers(USERNAME, username);
 
   // Go ahead and update the last signed in username here as well. Once a
   // user is signed in the two preferences should match. Doing it here as
@@ -136,14 +135,6 @@ void SigninManagerBase::RemoveSigninDiagnosticsObserver(
   signin_diagnostics_observers_.RemoveObserver(observer);
 }
 
-void SigninManagerBase::NotifyDiagnosticsObservers(
-    const UntimedSigninStatusField& field,
-    const std::string& value) {
-  FOR_EACH_OBSERVER(SigninDiagnosticsObserver,
-                    signin_diagnostics_observers_,
-                    NotifySigninValueChanged(field, value));
-}
-
 void SigninManagerBase::NotifyDiagnosticsObservers(
     const TimedSigninStatusField& field,
     const std::string& value) {

components/signin/core/browser/signin_manager_base.h

@@ -131,10 +131,7 @@ class SigninManagerBase : public KeyedService {
   // Makes sure list is empty on destruction.
   ObserverList<Observer, true> observer_list_;
 
-  // Helper methods to notify all registered diagnostics observers with.
-  void NotifyDiagnosticsObservers(
-      const signin_internals_util::UntimedSigninStatusField& field,
-      const std::string& value);
+  // Helper method to notify all registered diagnostics observers with.
   void NotifyDiagnosticsObservers(
       const signin_internals_util::TimedSigninStatusField& field,
       const std::string& value);