Push API: Don't wait for network when unsubscribing

chrome/browser/gcm/fake_gcm_profile_service.cc

Index: chrome/browser/gcm/fake_gcm_profile_service.cc
diff --git a/chrome/browser/gcm/fake_gcm_profile_service.cc b/chrome/browser/gcm/fake_gcm_profile_service.cc
index f2db5fbd296a730e6d0883c7aaffe190b6a48f0f..dbc16c929b8c2ccc1807b725f6dc6ac77e4d191b 100644
--- a/chrome/browser/gcm/fake_gcm_profile_service.cc
+++ b/chrome/browser/gcm/fake_gcm_profile_service.cc
@@ -10,12 +10,15 @@
 #include "base/format_macros.h"
 #include "base/location.h"
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 #include "base/threading/thread_task_runner_handle.h"
+#include "base/time/time.h"
 #include "chrome/browser/profiles/profile.h"
 #include "components/gcm_driver/fake_gcm_client_factory.h"
+#include "components/gcm_driver/gcm_backoff_policy.h"
 #include "components/gcm_driver/instance_id/fake_gcm_driver_for_instance_id.h"
 #include "content/public/browser/browser_context.h"
 
@@ -23,7 +26,25 @@ namespace gcm {
 
 namespace {
 
-class CustomFakeGCMDriver : public instance_id::FakeGCMDriverForInstanceID {
+using ResultCallback = base::Callback<void(GCMClient::Result result)>;
+
+void FailAfterMaxBackoff(ResultCallback callback) {
+  // Worst-case backoff, where the device never comes back online so all retries
+  // fail.
+  base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+      FROM_HERE,
+      // GCMClientImpl::{OnRegisterCompleted,OnUnregisterCompleted} both map
+      // {RegistrationRequest,UnregistrationRequest}::REACHED_MAX_RETRIES
+      // respectively to GCMClient::SERVER_ERROR.
+      base::Bind(callback, GCMClient::SERVER_ERROR),
+      base::TimeDelta::FromMilliseconds(
+          GetGCMBackoffPolicy().maximum_backoff_ms));
+}
+
+}  // namespace
+
+class FakeGCMProfileService::CustomFakeGCMDriver
+    : public instance_id::FakeGCMDriverForInstanceID {
  public:
   explicit CustomFakeGCMDriver(FakeGCMProfileService* service);
   ~CustomFakeGCMDriver() override;
@@ -31,8 +52,6 @@ class CustomFakeGCMDriver : public instance_id::FakeGCMDriverForInstanceID {
   void OnRegisterFinished(const std::string& app_id,
                           const std::string& registration_id,
                           GCMClient::Result result);
-  void OnUnregisterFinished(const std::string& app_id,
-                            GCMClient::Result result);
   void OnSendFinished(const std::string& app_id,
                       const std::string& message_id,
                       GCMClient::Result result);
@@ -41,7 +60,7 @@ class CustomFakeGCMDriver : public instance_id::FakeGCMDriverForInstanceID {
                          const IncomingMessage& message);
 
  protected:
-  // FakeGCMDriverForInstanceID overrides:
+  // FakeGCMDriver overrides:
   void RegisterImpl(const std::string& app_id,
                     const std::vector<std::string>& sender_ids) override;
   void UnregisterImpl(const std::string& app_id) override;
@@ -51,72 +70,166 @@ class CustomFakeGCMDriver : public instance_id::FakeGCMDriverForInstanceID {
                 const std::string& receiver_id,
                 const OutgoingMessage& message) override;
 
+  // FakeGCMDriverForInstanceID overrides:
+  void GetToken(const std::string& app_id,
+                const std::string& authorized_entity,
+                const std::string& scope,
+                const std::map<std::string, std::string>& options,
+                const GetTokenCallback& callback) override;
+  void DeleteToken(const std::string& app_id,
+                   const std::string& authorized_entity,
+                   const std::string& scope,
+                   const DeleteTokenCallback& callback) override;
+
  private:
+  void DoRegister(const std::string& app_id,
+                  const std::vector<std::string>& sender_ids,
+                  const std::string& registration_id);
+  void DoSend(const std::string& app_id,
+              const std::string& receiver_id,
+              const OutgoingMessage& message);
+
   FakeGCMProfileService* service_;
 
+  // Used to give each registration a unique registration id. Does not decrease
+  // when unregister is called.
+  int registration_count_ = 0;
+
+  base::WeakPtrFactory<CustomFakeGCMDriver> weak_factory_;  // Must be last.
+
   DISALLOW_COPY_AND_ASSIGN(CustomFakeGCMDriver);
 };
 
-CustomFakeGCMDriver::CustomFakeGCMDriver(FakeGCMProfileService* service)
+FakeGCMProfileService::CustomFakeGCMDriver::CustomFakeGCMDriver(
+    FakeGCMProfileService* service)
     : instance_id::FakeGCMDriverForInstanceID(
           base::ThreadTaskRunnerHandle::Get()),
-      service_(service) {}
+      service_(service),
+      weak_factory_(this) {}
 
-CustomFakeGCMDriver::~CustomFakeGCMDriver() {
-}
+FakeGCMProfileService::CustomFakeGCMDriver::~CustomFakeGCMDriver() {}
 
-void CustomFakeGCMDriver::RegisterImpl(
+void FakeGCMProfileService::CustomFakeGCMDriver::RegisterImpl(
     const std::string& app_id,
     const std::vector<std::string>& sender_ids) {
+  if (service_->is_offline_) {
+    FailAfterMaxBackoff(base::Bind(&CustomFakeGCMDriver::RegisterFinished,
+                                   weak_factory_.GetWeakPtr(), app_id,
+                                   std::string() /* registration_id */));
+    return;
+  }
+
+  // Generate fake registration IDs, encoding the number of sender IDs (used by
+  // GcmApiTest.RegisterValidation), then an incrementing count (even for the
+  // same app_id - there's no caching) so tests can distinguish registrations.
+  std::string registration_id = base::StringPrintf(
+      "%" PRIuS "-%d", sender_ids.size(), registration_count_);
+  ++registration_count_;
+
   base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE, base::Bind(&FakeGCMProfileService::RegisterFinished,
-                            base::Unretained(service_), app_id, sender_ids));
+      FROM_HERE,
+      base::Bind(&CustomFakeGCMDriver::DoRegister, base::Unretained(this),

[Peter Beverloo, 2017/02/07 18:11:08]

Here and elsewhere - why is it safe to use |this|? Why not use |weak_factory_|?

[johnme, 2017/02/08 14:14:36]

Done (the existing code used |this| for immediate PostTask thread hops,
implicitly placing some restrictions on how tests can use this class; I agree
that it'd be safer to consistently use weak pointers though).

+                 app_id, sender_ids, registration_id));
+}
+
+void FakeGCMProfileService::CustomFakeGCMDriver::DoRegister(
+    const std::string& app_id,
+    const std::vector<std::string>& sender_ids,
+    const std::string& registration_id) {
+  if (service_->collect_) {
+    service_->last_registered_app_id_ = app_id;
+    service_->last_registered_sender_ids_ = sender_ids;
+  }
+  RegisterFinished(app_id, registration_id, GCMClient::SUCCESS);
 }
 
-void CustomFakeGCMDriver::UnregisterImpl(const std::string& app_id) {
+void FakeGCMProfileService::CustomFakeGCMDriver::UnregisterImpl(
+    const std::string& app_id) {
+  if (service_->is_offline_) {
+    FailAfterMaxBackoff(base::Bind(&CustomFakeGCMDriver::UnregisterFinished,
+                                   weak_factory_.GetWeakPtr(), app_id));
+    return;
+  }
+
+  GCMClient::Result result = GCMClient::SUCCESS;
+  if (!service_->unregister_responses_.empty()) {
+    result = service_->unregister_responses_.front();
+    service_->unregister_responses_.pop_front();
+  }
   base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE, base::Bind(&FakeGCMProfileService::UnregisterFinished,
-                            base::Unretained(service_), app_id));
+      FROM_HERE, base::Bind(&CustomFakeGCMDriver::UnregisterFinished,
+                            base::Unretained(this), app_id, result));
 }
 
-void CustomFakeGCMDriver::UnregisterWithSenderIdImpl(
+void FakeGCMProfileService::CustomFakeGCMDriver::UnregisterWithSenderIdImpl(
     const std::string& app_id,
-    const std::string& sender_id) {}
+    const std::string& sender_id) {
+  NOTREACHED() << "This Android-specific method is not yet faked.";
+}
+
+void FakeGCMProfileService::CustomFakeGCMDriver::SendImpl(
+    const std::string& app_id,
+    const std::string& receiver_id,
+    const OutgoingMessage& message) {
+  if (service_->is_offline_) {
+    FailAfterMaxBackoff(base::Bind(&CustomFakeGCMDriver::SendFinished,
+                                   weak_factory_.GetWeakPtr(), app_id,
+                                   message.id));
+    return;
+  }
 
-void CustomFakeGCMDriver::SendImpl(const std::string& app_id,
-                                   const std::string& receiver_id,
-                                   const OutgoingMessage& message) {
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE,
-      base::Bind(&FakeGCMProfileService::SendFinished,
-                 base::Unretained(service_), app_id, receiver_id, message));
+      base::Bind(&CustomFakeGCMDriver::DoSend, base::Unretained(this), app_id,
+                 receiver_id, message));
 }
 
