Remove unused AccountTracker::Observer methods

As part of the move toward eliminating AccountTracker entirely (see bug),
this CL removes two AccountTracker::Observer methods that are
essentially unused in the codebase (their one production implementation
simply uses them to print logging messages, which could happen just as
easily in AccountTracker itself if needed).

The third method (OnSignInChanged()) is actually used. Eliminating this
method will likely require porting its consumers to observe the
OAuth2TokenService instead.

TBR=rockot@chromium.org

Bug: 729590
Change-Id: I2300f83e8f402ac54783eae212efc8debe598e0b
Reviewed-on: https://chromium-review.googlesource.com/538618Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Nicolas Zea <zea@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481852}

chrome/browser/extensions/api/identity/identity_api.cc

@@ -189,12 +189,6 @@ BrowserContextKeyedAPIFactory<IdentityAPI>* IdentityAPI::GetFactoryInstance() {
   return g_factory.Pointer();
 }
 
-void IdentityAPI::OnAccountAdded(const gaia::AccountIds& ids) {
-}
-
-void IdentityAPI::OnAccountRemoved(const gaia::AccountIds& ids) {
-}
-
 void IdentityAPI::OnAccountSignInChanged(const gaia::AccountIds& ids,
                                          bool is_signed_in) {
   api::identity::AccountInfo account_info;

chrome/browser/extensions/api/identity/identity_api.h

@@ -101,8 +101,6 @@ class IdentityAPI : public BrowserContextKeyedAPI,
   static BrowserContextKeyedAPIFactory<IdentityAPI>* GetFactoryInstance();
 
   // gaia::AccountTracker::Observer implementation:
-  void OnAccountAdded(const gaia::AccountIds& ids) override;
-  void OnAccountRemoved(const gaia::AccountIds& ids) override;
   void OnAccountSignInChanged(const gaia::AccountIds& ids,
                               bool is_signed_in) override;
 

components/gcm_driver/gcm_account_tracker.cc

@@ -101,17 +101,6 @@ void GCMAccountTracker::ScheduleReportTokens() {
       GetTimeToNextTokenReporting());
 }
 
-void GCMAccountTracker::OnAccountAdded(const gaia::AccountIds& ids) {
-  DVLOG(1) << "Account added: " << ids.email;
-  // We listen for the account signing in, which happens after account is added.
-}
-
-void GCMAccountTracker::OnAccountRemoved(const gaia::AccountIds& ids) {
-  DVLOG(1) << "Account removed: " << ids.email;
-  // We listen for the account signing out, which happens before account is
-  // removed.
-}
-
 void GCMAccountTracker::OnAccountSignInChanged(const gaia::AccountIds& ids,
                                                bool is_signed_in) {
   if (is_signed_in)

components/gcm_driver/gcm_account_tracker.h

@@ -96,8 +96,6 @@ class GCMAccountTracker : public gaia::AccountTracker::Observer,
   typedef std::map<std::string, AccountInfo> AccountInfos;
 
   // AccountTracker::Observer overrides.
-  void OnAccountAdded(const gaia::AccountIds& ids) override;
-  void OnAccountRemoved(const gaia::AccountIds& ids) override;
   void OnAccountSignInChanged(const gaia::AccountIds& ids,
                               bool is_signed_in) override;
 

google_apis/gaia/account_tracker.cc

@@ -139,18 +139,6 @@ void AccountTracker::SetAccountStateForTest(AccountIds ids, bool is_signed_in) {
   }
 }
 
