Push API: Refactor and fix unsubscribe API

chrome/browser/push_messaging/push_messaging_service_impl.h

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CHROME_BROWSER_PUSH_MESSAGING_PUSH_MESSAGING_SERVICE_IMPL_H_
 #define CHROME_BROWSER_PUSH_MESSAGING_PUSH_MESSAGING_SERVICE_IMPL_H_
 
 #include <stdint.h>
 #include <memory>
 #include <set>
@@ skipping 169 matching lines @@
 
   // GetEncryptionInfo method --------------------------------------------------
 
   void DidGetEncryptionInfo(
       const PushMessagingService::EncryptionInfoCallback& callback,
       const std::string& p256dh,
       const std::string& auth_secret) const;
 
   // Unsubscribe methods -------------------------------------------------------
 
-  void Unsubscribe(const std::string& app_id,
-                   const std::string& sender_id,
-                   const content::PushMessagingService::UnregisterCallback&);
+  // |origin|, |service_worker_registration_id| and |app_id| should be provided
+  // whenever they can be obtained. It's valid for |origin| to be empty and
+  // |service_worker_registration_id| to be kInvalidServiceWorkerRegistrationId,
+  // or for app_id to be empty, but not both at once.
+  void UnsubscribeInternal(
+      content::PushUnregistrationReason reason,
+      const GURL& origin,
+      int64_t service_worker_registration_id,
+      const std::string& app_id,
+      const std::string& sender_id,
+      const content::PushMessagingService::UnregisterCallback& callback);

[Peter Beverloo, 2016/09/30 14:26:07]

q: could we use std::move() for the callback as opposed to pass by ref?

[johnme, 2016/09/30 17:02:10]

Maybe. But I think I'd rather be consistent with the rest of the class for now,
and we can bulk update these later. Otherwise it'll be hard to remember when to
wrap parameters with std::move.

+
+  void DidClearPushSubscriptionId(
+      content::PushUnregistrationReason reason,
+      const std::string& app_id,
+      const std::string& sender_id,
+      const content::PushMessagingService::UnregisterCallback& callback);
 
   void DidDeleteID(const std::string& app_id,
                    bool was_subscribed,
                    const content::PushMessagingService::UnregisterCallback&,
                    instance_id::InstanceID::Result result);
 
-  void DidUnsubscribeInstanceID(
-      const std::string& app_id,
+  void DidUnsubscribe(
+      const std::string& app_id_if_was_instance_id,

[Peter Beverloo, 2016/09/30 14:26:07]

nit: if_was -> when?

[johnme, 2016/09/30 17:02:10]

Done.

       bool was_subscribed,
-      const content::PushMessagingService::UnregisterCallback&,
-      instance_id::InstanceID::Result result);
-
-  void DidUnsubscribe(bool was_subscribed,
-                      const content::PushMessagingService::UnregisterCallback&,
-                      gcm::GCMClient::Result result);
+      const content::PushMessagingService::UnregisterCallback& callback,
+      content::PushUnregistrationStatus status);
 
   // OnContentSettingChanged methods -------------------------------------------
 
   void UnsubscribeBecausePermissionRevoked(
       const PushMessagingAppIdentifier& app_identifier,
-      const base::Closure& closure,
+      const content::PushMessagingService::UnregisterCallback& callback,
       const std::string& sender_id,
       bool success,
       bool not_found);
 
   // Helper methods ------------------------------------------------------------
 
   // Normalizes the |sender_info|. In most cases the |sender_info| will be
   // passed through to the GCM Driver as-is, but NIST P-256 application server
   // keys have to be encoded using the URL-safe variant of the base64 encoding.
   std::string NormalizeSenderInfo(const std::string& sender_info) const;
@@ skipping 52 matching lines @@
   // we can finish processing them without being interrupted.
   std::unique_ptr<ScopedKeepAlive> in_flight_keep_alive_;
 #endif
 
   base::WeakPtrFactory<PushMessagingServiceImpl> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(PushMessagingServiceImpl);
 };
 
 #endif  // CHROME_BROWSER_PUSH_MESSAGING_PUSH_MESSAGING_SERVICE_IMPL_H_