-void CustomFakeGCMDriver::OnRegisterFinished(
+void FakeGCMProfileService::CustomFakeGCMDriver::DoSend(
     const std::string& app_id,
-    const std::string& registration_id,
-    GCMClient::Result result) {
-  RegisterFinished(app_id, registration_id, result);
+    const std::string& receiver_id,
+    const OutgoingMessage& message) {
+  if (service_->collect_) {
+    service_->last_sent_message_ = message;
+    service_->last_receiver_id_ = receiver_id;
+  }
+  SendFinished(app_id, message.id, GCMClient::SUCCESS);
 }
 
-void CustomFakeGCMDriver::OnUnregisterFinished(const std::string& app_id,
-                                               GCMClient::Result result) {
-  UnregisterFinished(app_id, result);
+void FakeGCMProfileService::CustomFakeGCMDriver::GetToken(
+    const std::string& app_id,
+    const std::string& authorized_entity,
+    const std::string& scope,
+    const std::map<std::string, std::string>& options,
+    const GetTokenCallback& callback) {
+  if (service_->is_offline_) {
+    FailAfterMaxBackoff(base::Bind(callback, std::string() /* token */));
+    return;
+  }
+
+  instance_id::FakeGCMDriverForInstanceID::GetToken(app_id, authorized_entity,
+                                                    scope, options, callback);
 }
 
-void CustomFakeGCMDriver::OnSendFinished(const std::string& app_id,
-                                         const std::string& message_id,
-                                         GCMClient::Result result) {
-  SendFinished(app_id, message_id, result);
+void FakeGCMProfileService::CustomFakeGCMDriver::DeleteToken(
+    const std::string& app_id,
+    const std::string& authorized_entity,
+    const std::string& scope,
+    const DeleteTokenCallback& callback) {
+  if (service_->is_offline_) {
+    FailAfterMaxBackoff(callback);
+    return;
+  }
+
+  instance_id::FakeGCMDriverForInstanceID::DeleteToken(
+      app_id, authorized_entity, scope, callback);
 }
 
-void CustomFakeGCMDriver::OnDispatchMessage(const std::string& app_id,
-                                            const IncomingMessage& message) {
+void FakeGCMProfileService::CustomFakeGCMDriver::OnDispatchMessage(
+    const std::string& app_id,
+    const IncomingMessage& message) {
   DispatchMessage(app_id, message);
 }
 
-}  // namespace
-
 // static
 std::unique_ptr<KeyedService> FakeGCMProfileService::Build(
     content::BrowserContext* context) {
@@ -127,73 +240,15 @@ std::unique_ptr<KeyedService> FakeGCMProfileService::Build(
   return std::move(service);
 }
 
-FakeGCMProfileService::FakeGCMProfileService(Profile* profile)
-    : collect_(false),
-      registration_count_(0) {
-}
+FakeGCMProfileService::FakeGCMProfileService(Profile* profile) {}
 
 FakeGCMProfileService::~FakeGCMProfileService() {}
 
-void FakeGCMProfileService::RegisterFinished(
-    const std::string& app_id,
-    const std::vector<std::string>& sender_ids) {
-  if (collect_) {
-    last_registered_app_id_ = app_id;
-    last_registered_sender_ids_ = sender_ids;
-  }
-
-  // Generate fake registration IDs, encoding the number of sender IDs (used by
-  // GcmApiTest.RegisterValidation), then an incrementing count (even for the
-  // same app_id - there's no caching) so tests can distinguish registrations.
-  std::string registration_id = base::StringPrintf("%" PRIuS "-%d",
-                                                   sender_ids.size(),
-                                                   registration_count_);
-  ++registration_count_;
-
-  CustomFakeGCMDriver* custom_driver =
-      static_cast<CustomFakeGCMDriver*>(driver());
-  custom_driver->OnRegisterFinished(
-      app_id, registration_id, GCMClient::SUCCESS);
-}
-
-void FakeGCMProfileService::UnregisterFinished(const std::string& app_id) {
-  GCMClient::Result result = GCMClient::SUCCESS;
-  if (!unregister_responses_.empty()) {
-    result = unregister_responses_.front();
-    unregister_responses_.pop_front();
-  }
-
-  CustomFakeGCMDriver* custom_driver =
-      static_cast<CustomFakeGCMDriver*>(driver());
-  custom_driver->OnUnregisterFinished(app_id, result);
-
-  if (!unregister_callback_.is_null())
-    unregister_callback_.Run(app_id);
-}
-
-void FakeGCMProfileService::SendFinished(const std::string& app_id,
-                                         const std::string& receiver_id,
-                                         const OutgoingMessage& message) {
-  if (collect_) {
-    last_sent_message_ = message;
-    last_receiver_id_ = receiver_id;
-  }
-
-  CustomFakeGCMDriver* custom_driver =
-      static_cast<CustomFakeGCMDriver*>(driver());
-  custom_driver->OnSendFinished(app_id, message.id, GCMClient::SUCCESS);
-}
-
 void FakeGCMProfileService::AddExpectedUnregisterResponse(
     GCMClient::Result result) {
   unregister_responses_.push_back(result);
 }
 
-void FakeGCMProfileService::SetUnregisterCallback(
-    const UnregisterCallback& callback) {
-  unregister_callback_ = callback;
-}
-
 void FakeGCMProfileService::DispatchMessage(const std::string& app_id,
                                             const IncomingMessage& message) {
   CustomFakeGCMDriver* custom_driver =