Push API: Don't wait for network when unsubscribing

chrome/browser/gcm/fake_gcm_profile_service.h

Index: chrome/browser/gcm/fake_gcm_profile_service.h
diff --git a/chrome/browser/gcm/fake_gcm_profile_service.h b/chrome/browser/gcm/fake_gcm_profile_service.h
index 7b4db7b858f0980e8677105a82af3be567708404..a7f6214fce74c348a7e2b3a3fb495a6d436946d2 100644
--- a/chrome/browser/gcm/fake_gcm_profile_service.h
+++ b/chrome/browser/gcm/fake_gcm_profile_service.h
@@ -24,8 +24,6 @@ namespace gcm {
 // Acts as a bridge between GCM API and GCMClient layer for testing purposes.
 class FakeGCMProfileService : public GCMProfileService {
  public:
-  typedef base::Callback<void(const std::string&)> UnregisterCallback;
-
   // Helper function to be used with
   // KeyedService::SetTestingFactory().
   static std::unique_ptr<KeyedService> Build(content::BrowserContext* context);
@@ -33,17 +31,8 @@ class FakeGCMProfileService : public GCMProfileService {
   explicit FakeGCMProfileService(Profile* profile);
   ~FakeGCMProfileService() override;
 
-  void RegisterFinished(const std::string& app_id,
-                        const std::vector<std::string>& sender_ids);
-  void UnregisterFinished(const std::string& app_id);
-  void SendFinished(const std::string& app_id,
-                    const std::string& receiver_id,
-                    const OutgoingMessage& message);
-
   void AddExpectedUnregisterResponse(GCMClient::Result result);
 
-  void SetUnregisterCallback(const UnregisterCallback& callback);
-
   void DispatchMessage(const std::string& app_id,
                        const IncomingMessage& message);
 
@@ -67,19 +56,24 @@ class FakeGCMProfileService : public GCMProfileService {
     collect_ = collect;
   }
 
+  // Simple offline simulation: requests fail after max backoff.
+  void set_offline(bool is_offline) { is_offline_ = is_offline; }
+
  private:
+  class CustomFakeGCMDriver;
+  friend class CustomFakeGCMDriver;

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

nit: since the CustomFakeGCMDriver already stores a pointer to the
FakeGCMProfileService*, can we avoid (a) the nested class and (b) the friend
declaration by just adding a public is_offline()?

Since this entire class is meant for testing, that seems nicer.

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

No, it's not just is_offline_, it's also collect_, last_registered_app_id_,
last_registered_sender_ids_, unregister_responses_, last_sent_message_ and
last_receiver_id_. So, um, every single private variable of
FakeGCMProfileService :) I think that validates this approach of treating the
CustomFakeGCMDriver as part of the internals of FakeGCMProfileService.

+
   // Indicates whether the service will collect paramters of the calls for
-  // furter verification in tests.
-  bool collect_;
-  // Used to give each registration a unique registration id. Does not decrease
-  // when unregister is called.
-  int registration_count_;
+  // further verification in tests.
+  bool collect_ = false;
+
+  bool is_offline_ = false;

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

nit: it's a bit unfortunate that collect_ is documented by the property, but
is_offline_ is documented by the setter. Can we align?

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

Done (documented both at public setter).

+
   std::string last_registered_app_id_;
   std::vector<std::string> last_registered_sender_ids_;
   std::list<GCMClient::Result> unregister_responses_;
   OutgoingMessage last_sent_message_;
   std::string last_receiver_id_;
-  UnregisterCallback unregister_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(FakeGCMProfileService);
 };