[Tango->FCM] Separate status enum for the subscription channel.

Currently fcm channel and subscription to topics use same enum in order to track their
state. This Cl introduces separate enum for the Subscription channel state tracking.

Bug: 801985, 878446

Change-Id: If5a4781fdd2e12b8d94245375a1480e237248321
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1505441Reviewed-by: Tatiana Gornak <melandory@chromium.org>
Reviewed-by: Jan Krcal <jkrcal@chromium.org>
Commit-Queue: Tatiana Gornak <melandory@chromium.org>
Cr-Commit-Position: refs/heads/master@{#638585}

components/invalidation/impl/channels_states.cc

@@ -17,4 +17,17 @@ const char* FcmChannelStateToString(FcmChannelState state) {
   }
 }
 
+const char* SubscriptionChannelStateToString(SubscriptionChannelState state) {
+  switch (state) {
+    case SubscriptionChannelState::NOT_STARTED:
+      return "NOT_STARTED";
+    case SubscriptionChannelState::ENABLED:
+      return "ENABLED";
+    case SubscriptionChannelState::ACCESS_TOKEN_FAILURE:
+      return "ACCESS_TOKEN_FAILURE";
+    case SubscriptionChannelState::SUBSCRIPTION_FAILURE:
+      return "SUBSCRIPTION_FAILURE";
+  }
+}
+
 }  // namespace syncer

components/invalidation/impl/channels_states.h

@@ -17,8 +17,19 @@ enum class FcmChannelState {
   kMaxValue = NO_INSTANCE_ID_TOKEN,
 };
 
+enum class SubscriptionChannelState {
+  NOT_STARTED,
+  ENABLED,
+  ACCESS_TOKEN_FAILURE,
+  SUBSCRIPTION_FAILURE,
+
+  kMaxValue = SUBSCRIPTION_FAILURE,
+};
+
 const char* FcmChannelStateToString(FcmChannelState state);
 
+const char* SubscriptionChannelStateToString(SubscriptionChannelState state);
+
 }  // namespace syncer
 
 #endif  // COMPONENTS_INVALIDATION_IMPL_CHANNELS_STATES_H_

components/invalidation/impl/fcm_invalidation_listener.cc

