[SyncInvalidations] Validate FCM token periodically.

FCM registration token can become invalid. This CL re-requests the token once a day and notifies token observers in case token has become invalid.

Code is mostly copied from the old invalidations implementation (components/invalidation/impl/fcm_network_handler.*).

Change-Id: I67ad7e4f315e38bc8d34710ca4551ebd2e080ade
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2391071
Commit-Queue: Paula Vidas <paulavidas@google.com>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#804730}

components/sync/invalidations/fcm_handler.cc

@@ -5,6 +5,7 @@
 #include "components/sync/invalidations/fcm_handler.h"
 
 #include <map>
+#include <utility>
 
 #include "base/logging.h"
 #include "base/time/time.h"
@@ -17,6 +18,9 @@ namespace syncer {
 
 const char kPayloadKey[] = "payload";
 
+// Lower bound time between two token validations when listening.
+const int kTokenValidationPeriodMinutesDefault = 60 * 24;
+
 FCMHandler::FCMHandler(gcm::GCMDriver* gcm_driver,
                        instance_id::InstanceIDDriver* instance_id_driver,
                        const std::string& sender_id,
@@ -35,19 +39,16 @@ void FCMHandler::StartListening() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(!IsListening());
   gcm_driver_->AddAppHandler(app_id_, this);
-  // TODO(crbug.com/1108780): set appropriate TTL.
-  instance_id_driver_->GetInstanceID(app_id_)->GetToken(
-      sender_id_, instance_id::kGCMScope, /*time_to_live=*/base::TimeDelta(),
-      /*options=*/std::map<std::string, std::string>(),
-      /*flags=*/{instance_id::InstanceID::Flags::kIsLazy},
-      base::BindRepeating(&FCMHandler::DidRetrieveToken,
-                          weak_ptr_factory_.GetWeakPtr()));
+  StartTokenFetch(base::BindOnce(&FCMHandler::DidRetrieveToken,
+                                 weak_ptr_factory_.GetWeakPtr()));
 }
 
 void FCMHandler::StopListening() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  if (IsListening())
+  if (IsListening()) {
     gcm_driver_->RemoveAppHandler(app_id_);
+    token_validation_timer_.AbandonAndStop();
+  }
 }
 
 const std::string& FCMHandler::GetFCMRegistrationToken() const {
@@ -129,22 +130,72 @@ bool FCMHandler::IsListening() const {
 void FCMHandler::DidRetrieveToken(const std::string& subscription_token,
                                   instance_id::InstanceID::Result result) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  // TODO(crbug.com/1108783): add a UMA histogram to monitor results.
-  if (result == instance_id::InstanceID::SUCCESS) {
-    if (fcm_registration_token_ == subscription_token) {
-      // Nothing has changed, do not notify observers.
-      return;
-    }
+  if (!IsListening()) {
+    // After we requested the token, |StopListening| has been called. Thus,
+    // ignore the token.
+    return;
+  }
 
+  // TODO(crbug.com/1108783): add a UMA histogram to monitor results.
+  // Notify observers only if the token has changed.
+  if (result == instance_id::InstanceID::SUCCESS &&
+      fcm_registration_token_ != subscription_token) {
     fcm_registration_token_ = subscription_token;
     for (FCMRegistrationTokenObserver& token_observer : token_observers_) {
       token_observer.OnFCMRegistrationTokenChanged();
     }
-  } else {
+  } else if (result != instance_id::InstanceID::SUCCESS) {
     DLOG(WARNING) << "Messaging subscription failed: " << result;
   }
 
-  // TODO(crbug.com/1102336): schedule next token validation.
+  ScheduleNextTokenValidation();
+}
+
+void FCMHandler::ScheduleNextTokenValidation() {
+  DCHECK(IsListening());
+
+  token_validation_timer_.Start(
+      FROM_HERE,
+      base::TimeDelta::FromMinutes(kTokenValidationPeriodMinutesDefault),
+      base::BindOnce(&FCMHandler::StartTokenValidation,
+                     weak_ptr_factory_.GetWeakPtr()));
+}
+
+void FCMHandler::StartTokenValidation() {
+  DCHECK(IsListening());
+  StartTokenFetch(base::BindOnce(&FCMHandler::DidReceiveTokenForValidation,
+                                 weak_ptr_factory_.GetWeakPtr()));
+}
+
+void FCMHandler::DidReceiveTokenForValidation(
+    const std::string& new_token,
+    instance_id::InstanceID::Result result) {
+  if (!IsListening()) {
+    // After we requested the token, |StopListening| has been called. Thus,
+    // ignore the token.
+    return;
+  }
+
+  // Notify observers only if the token has changed.
+  if (result == instance_id::InstanceID::SUCCESS &&
+      fcm_registration_token_ != new_token) {
+    fcm_registration_token_ = new_token;
+    for (FCMRegistrationTokenObserver& token_observer : token_observers_) {
+      token_observer.OnFCMRegistrationTokenChanged();
+    }
+  }
+
+  ScheduleNextTokenValidation();
+}
+
+void FCMHandler::StartTokenFetch(
+    instance_id::InstanceID::GetTokenCallback callback) {
+  // TODO(crbug.com/1108780): set appropriate TTL.
+  instance_id_driver_->GetInstanceID(app_id_)->GetToken(
+      sender_id_, instance_id::kGCMScope,
+      /*time_to_live=*/base::TimeDelta(),
+      /*options=*/std::map<std::string, std::string>(),
+      /*flags=*/{instance_id::InstanceID::Flags::kIsLazy}, std::move(callback));
 }
 
 }  // namespace syncer

components/sync/invalidations/fcm_handler.h

@@ -10,6 +10,7 @@
 
 #include "base/observer_list.h"
 #include "base/sequence_checker.h"
+#include "base/timer/timer.h"
 #include "components/gcm_driver/gcm_app_handler.h"
 #include "components/gcm_driver/instance_id/instance_id.h"
 #include "components/keyed_service/core/keyed_service.h"
@@ -78,6 +79,12 @@ class FCMHandler : public gcm::GCMAppHandler {
   // Called when a subscription token is obtained from the GCM server.
   void DidRetrieveToken(const std::string& subscription_token,
                         instance_id::InstanceID::Result result);
+  void ScheduleNextTokenValidation();
+  void StartTokenValidation();
+  void DidReceiveTokenForValidation(const std::string& new_token,
+                                    instance_id::InstanceID::Result result);
+
+  void StartTokenFetch(instance_id::InstanceID::GetTokenCallback callback);
 
   SEQUENCE_CHECKER(sequence_checker_);
 
@@ -89,6 +96,8 @@ class FCMHandler : public gcm::GCMAppHandler {
   // Contains an FCM registration token if not empty.
   std::string fcm_registration_token_;
 
+  base::OneShotTimer token_validation_timer_;
+
   // Contains all listeners to notify about each incoming message in OnMessage
   // method.
   base::ObserverList<InvalidationsListener,

components/sync/invalidations/fcm_handler_unittest.cc

@@ -34,6 +34,8 @@ const char kDefaultSenderId[] = "fake_sender_id";
 const char kSyncInvalidationsAppId[] = "com.google.chrome.sync.invalidations";
 const char kPayloadKey[] = "payload";
 
+const int kTokenValidationPeriodMinutesDefault = 60 * 24;
+
 class MockInstanceID : public InstanceID {
  public:
   MockInstanceID() : InstanceID("app_id", /*gcm_driver=*/nullptr) {}
@@ -95,7 +97,8 @@ class FCMHandlerTest : public testing::Test {
   }
 
  protected:
-  base::test::SingleThreadTaskEnvironment task_environment_;
+  base::test::SingleThreadTaskEnvironment task_environment_{
+      base::test::TaskEnvironment::TimeSource::MOCK_TIME};
 
   gcm::FakeGCMDriver fake_gcm_driver_;
   NiceMock<MockInstanceIDDriver> mock_instance_id_driver_;
@@ -146,5 +149,63 @@ TEST_F(FCMHandlerTest, ShouldNotifyOnTokenChange) {
   fcm_handler_.RemoveTokenObserver(&mock_token_observer);
 }
 
+TEST_F(FCMHandlerTest, ShouldScheduleTokenValidationAndActOnNewToken) {
+  NiceMock<MockTokenObserver> mock_token_observer;
+  fcm_handler_.AddTokenObserver(&mock_token_observer);
+
+  // Check that the handler gets the token through GetToken and notifies the
+  // observer.
+  EXPECT_CALL(mock_instance_id_, GetToken(_, _, _, _, _, _))
+      .WillOnce(WithArg<5>(Invoke([](InstanceID::GetTokenCallback callback) {
+        std::move(callback).Run("token", InstanceID::Result::SUCCESS);
+      })));
+  EXPECT_CALL(mock_token_observer, OnFCMRegistrationTokenChanged()).Times(1);
+  fcm_handler_.StartListening();
+
+  // Adjust the time and check that validation will happen in time.
+  // The old token is invalid, so token observer should be informed.
+  task_environment_.FastForwardBy(
+      base::TimeDelta::FromMinutes(kTokenValidationPeriodMinutesDefault) -
+      base::TimeDelta::FromSeconds(1));
+  // When it is time, validation happens.
+  EXPECT_CALL(mock_instance_id_, GetToken(_, _, _, _, _, _))
+      .WillOnce(WithArg<5>(Invoke([](InstanceID::GetTokenCallback callback) {
+        std::move(callback).Run("new token", InstanceID::Result::SUCCESS);
+      })));
+  EXPECT_CALL(mock_token_observer, OnFCMRegistrationTokenChanged()).Times(1);
+  task_environment_.FastForwardBy(base::TimeDelta::FromSeconds(1));
+
+  fcm_handler_.RemoveTokenObserver(&mock_token_observer);
+}
+
+TEST_F(FCMHandlerTest, ShouldScheduleTokenValidationAndNotActOnSameToken) {
+  NiceMock<MockTokenObserver> mock_token_observer;
+  fcm_handler_.AddTokenObserver(&mock_token_observer);
+
+  // Check that the handler gets the token through GetToken and notifies the
+  // observer.
+  EXPECT_CALL(mock_instance_id_, GetToken(_, _, _, _, _, _))
+      .WillOnce(WithArg<5>(Invoke([](InstanceID::GetTokenCallback callback) {
+        std::move(callback).Run("token", InstanceID::Result::SUCCESS);
+      })));
+  EXPECT_CALL(mock_token_observer, OnFCMRegistrationTokenChanged()).Times(1);
+  fcm_handler_.StartListening();
+
+  // Adjust the time and check that validation will happen in time.
+  // The old token is valid, so token observer should not be informed.
+  task_environment_.FastForwardBy(
+      base::TimeDelta::FromMinutes(kTokenValidationPeriodMinutesDefault) -
+      base::TimeDelta::FromSeconds(1));
+  // When it is time, validation happens.
+  EXPECT_CALL(mock_instance_id_, GetToken(_, _, _, _, _, _))
+      .WillOnce(WithArg<5>(Invoke([](InstanceID::GetTokenCallback callback) {
+        std::move(callback).Run("token", InstanceID::Result::SUCCESS);
+      })));
+  EXPECT_CALL(mock_token_observer, OnFCMRegistrationTokenChanged()).Times(0);
+  task_environment_.FastForwardBy(base::TimeDelta::FromSeconds(1));
+
+  fcm_handler_.RemoveTokenObserver(&mock_token_observer);
+}
+
 }  // namespace
 }  // namespace syncer