Add unregistration to BinaryFCMService

Token are now unregistered when the BinaryUploadService is done using
them. This is useful for privacy, since the GCM device id will reset
when there are no active registrations.

Bug: 1020418
Change-Id: I3a74203a2817e463fb878ce9a8da20773e58033e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2003354Reviewed-by: Bettina Dea <bdea@chromium.org>
Commit-Queue: Daniel Rubery <drubery@chromium.org>
Cr-Commit-Position: refs/heads/master@{#734306}

chrome/browser/safe_browsing/cloud_content_scanning/binary_fcm_service.cc

@@ -7,6 +7,7 @@
 #include <memory>
 
 #include "base/base64.h"
+#include "base/bind_helpers.h"
 #include "base/logging.h"
 #include "base/metrics/histogram_functions.h"
 #include "chrome/browser/gcm/gcm_profile_service_factory.h"
@@ -72,6 +73,10 @@ BinaryFCMService::BinaryFCMService()
 BinaryFCMService::~BinaryFCMService() {
   if (gcm_driver_ != nullptr)
     gcm_driver_->RemoveAppHandler(kBinaryFCMServiceAppId);
+  for (const auto& token_to_callback : message_token_map_) {
+    const std::string& token = token_to_callback.first;
+    UnregisterInstanceID(token, base::DoNothing());
+  }
 }
 
 void BinaryFCMService::GetInstanceID(GetInstanceIDCallback callback) {
@@ -94,6 +99,16 @@ void BinaryFCMService::ClearCallbackForToken(const std::string& token) {
   message_token_map_.erase(token);
 }
 
+void BinaryFCMService::UnregisterInstanceID(
+    const std::string& token,
+    UnregisterInstanceIDCallback callback) {
+  instance_id_driver_->GetInstanceID(kBinaryFCMServiceAppId)
+      ->DeleteToken(kBinaryFCMServiceSenderId, instance_id::kGCMScope,
+                    base::BindOnce(&BinaryFCMService::OnInstanceIDUnregistered,
+                                   weakptr_factory_.GetWeakPtr(), token,
+                                   std::move(callback)));
+}
+
 void BinaryFCMService::OnGetInstanceID(GetInstanceIDCallback callback,
                                        const std::string& instance_id,
                                        instance_id::InstanceID::Result result) {
@@ -154,4 +169,25 @@ bool BinaryFCMService::CanHandle(const std::string& app_id) const {
   return app_id == kBinaryFCMServiceAppId;
 }
 
+void BinaryFCMService::OnInstanceIDUnregistered(
+    const std::string& token,
+    UnregisterInstanceIDCallback callback,
+    instance_id::InstanceID::Result result) {
+  switch (result) {
+    case instance_id::InstanceID::Result::SUCCESS:
+      std::move(callback).Run(true);
+      break;
+    case instance_id::InstanceID::Result::INVALID_PARAMETER:
+    case instance_id::InstanceID::Result::DISABLED:
+    case instance_id::InstanceID::Result::UNKNOWN_ERROR:
+      std::move(callback).Run(false);
+      break;
+    case instance_id::InstanceID::Result::ASYNC_OPERATION_PENDING:
+    case instance_id::InstanceID::Result::NETWORK_ERROR:
+    case instance_id::InstanceID::Result::SERVER_ERROR:
+      UnregisterInstanceID(token, std::move(callback));
+      break;
+  }
+}
+
 }  // namespace safe_browsing

chrome/browser/safe_browsing/cloud_content_scanning/binary_fcm_service.h

@@ -5,6 +5,7 @@
 #ifndef CHROME_BROWSER_SAFE_BROWSING_CLOUD_CONTENT_SCANNING_BINARY_FCM_SERVICE_H_
 #define CHROME_BROWSER_SAFE_BROWSING_CLOUD_CONTENT_SCANNING_BINARY_FCM_SERVICE_H_
 
+#include "base/callback_forward.h"
 #include "base/memory/weak_ptr.h"
 #include "components/gcm_driver/gcm_app_handler.h"
 #include "components/gcm_driver/instance_id/instance_id.h"
@@ -43,8 +44,11 @@ class BinaryFCMService : public gcm::GCMAppHandler {
       base::OnceCallback<void(const std::string& token)>;
   using OnMessageCallback =
       base::RepeatingCallback<void(DeepScanningClientResponse)>;
+  using UnregisterInstanceIDCallback = base::OnceCallback<void(bool)>;
 
   virtual void GetInstanceID(GetInstanceIDCallback callback);
+  virtual void UnregisterInstanceID(const std::string& token,
+                                    UnregisterInstanceIDCallback callback);
   void SetCallbackForToken(const std::string& token,
                            OnMessageCallback callback);
   void ClearCallbackForToken(const std::string& token);
@@ -73,6 +77,10 @@ class BinaryFCMService : public gcm::GCMAppHandler {
                        const std::string& instance_id,
                        instance_id::InstanceID::Result result);
 
+  void OnInstanceIDUnregistered(const std::string& token,
+                                UnregisterInstanceIDCallback callback,
+                                instance_id::InstanceID::Result result);
+
   // References to the profile's GCMDriver and InstanceIDDriver. Both are
   // unowned.
   gcm::GCMDriver* gcm_driver_;

chrome/browser/safe_browsing/cloud_content_scanning/binary_fcm_service_unittest.cc

@@ -12,6 +12,8 @@
 #include "chrome/test/base/testing_profile.h"
 #include "components/gcm_driver/common/gcm_message.h"
 #include "components/gcm_driver/fake_gcm_profile_service.h"
+#include "components/gcm_driver/gcm_client.h"
+#include "components/gcm_driver/instance_id/instance_id.h"
 #include "components/safe_browsing/core/proto/webprotect.pb.h"
 #include "content/public/test/browser_task_environment.h"
 #include "content/public/test/test_utils.h"
@@ -193,4 +195,52 @@ TEST_F(BinaryFCMServiceTest, EmitsMessageHasValidTokenHistogram) {
   }
 }
 
+TEST_F(BinaryFCMServiceTest, UnregisterToken) {
+  // Get an instance ID
+  std::string received_instance_id = BinaryFCMService::kInvalidId;
+  binary_fcm_service_->GetInstanceID(base::BindOnce(
+      [](std::string* target_id, const std::string& instance_id) {
+        *target_id = instance_id;
+      },
+      &received_instance_id));
+  content::RunAllTasksUntilIdle();
+  EXPECT_NE(received_instance_id, BinaryFCMService::kInvalidId);
+
+  // Delete it
+  bool unregistered = false;
+  binary_fcm_service_->UnregisterInstanceID(
+      received_instance_id,
+      base::BindOnce([](bool* target, bool value) { *target = value; },
+                     &unregistered));
+  content::RunAllTasksUntilIdle();
+  EXPECT_TRUE(unregistered);
+}
+
+TEST_F(BinaryFCMServiceTest, UnregisterTokenRetriesFailures) {
+  // Get an instance ID
+  std::string received_instance_id = BinaryFCMService::kInvalidId;
+  binary_fcm_service_->GetInstanceID(base::BindOnce(
+      [](std::string* target_id, const std::string& instance_id) {
+        *target_id = instance_id;
+      },
+      &received_instance_id));
+  content::RunAllTasksUntilIdle();
+  EXPECT_NE(received_instance_id, BinaryFCMService::kInvalidId);
+
+  // Queue one failure, then success
+  gcm::FakeGCMProfileService* gcm_service =
+      static_cast<gcm::FakeGCMProfileService*>(
+          gcm::GCMProfileServiceFactory::GetForProfile(&profile_));
+  gcm_service->AddExpectedUnregisterResponse(gcm::GCMClient::NETWORK_ERROR);
+
+  // Delete it
+  bool unregistered = false;
+  binary_fcm_service_->UnregisterInstanceID(
+      received_instance_id,
+      base::BindOnce([](bool* target, bool value) { *target = value; },
+                     &unregistered));
+  content::RunAllTasksUntilIdle();
+  EXPECT_TRUE(unregistered);
+}
+
 }  // namespace safe_browsing

chrome/browser/safe_browsing/cloud_content_scanning/binary_upload_service.cc

@@ -313,6 +313,11 @@ void BinaryUploadService::FinishRequest(Request* request,
   auto token_it = active_tokens_.find(request);
   if (token_it != active_tokens_.end()) {
     binary_fcm_service_->ClearCallbackForToken(token_it->second);
+
+    // The BinaryFCMService will handle all recoverable errors. In case of
+    // unrecoverable error, there's nothing we can do here.
+    binary_fcm_service_->UnregisterInstanceID(token_it->second,
+                                              base::DoNothing());
     active_tokens_.erase(token_it);
   }
 }

chrome/browser/safe_browsing/cloud_content_scanning/binary_upload_service_unittest.cc

@@ -92,6 +92,9 @@ class MockBinaryFCMService : public BinaryFCMService {
 
   MOCK_METHOD1(GetInstanceID,
                void(BinaryFCMService::GetInstanceIDCallback callback));
+  MOCK_METHOD2(UnregisterInstanceID,
+               void(const std::string& token,
+                    BinaryFCMService::UnregisterInstanceIDCallback callback));
 };
 
 class BinaryUploadServiceTest : public testing::Test {
@@ -121,6 +124,12 @@ class BinaryUploadServiceTest : public testing::Test {
             Invoke([id](BinaryFCMService::GetInstanceIDCallback callback) {
               std::move(callback).Run(id);
             }));
+    ON_CALL(*fcm_service_, UnregisterInstanceID(_, _))
+        .WillByDefault(
+            Invoke([](const std::string& token,
+                      BinaryFCMService::UnregisterInstanceIDCallback callback) {
+              std::move(callback).Run(true);
+            }));
   }
 
   void UploadForDeepScanning(