@@ -58,7 +58,7 @@ void FCMInvalidationListener::UpdateRegisteredTopics(const TopicSet& topics) {
 
 void FCMInvalidationListener::Ready(InvalidationClient* client) {
   DCHECK_EQ(client, invalidation_client_.get());
-  subscription_channel_state_ = INVALIDATIONS_ENABLED;
+  subscription_channel_state_ = SubscriptionChannelState::ENABLED;
   EmitStateChange();
   DoRegistrationUpdate();
 }
@@ -199,25 +199,22 @@ void FCMInvalidationListener::Stop() {
   }
   per_user_topic_registration_manager_.reset();
 
-  subscription_channel_state_ = DEFAULT_INVALIDATION_ERROR;
+  subscription_channel_state_ = SubscriptionChannelState::NOT_STARTED;
   fcm_network_state_ = FcmChannelState::NOT_STARTED;
 }
 
 InvalidatorState FCMInvalidationListener::GetState() const {
-  if (subscription_channel_state_ == INVALIDATION_CREDENTIALS_REJECTED) {
-    // If either the ticl or the push client rejected our credentials,
-    // return INVALIDATION_CREDENTIALS_REJECTED.
+  if (subscription_channel_state_ ==
+      SubscriptionChannelState::ACCESS_TOKEN_FAILURE) {
     return INVALIDATION_CREDENTIALS_REJECTED;
   }
-  if (subscription_channel_state_ == INVALIDATIONS_ENABLED &&
+  if (subscription_channel_state_ == SubscriptionChannelState::ENABLED &&
       fcm_network_state_ == FcmChannelState::ENABLED) {
     // If the ticl is ready and the push client notifications are
     // enabled, return INVALIDATIONS_ENABLED.
     return INVALIDATIONS_ENABLED;
   }
-  if (subscription_channel_state_ == SUBSCRIPTION_FAILURE) {
-    return SUBSCRIPTION_FAILURE;
-  }
+
   // Otherwise, we have a transient error.
   return TRANSIENT_INVALIDATION_ERROR;
 }
@@ -232,8 +229,8 @@ void FCMInvalidationListener::OnFCMChannelStateChanged(FcmChannelState state) {
 }
 
 void FCMInvalidationListener::OnSubscriptionChannelStateChanged(
-    InvalidatorState invalidator_state) {
-  subscription_channel_state_ = invalidator_state;
+    SubscriptionChannelState state) {
+  subscription_channel_state_ = state;
   EmitStateChange();
 }
 
@@ -242,8 +239,9 @@ base::DictionaryValue FCMInvalidationListener::CollectDebugData() const {
       per_user_topic_registration_manager_->CollectDebugData();
   status.SetString("InvalidationListener.FCM-channel-state",
                    FcmChannelStateToString(fcm_network_state_));
-  status.SetString("InvalidationListener.Subscription-channel-state",
-                   InvalidatorStateToString(subscription_channel_state_));
+  status.SetString(
+      "InvalidationListener.Subscription-channel-state",
+      SubscriptionChannelStateToString(subscription_channel_state_));
   for (const Topic& topic : registered_topics_) {
     if (!status.HasKey(topic)) {
       status.SetString(topic, "Unregistered");

components/invalidation/impl/fcm_invalidation_listener.h

@@ -87,7 +87,7 @@ class FCMInvalidationListener : public InvalidationListener,
 
   // PerUserTopicRegistrationManager::Observer implementation.
   void OnSubscriptionChannelStateChanged(
-      InvalidatorState invalidator_state) override;
+      SubscriptionChannelState state) override;
 
   void DoRegistrationUpdate();
 
@@ -139,7 +139,8 @@ class FCMInvalidationListener : public InvalidationListener,
   TopicSet registered_topics_;
 
   // The states of the HTTP and FCM channel.
-  InvalidatorState subscription_channel_state_ = DEFAULT_INVALIDATION_ERROR;
+  SubscriptionChannelState subscription_channel_state_ =
+      SubscriptionChannelState::NOT_STARTED;
   FcmChannelState fcm_network_state_ = FcmChannelState::NOT_STARTED;
 
   std::unique_ptr<PerUserTopicRegistrationManager>

components/invalidation/impl/per_user_topic_registration_manager.cc

@@ -270,7 +270,7 @@ void PerUserTopicRegistrationManager::ActOnSuccesfullRegistration(
   if (all_subscription_completed &&
       base::FeatureList::IsEnabled(
           invalidation::switches::kFCMInvalidationsConservativeEnabling)) {
-    NotifySubscriptionChannelStateChange(INVALIDATIONS_ENABLED);
+    NotifySubscriptionChannelStateChange(SubscriptionChannelState::ENABLED);
   }
 }
 
@@ -308,7 +308,8 @@ void PerUserTopicRegistrationManager::RegistrationFinishedForTopic(
       if (type == PerUserTopicRegistrationRequest::SUBSCRIBE &&
           base::FeatureList::IsEnabled(
               invalidation::switches::kFCMInvalidationsConservativeEnabling)) {
-        NotifySubscriptionChannelStateChange(SUBSCRIPTION_FAILURE);
+        NotifySubscriptionChannelStateChange(
+            SubscriptionChannelState::SUBSCRIPTION_FAILURE);
       }
       if (!code.ShouldRetry()) {
         registration_statuses_.erase(it);
@@ -374,14 +375,15 @@ void PerUserTopicRegistrationManager::OnAccessTokenRequestSucceeded(
   request_access_token_backoff_.Reset();
   access_token_ = access_token;
   // Emit ENABLED when successfully got the token.
-  NotifySubscriptionChannelStateChange(INVALIDATIONS_ENABLED);
+  NotifySubscriptionChannelStateChange(SubscriptionChannelState::ENABLED);
   DoRegistrationUpdate();
 }
 
 void PerUserTopicRegistrationManager::OnAccessTokenRequestFailed(
     GoogleServiceAuthError error) {
   DCHECK_NE(error.state(), GoogleServiceAuthError::NONE);
-  NotifySubscriptionChannelStateChange(INVALIDATION_CREDENTIALS_REJECTED);
+  NotifySubscriptionChannelStateChange(
+      SubscriptionChannelState::ACCESS_TOKEN_FAILURE);
   request_access_token_backoff_.InformOfRequest(false);
   request_access_token_retry_timer_.Start(
       FROM_HERE, request_access_token_backoff_.GetTimeUntilRelease(),
@@ -410,18 +412,19 @@ void PerUserTopicRegistrationManager::DropAllSavedRegistrationsOnTokenChange(
 }
 
 void PerUserTopicRegistrationManager::NotifySubscriptionChannelStateChange(
-    InvalidatorState invalidator_state) {
-  // TRANSIENT_INVALIDATION_ERROR is the default state of the subscription
+    SubscriptionChannelState state) {
+  // NOT_STARTED is the default state of the subscription
   // channel and shouldn't explicitly issued.
-  DCHECK(invalidator_state != TRANSIENT_INVALIDATION_ERROR);
-  if (last_issued_state_ == invalidator_state) {
+  DCHECK(state != SubscriptionChannelState::NOT_STARTED);
+  if (last_issued_state_ == state) {
     // Notify only on state change.
     return;
   }
 
-  last_issued_state_ = invalidator_state;
-  for (auto& observer : observers_)
-    observer.OnSubscriptionChannelStateChanged(invalidator_state);
+  last_issued_state_ = state;
+  for (auto& observer : observers_) {
+    observer.OnSubscriptionChannelStateChanged(state);
+  }
 }
 
 base::DictionaryValue PerUserTopicRegistrationManager::CollectDebugData()

components/invalidation/impl/per_user_topic_registration_manager.h

@@ -13,6 +13,7 @@
 #include "base/sequence_checker.h"
 #include "base/time/time.h"
 #include "base/timer/timer.h"
+#include "components/invalidation/impl/channels_states.h"
 #include "components/invalidation/impl/per_user_topic_registration_request.h"
 #include "components/invalidation/public/identity_provider.h"
 #include "components/invalidation/public/invalidation_export.h"
@@ -46,7 +47,7 @@ class INVALIDATION_EXPORT PerUserTopicRegistrationManager {
   class Observer {
    public:
     virtual void OnSubscriptionChannelStateChanged(
-        InvalidatorState invalidator_state) = 0;
+        SubscriptionChannelState state) = 0;
   };
 
   PerUserTopicRegistrationManager(
@@ -105,7 +106,8 @@ class INVALIDATION_EXPORT PerUserTopicRegistrationManager {
 
   void DropAllSavedRegistrationsOnTokenChange(
       const std::string& instance_id_token);
-  void NotifySubscriptionChannelStateChange(InvalidatorState invalidator_state);
+  void NotifySubscriptionChannelStateChange(
+      SubscriptionChannelState invalidator_state);
 
   std::map<Topic, std::unique_ptr<RegistrationEntry>> registration_statuses_;
 
@@ -131,7 +133,8 @@ class INVALIDATION_EXPORT PerUserTopicRegistrationManager {
   const std::string project_id_;
 
   base::ObserverList<Observer>::Unchecked observers_;
-  InvalidatorState last_issued_state_ = TRANSIENT_INVALIDATION_ERROR;
+  SubscriptionChannelState last_issued_state_ =
+      SubscriptionChannelState::NOT_STARTED;
 
   SEQUENCE_CHECKER(sequence_checker_);
 

components/invalidation/impl/per_user_topic_registration_manager_unittest.cc

@@ -95,14 +95,15 @@ network::URLLoaderCompletionStatus CreateStatusForTest(
 class RegistrationManagerStateObserver
     : public PerUserTopicRegistrationManager::Observer {
  public:
-  void OnSubscriptionChannelStateChanged(InvalidatorState state) override {
+  void OnSubscriptionChannelStateChanged(
+      SubscriptionChannelState state) override {
     state_ = state;
   }
 
-  InvalidatorState observed_state() const { return state_; }
+  SubscriptionChannelState observed_state() const { return state_; }
 
  private:
-  InvalidatorState state_ = TRANSIENT_INVALIDATION_ERROR;
+  SubscriptionChannelState state_ = SubscriptionChannelState::NOT_STARTED;
 };
 
 class PerUserTopicRegistrationManagerTest : public testing::Test {
@@ -138,7 +139,9 @@ class PerUserTopicRegistrationManagerTest : public testing::Test {
 
   TestingPrefServiceSimple* pref_service() { return &pref_service_; }
 
-  InvalidatorState observed_state() { return state_observer_.observed_state(); }
+  SubscriptionChannelState observed_state() {
+    return state_observer_.observed_state();
+  }
 
   void AddCorrectSubscriptionResponce(
       const std::string& private_topic = std::string(),
@@ -416,7 +419,7 @@ TEST_F(
       ids, kFakeInstanceIdToken);
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(ids, per_user_topic_registration_manager->GetRegisteredIds());
-  EXPECT_EQ(observed_state(), INVALIDATIONS_ENABLED);
+  EXPECT_EQ(observed_state(), SubscriptionChannelState::ENABLED);
 
   // Disable some ids.
   TopicSet disabled_ids = GetSequenceOfTopics(3);
@@ -434,7 +437,7 @@ TEST_F(
       FullSubscriptionUrl(kFakeInstanceIdToken).spec(),
       std::string() /* content */, net::HTTP_NOT_FOUND);
 
-  EXPECT_EQ(observed_state(), INVALIDATIONS_ENABLED);
+  EXPECT_EQ(observed_state(), SubscriptionChannelState::ENABLED);
 }
 
 TEST_F(PerUserTopicRegistrationManagerTest,
@@ -453,7 +456,7 @@ TEST_F(PerUserTopicRegistrationManagerTest,
       ids, kFakeInstanceIdToken);
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(ids, per_user_topic_registration_manager->GetRegisteredIds());
-  EXPECT_EQ(observed_state(), INVALIDATIONS_ENABLED);
+  EXPECT_EQ(observed_state(), SubscriptionChannelState::ENABLED);
 
   // Disable some ids.
   TopicSet disabled_ids = GetSequenceOfTopics(3);
@@ -472,14 +475,14 @@ TEST_F(PerUserTopicRegistrationManagerTest,
   per_user_topic_registration_manager->UpdateRegisteredTopics(
       ids, kFakeInstanceIdToken);
   base::RunLoop().RunUntilIdle();
-  EXPECT_EQ(observed_state(), SUBSCRIPTION_FAILURE);
+  EXPECT_EQ(observed_state(), SubscriptionChannelState::SUBSCRIPTION_FAILURE);
 
   // Configure correct response and retry.
   AddCorrectSubscriptionResponce();
   per_user_topic_registration_manager->UpdateRegisteredTopics(
       ids, kFakeInstanceIdToken);
   base::RunLoop().RunUntilIdle();
-  EXPECT_EQ(observed_state(), INVALIDATIONS_ENABLED);
+  EXPECT_EQ(observed_state(), SubscriptionChannelState::ENABLED);
 }
 
 }  // namespace syncer