-void AccountTracker::NotifyAccountAdded(const AccountState& account) {
-  DCHECK(!account.ids.gaia.empty());
-  for (auto& observer : observer_list_)
-    observer.OnAccountAdded(account.ids);
-}
-
-void AccountTracker::NotifyAccountRemoved(const AccountState& account) {
-  DCHECK(!account.ids.gaia.empty());
-  for (auto& observer : observer_list_)
-    observer.OnAccountRemoved(account.ids);
-}
-
 void AccountTracker::NotifySignInChanged(const AccountState& account) {
   // TODO(robliao): Remove ScopedTracker below once https://crbug.com/422460 is
   // fixed.
@@ -207,7 +195,6 @@ void AccountTracker::StopTrackingAccount(const std::string account_key) {
     AccountState& account = accounts_[account_key];
     if (!account.ids.gaia.empty()) {
       UpdateSignInState(account_key, false);
-      NotifyAccountRemoved(account);
     }
     accounts_.erase(account_key);
   }
@@ -268,7 +255,6 @@ void AccountTracker::OnUserInfoFetchSuccess(AccountIdFetcher* fetcher,
   AccountState& account = accounts_[account_key];
 
   account.ids.gaia = gaia_id;
-  NotifyAccountAdded(account);
 
   if (account.is_signed_in)
     NotifySignInChanged(account);

google_apis/gaia/account_tracker.h

@@ -49,8 +49,6 @@ class AccountTracker : public OAuth2TokenService::Observer,
 
   class Observer {
    public:
-    virtual void OnAccountAdded(const AccountIds& ids) = 0;
-    virtual void OnAccountRemoved(const AccountIds& ids) = 0;
     virtual void OnAccountSignInChanged(const AccountIds& ids,
                                         bool is_signed_in) = 0;
   };
@@ -92,8 +90,6 @@ class AccountTracker : public OAuth2TokenService::Observer,
     bool is_signed_in;
   };
 
-  void NotifyAccountAdded(const AccountState& account);
-  void NotifyAccountRemoved(const AccountState& account);
   void NotifySignInChanged(const AccountState& account);
 
   void UpdateSignInState(const std::string& account_key, bool is_signed_in);

google_apis/gaia/account_tracker_unittest.cc

@@ -23,8 +23,6 @@ namespace {
 const char kPrimaryAccountKey[] = "primary_account@example.com";
 
 enum TrackingEventType {
-  ADDED,
-  REMOVED,
   SIGN_IN,
   SIGN_OUT
 };
@@ -56,12 +54,6 @@ class TrackingEvent {
   std::string ToString() const {
     const char * typestr = "INVALID";
     switch (type_) {
-      case ADDED:
-        typestr = "ADD";
-        break;
-      case REMOVED:
-        typestr = "REM";
-        break;
       case SIGN_IN:
         typestr = " IN";
         break;
@@ -136,8 +128,6 @@ class AccountTrackerObserver : public AccountTracker::Observer {
   void SortEventsByUser();
 
   // AccountTracker::Observer implementation
-  void OnAccountAdded(const AccountIds& ids) override;
-  void OnAccountRemoved(const AccountIds& ids) override;
   void OnAccountSignInChanged(const AccountIds& ids,
                               bool is_signed_in) override;
 
@@ -148,14 +138,6 @@ class AccountTrackerObserver : public AccountTracker::Observer {
   std::vector<TrackingEvent> events_;
 };
 
-void AccountTrackerObserver::OnAccountAdded(const AccountIds& ids) {
-  events_.push_back(TrackingEvent(ADDED, ids.email, ids.gaia));
-}
-
-void AccountTrackerObserver::OnAccountRemoved(const AccountIds& ids) {
-  events_.push_back(TrackingEvent(REMOVED, ids.email, ids.gaia));
-}
-
 void AccountTrackerObserver::OnAccountSignInChanged(const AccountIds& ids,
                                                     bool is_signed_in) {
   events_.push_back(
@@ -397,8 +379,7 @@ TEST_F(IdentityAccountTrackerTest, PrimaryLoginThenTokenAvailable) {
 
   ReturnOAuthUrlFetchSuccess(kPrimaryAccountKey);
   EXPECT_TRUE(
-      observer()->CheckEvents(TrackingEvent(ADDED, kPrimaryAccountKey),
-                              TrackingEvent(SIGN_IN, kPrimaryAccountKey)));
+      observer()->CheckEvents(TrackingEvent(SIGN_IN, kPrimaryAccountKey)));
 }
 
 TEST_F(IdentityAccountTrackerTest, PrimaryTokenAvailableThenLogin) {
@@ -408,8 +389,7 @@ TEST_F(IdentityAccountTrackerTest, PrimaryTokenAvailableThenLogin) {
   NotifyLogin(kPrimaryAccountKey);
   ReturnOAuthUrlFetchSuccess(kPrimaryAccountKey);
   EXPECT_TRUE(
-      observer()->CheckEvents(TrackingEvent(ADDED, kPrimaryAccountKey),
-                              TrackingEvent(SIGN_IN, kPrimaryAccountKey)));
+      observer()->CheckEvents(TrackingEvent(SIGN_IN, kPrimaryAccountKey)));
 }
 
 TEST_F(IdentityAccountTrackerTest, PrimaryTokenAvailableAndRevokedThenLogin) {
@@ -419,11 +399,10 @@ TEST_F(IdentityAccountTrackerTest, PrimaryTokenAvailableAndRevokedThenLogin) {
   NotifyLogin(kPrimaryAccountKey);
   ReturnOAuthUrlFetchSuccess(kPrimaryAccountKey);
   EXPECT_TRUE(
-      observer()->CheckEvents(TrackingEvent(ADDED, kPrimaryAccountKey),
-                              TrackingEvent(SIGN_IN, kPrimaryAccountKey)));
+      observer()->CheckEvents(TrackingEvent(SIGN_IN, kPrimaryAccountKey)));
 }
 
-TEST_F(IdentityAccountTrackerTest, PrimaryRevokeThenLogout) {
+TEST_F(IdentityAccountTrackerTest, PrimaryRevoke) {
   NotifyLogin(kPrimaryAccountKey);
   NotifyTokenAvailable(kPrimaryAccountKey);
   ReturnOAuthUrlFetchSuccess(kPrimaryAccountKey);
@@ -432,10 +411,6 @@ TEST_F(IdentityAccountTrackerTest, PrimaryRevokeThenLogout) {
   NotifyTokenRevoked(kPrimaryAccountKey);
   EXPECT_TRUE(
       observer()->CheckEvents(TrackingEvent(SIGN_OUT, kPrimaryAccountKey)));
-
-  NotifyLogout();
-  EXPECT_TRUE(
-      observer()->CheckEvents(TrackingEvent(REMOVED, kPrimaryAccountKey)));
 }
 
 TEST_F(IdentityAccountTrackerTest, PrimaryRevokeThenLogin) {
@@ -469,8 +444,7 @@ TEST_F(IdentityAccountTrackerTest, PrimaryLogoutThenRevoke) {
 
   NotifyLogout();
   EXPECT_TRUE(
-      observer()->CheckEvents(TrackingEvent(SIGN_OUT, kPrimaryAccountKey),
-                              TrackingEvent(REMOVED, kPrimaryAccountKey)));
+      observer()->CheckEvents(TrackingEvent(SIGN_OUT, kPrimaryAccountKey)));
 
   NotifyTokenRevoked(kPrimaryAccountKey);
   EXPECT_TRUE(observer()->CheckEvents());
@@ -487,7 +461,6 @@ TEST_F(IdentityAccountTrackerTest, PrimaryLogoutFetchCancelAvailable) {
   NotifyTokenAvailable(kPrimaryAccountKey);
   ReturnOAuthUrlFetchSuccess(kPrimaryAccountKey);
   EXPECT_TRUE(observer()->CheckEvents(
-      TrackingEvent(ADDED, kPrimaryAccountKey),
       TrackingEvent(SIGN_IN, kPrimaryAccountKey)));
 }
 
@@ -501,7 +474,6 @@ TEST_F(IdentityAccountTrackerTest, Available) {
 
   ReturnOAuthUrlFetchSuccess("user@example.com");
   EXPECT_TRUE(observer()->CheckEvents(
-      TrackingEvent(ADDED, "user@example.com"),
       TrackingEvent(SIGN_IN, "user@example.com")));
 }
 
@@ -519,7 +491,6 @@ TEST_F(IdentityAccountTrackerTest, AvailableRevokeAvailable) {
   ReturnOAuthUrlFetchSuccess("user@example.com");
   NotifyTokenRevoked("user@example.com");
   EXPECT_TRUE(observer()->CheckEvents(
-      TrackingEvent(ADDED, "user@example.com"),
       TrackingEvent(SIGN_IN, "user@example.com"),
       TrackingEvent(SIGN_OUT, "user@example.com")));
 
@@ -538,7 +509,6 @@ TEST_F(IdentityAccountTrackerTest, AvailableRevokeAvailableWithPendingFetch) {
   NotifyTokenAvailable("user@example.com");
   ReturnOAuthUrlFetchSuccess("user@example.com");
   EXPECT_TRUE(observer()->CheckEvents(
-      TrackingEvent(ADDED, "user@example.com"),
       TrackingEvent(SIGN_IN, "user@example.com")));
 }
 
@@ -549,7 +519,6 @@ TEST_F(IdentityAccountTrackerTest, AvailableRevokeRevoke) {
   ReturnOAuthUrlFetchSuccess("user@example.com");
   NotifyTokenRevoked("user@example.com");
   EXPECT_TRUE(observer()->CheckEvents(
-      TrackingEvent(ADDED, "user@example.com"),
       TrackingEvent(SIGN_IN, "user@example.com"),
       TrackingEvent(SIGN_OUT, "user@example.com")));
 
@@ -563,7 +532,6 @@ TEST_F(IdentityAccountTrackerTest, AvailableAvailable) {
   NotifyTokenAvailable("user@example.com");
   ReturnOAuthUrlFetchSuccess("user@example.com");
   EXPECT_TRUE(observer()->CheckEvents(
-      TrackingEvent(ADDED, "user@example.com"),
       TrackingEvent(SIGN_IN, "user@example.com")));
 
   NotifyTokenAvailable("user@example.com");
@@ -576,13 +544,11 @@ TEST_F(IdentityAccountTrackerTest, TwoAccounts) {
   NotifyTokenAvailable("alpha@example.com");
   ReturnOAuthUrlFetchSuccess("alpha@example.com");
   EXPECT_TRUE(observer()->CheckEvents(
-      TrackingEvent(ADDED, "alpha@example.com"),
       TrackingEvent(SIGN_IN, "alpha@example.com")));
 
   NotifyTokenAvailable("beta@example.com");
   ReturnOAuthUrlFetchSuccess("beta@example.com");
   EXPECT_TRUE(observer()->CheckEvents(
-      TrackingEvent(ADDED, "beta@example.com"),
       TrackingEvent(SIGN_IN, "beta@example.com")));
 
   NotifyTokenRevoked("alpha@example.com");
@@ -604,7 +570,6 @@ TEST_F(IdentityAccountTrackerTest, AvailableTokenFetchFailAvailable) {
   NotifyTokenAvailable("user@example.com");
   ReturnOAuthUrlFetchSuccess("user@example.com");
   EXPECT_TRUE(observer()->CheckEvents(
-      TrackingEvent(ADDED, "user@example.com"),
       TrackingEvent(SIGN_IN, "user@example.com")));
 }
 
@@ -618,20 +583,15 @@ TEST_F(IdentityAccountTrackerTest, MultiSignOutSignIn) {
 
   observer()->SortEventsByUser();
   EXPECT_TRUE(observer()->CheckEvents(
-      TrackingEvent(ADDED, "alpha@example.com"),
       TrackingEvent(SIGN_IN, "alpha@example.com"),
-      TrackingEvent(ADDED, "beta@example.com"),
       TrackingEvent(SIGN_IN, "beta@example.com")));
 
   NotifyLogout();
   observer()->SortEventsByUser();
-  EXPECT_TRUE(observer()->CheckEvents(
-      TrackingEvent(SIGN_OUT, "alpha@example.com"),
-      TrackingEvent(REMOVED, "alpha@example.com"),
-      TrackingEvent(SIGN_OUT, "beta@example.com"),
-      TrackingEvent(REMOVED, "beta@example.com"),
-      TrackingEvent(SIGN_OUT, kPrimaryAccountKey),
-      TrackingEvent(REMOVED, kPrimaryAccountKey)));
+  EXPECT_TRUE(
+      observer()->CheckEvents(TrackingEvent(SIGN_OUT, "alpha@example.com"),
+                              TrackingEvent(SIGN_OUT, "beta@example.com"),
+                              TrackingEvent(SIGN_OUT, kPrimaryAccountKey)));
 
   // No events fire at all while profile is signed out.
   NotifyTokenRevoked("alpha@example.com");
@@ -646,11 +606,8 @@ TEST_F(IdentityAccountTrackerTest, MultiSignOutSignIn) {
   ReturnOAuthUrlFetchSuccess(kPrimaryAccountKey);
   observer()->SortEventsByUser();
   EXPECT_TRUE(observer()->CheckEvents(
-      TrackingEvent(ADDED, "beta@example.com"),
       TrackingEvent(SIGN_IN, "beta@example.com"),
-      TrackingEvent(ADDED, "gamma@example.com"),
       TrackingEvent(SIGN_IN, "gamma@example.com"),
-      TrackingEvent(ADDED, kPrimaryAccountKey),
       TrackingEvent(SIGN_IN, kPrimaryAccountKey)));
 
   // Revoking the primary token does not affect other accounts.
@@ -686,9 +643,7 @@ TEST_F(IdentityAccountTrackerTest, MultiLogoutRemovesAllAccounts) {
   observer()->SortEventsByUser();
   EXPECT_TRUE(
       observer()->CheckEvents(TrackingEvent(SIGN_OUT, kPrimaryAccountKey),
-                              TrackingEvent(REMOVED, kPrimaryAccountKey),
-                              TrackingEvent(SIGN_OUT, "user@example.com"),
-                              TrackingEvent(REMOVED, "user@example.com")));
+                              TrackingEvent(SIGN_OUT, "user@example.com")));
 }
 
 TEST_F(IdentityAccountTrackerTest, MultiRevokePrimaryDoesNotRemoveAllAccounts) {