Make Web Push use InstanceID tokens instead of GCM registrations

chrome/browser/push_messaging/push_messaging_service_impl.cc

Index: chrome/browser/push_messaging/push_messaging_service_impl.cc
diff --git a/chrome/browser/push_messaging/push_messaging_service_impl.cc b/chrome/browser/push_messaging/push_messaging_service_impl.cc
index f09d370b1bfd8a97ba228468c6113b059f235be2..2642f3f31a39d9f9366ebc9d8d297d3b3f30140c 100644
--- a/chrome/browser/push_messaging/push_messaging_service_impl.cc
+++ b/chrome/browser/push_messaging/push_messaging_service_impl.cc
@@ -25,6 +25,8 @@
 #include "chrome/browser/push_messaging/push_messaging_service_factory.h"
 #include "chrome/browser/push_messaging/push_messaging_service_observer.h"
 #include "chrome/browser/services/gcm/gcm_profile_service_factory.h"
+#include "chrome/browser/services/gcm/instance_id/instance_id_profile_service.h"
+#include "chrome/browser/services/gcm/instance_id/instance_id_profile_service_factory.h"
 #include "chrome/browser/ui/chrome_pages.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/features.h"
@@ -33,6 +35,8 @@
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/gcm_driver/gcm_driver.h"
 #include "components/gcm_driver/gcm_profile_service.h"
+#include "components/gcm_driver/instance_id/instance_id.h"
+#include "components/gcm_driver/instance_id/instance_id_driver.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/prefs/pref_service.h"
 #include "components/rappor/rappor_utils.h"
@@ -54,7 +58,14 @@
 #include "chrome/browser/lifetime/scoped_keep_alive.h"
 #endif
 
+using instance_id::InstanceID;
+
 namespace {
+
+// Scope passed to getToken to obtain GCM registration tokens.
+// Must match Java GoogleCloudMessaging.INSTANCE_ID_SCOPE.
+const char kGCMScope[] = "GCM";
+
 const int kMaxRegistrations = 1000000;
 
 // Chrome does not yet support silent push messages, and requires websites to
@@ -182,7 +193,7 @@ void PushMessagingServiceImpl::DecreasePushSubscriptionCount(int subtract,
 }
 
 bool PushMessagingServiceImpl::CanHandle(const std::string& app_id) const {
-  return !PushMessagingAppIdentifier::FindByAppId(profile_, app_id).is_null();
+  return app_id.rfind(kPushMessagingAppIdentifierPrefix, 0) == 0;

[Peter Beverloo, 2016/06/02 15:53:17]

Why can't we do a prefix search? (I.e. base::StartsWith(app_id,
kPushMessagingAppIdentifierPrefix, ...));

[johnme, 2016/06/07 14:16:43]

Done (this was already doing a prefix search; technically StartsWith is 3 lines
[including include] instead of 1, and constructs an extra StringPiece object,
but I agree it's more readable; also it makes sense to allow case insensitive
starts with so that the DCHECKs in PushMessagingAppIdentifier get a chance to
fire).

 }
 
 void PushMessagingServiceImpl::ShutdownHandler() {
@@ -398,10 +409,9 @@ void PushMessagingServiceImpl::SubscribeFromDocument(
   // Push does not allow permission requests from iframes.
   profile_->GetPermissionManager()->RequestPermission(
       content::PermissionType::PUSH_MESSAGING, web_contents->GetMainFrame(),
-      requesting_origin,
-      base::Bind(&PushMessagingServiceImpl::DidRequestPermission,
-                 weak_factory_.GetWeakPtr(), app_identifier, options,
-                 callback));
+      requesting_origin, base::Bind(&PushMessagingServiceImpl::DoSubscribe,
+                                    weak_factory_.GetWeakPtr(), app_identifier,
+                                    options, callback));
 }
 
 void PushMessagingServiceImpl::SubscribeFromWorker(
@@ -421,8 +431,7 @@ void PushMessagingServiceImpl::SubscribeFromWorker(
   }
 
   blink::WebPushPermissionStatus permission_status =
-      PushMessagingServiceImpl::GetPermissionStatus(requesting_origin,
-                                                    options.user_visible_only);
+      GetPermissionStatus(requesting_origin, options.user_visible_only);
 
   if (permission_status != blink::WebPushPermissionStatusGranted) {
     SubscribeEndWithError(register_callback,
@@ -430,14 +439,8 @@ void PushMessagingServiceImpl::SubscribeFromWorker(
     return;
   }
 
-  IncreasePushSubscriptionCount(1, true /* is_pending */);
-  std::vector<std::string> sender_ids(1,
-                                      NormalizeSenderInfo(options.sender_info));
-
-  GetGCMDriver()->Register(app_identifier.app_id(), sender_ids,
-                           base::Bind(&PushMessagingServiceImpl::DidSubscribe,
-                                      weak_factory_.GetWeakPtr(),
-                                      app_identifier, register_callback));
+  DoSubscribe(app_identifier, options, register_callback,
+              blink::mojom::PermissionStatus::GRANTED);
 }
 
 blink::WebPushPermissionStatus PushMessagingServiceImpl::GetPermissionStatus(
@@ -457,6 +460,28 @@ bool PushMessagingServiceImpl::SupportNonVisibleMessages() {
   return false;
 }
 
+void PushMessagingServiceImpl::DoSubscribe(
+    const PushMessagingAppIdentifier& app_identifier,
+    const content::PushSubscriptionOptions& options,
+    const content::PushMessagingService::RegisterCallback& register_callback,
+    blink::mojom::PermissionStatus permission_status) {
+  if (permission_status != blink::mojom::PermissionStatus::GRANTED) {
+    SubscribeEndWithError(register_callback,
+                          content::PUSH_REGISTRATION_STATUS_PERMISSION_DENIED);
+    return;
+  }
+
+  IncreasePushSubscriptionCount(1, true /* is_pending */);
+
+  GetInstanceIDDriver()
+      ->GetInstanceID(app_identifier.app_id())
+      ->GetToken(NormalizeSenderInfo(options.sender_info), kGCMScope,
+                 std::map<std::string, std::string>() /* options */,

[Peter Beverloo, 2016/06/02 15:53:18]

Will this call be modified when we switch to subtypes? Do we need to branch
again in the tests when that happens?

[johnme, 2016/06/07 14:16:43]

The GetInstanceID call will be modified then. Assuming we land both InstanceID
and desktop subtypes in the same Chrome build, we don't need to fork the tests
(the tests that use InstanceID will just start using subtypes).

+                 base::Bind(&PushMessagingServiceImpl::DidSubscribe,
+                            weak_factory_.GetWeakPtr(), app_identifier,
+                            options.sender_info, register_callback));
+}
+
 void PushMessagingServiceImpl::SubscribeEnd(
     const content::PushMessagingService::RegisterCallback& callback,
     const std::string& subscription_id,
@@ -476,35 +501,36 @@ void PushMessagingServiceImpl::SubscribeEndWithError(
 
 void PushMessagingServiceImpl::DidSubscribe(
     const PushMessagingAppIdentifier& app_identifier,
+    const std::string& sender_id,
     const content::PushMessagingService::RegisterCallback& callback,
     const std::string& subscription_id,
-    gcm::GCMClient::Result result) {
+    InstanceID::Result result) {
   DecreasePushSubscriptionCount(1, true /* was_pending */);
 
   content::PushRegistrationStatus status =
       content::PUSH_REGISTRATION_STATUS_SERVICE_ERROR;
 
   switch (result) {
-    case gcm::GCMClient::SUCCESS:
+    case InstanceID::SUCCESS:
       // Make sure that this subscription has associated encryption keys prior
       // to returning it to the developer - they'll need this information in
       // order to send payloads to the user.
-      GetGCMDriver()->GetEncryptionInfo(
-          app_identifier.app_id(),
+      GetEncryptionInfoImpl(
+          app_identifier.app_id(), sender_id,
           base::Bind(&PushMessagingServiceImpl::DidSubscribeWithEncryptionInfo,
                      weak_factory_.GetWeakPtr(), app_identifier, callback,
                      subscription_id));
-
       return;
-    case gcm::GCMClient::INVALID_PARAMETER:
-    case gcm::GCMClient::GCM_DISABLED:
-    case gcm::GCMClient::ASYNC_OPERATION_PENDING:
-    case gcm::GCMClient::SERVER_ERROR:
-    case gcm::GCMClient::UNKNOWN_ERROR:
+    case InstanceID::INVALID_PARAMETER:
+    case InstanceID::DISABLED:
+    case InstanceID::ASYNC_OPERATION_PENDING:
+    case InstanceID::SERVER_ERROR:
+    case InstanceID::UNKNOWN_ERROR:
+      DLOG(ERROR) << "Push messaging subscription failed; InstanceID::Result = "
+                  << result;
       status = content::PUSH_REGISTRATION_STATUS_SERVICE_ERROR;
       break;
-    case gcm::GCMClient::NETWORK_ERROR:
-    case gcm::GCMClient::TTL_EXCEEDED:
+    case InstanceID::NETWORK_ERROR:
       status = content::PUSH_REGISTRATION_STATUS_NETWORK_ERROR;
       break;
   }
@@ -534,32 +560,12 @@ void PushMessagingServiceImpl::DidSubscribeWithEncryptionInfo(
                content::PUSH_REGISTRATION_STATUS_SUCCESS_FROM_PUSH_SERVICE);
 }
 
-void PushMessagingServiceImpl::DidRequestPermission(
-    const PushMessagingAppIdentifier& app_identifier,
-    const content::PushSubscriptionOptions& options,
-    const content::PushMessagingService::RegisterCallback& register_callback,
-    blink::mojom::PermissionStatus permission_status) {
-  if (permission_status != blink::mojom::PermissionStatus::GRANTED) {
-    SubscribeEndWithError(register_callback,
-                          content::PUSH_REGISTRATION_STATUS_PERMISSION_DENIED);
-    return;
-  }
-
-  IncreasePushSubscriptionCount(1, true /* is_pending */);
-  std::vector<std::string> sender_ids(1,
-                                      NormalizeSenderInfo(options.sender_info));
-
-  GetGCMDriver()->Register(app_identifier.app_id(), sender_ids,
-                           base::Bind(&PushMessagingServiceImpl::DidSubscribe,
-                                      weak_factory_.GetWeakPtr(),
-                                      app_identifier, register_callback));
-}
-
 // GetEncryptionInfo methods ---------------------------------------------------
 
 void PushMessagingServiceImpl::GetEncryptionInfo(
     const GURL& origin,
     int64_t service_worker_registration_id,
+    const std::string& sender_id,
     const PushMessagingService::EncryptionInfoCallback& callback) {
   PushMessagingAppIdentifier app_identifier =
       PushMessagingAppIdentifier::FindByServiceWorker(
@@ -567,8 +573,8 @@ void PushMessagingServiceImpl::GetEncryptionInfo(
 
   DCHECK(!app_identifier.is_null());
 
-  GetGCMDriver()->GetEncryptionInfo(
-      app_identifier.app_id(),
+  GetEncryptionInfoImpl(
+      app_identifier.app_id(), sender_id,
       base::Bind(&PushMessagingServiceImpl::DidGetEncryptionInfo,
                  weak_factory_.GetWeakPtr(), callback));
 }
@@ -595,10 +601,8 @@ void PushMessagingServiceImpl::Unsubscribe(
       PushMessagingAppIdentifier::FindByServiceWorker(
           profile_, requesting_origin, service_worker_registration_id);
   if (app_identifier.is_null()) {
-    if (!callback.is_null()) {
-      callback.Run(
-          content::PUSH_UNREGISTRATION_STATUS_SUCCESS_WAS_NOT_REGISTERED);
-    }
+    callback.Run(
+        content::PUSH_UNREGISTRATION_STATUS_SUCCESS_WAS_NOT_REGISTERED);
     return;
   }
 
@@ -615,37 +619,58 @@ void PushMessagingServiceImpl::Unsubscribe(
   // retry unregistration if it fails due to network errors (crbug.com/465399).
   PushMessagingAppIdentifier app_identifier =
       PushMessagingAppIdentifier::FindByAppId(profile_, app_id);
-  bool was_registered = !app_identifier.is_null();
-  if (was_registered)
+  bool was_subscribed = !app_identifier.is_null();
+  if (was_subscribed)
     app_identifier.DeleteFromPrefs(profile_);
 
-  const auto& unregister_callback =
-      base::Bind(&PushMessagingServiceImpl::DidUnsubscribe,
-                 weak_factory_.GetWeakPtr(), was_registered, callback);
-#if defined(OS_ANDROID)
-  // On Android the backend is different, and requires the original sender_id.
-  // UnsubscribeBecausePermissionRevoked sometimes calls us with an empty one.
-  if (sender_id.empty()) {
-    unregister_callback.Run(gcm::GCMClient::INVALID_PARAMETER);
+  if (PushMessagingAppIdentifier::UseInstanceID(app_id)) {
+    GetInstanceIDDriver()->GetInstanceID(app_id)->DeleteID(base::Bind(
+        &PushMessagingServiceImpl::DidDeleteID, weak_factory_.GetWeakPtr(),
+        app_id, was_subscribed, callback));
   } else {
-    GetGCMDriver()->UnregisterWithSenderId(
-        app_id, NormalizeSenderInfo(sender_id), unregister_callback);
-  }
+    const auto& unregister_callback =

[Peter Beverloo, 2016/06/02 15:53:18]

nit: this probably shouldn't be a reference?

[johnme, 2016/06/07 14:16:43]

Done (though technically it should be equivalent)

+        base::Bind(&PushMessagingServiceImpl::DidUnsubscribe,
+                   weak_factory_.GetWeakPtr(), was_subscribed, callback);
+#if defined(OS_ANDROID)
+    // On Android the backend is different, and requires the original sender_id.
+    // UnsubscribeBecausePermissionRevoked sometimes calls us with an empty one.
+    if (sender_id.empty()) {
+      unregister_callback.Run(gcm::GCMClient::INVALID_PARAMETER);
+    } else {
+      GetGCMDriver()->UnregisterWithSenderId(
+          app_id, NormalizeSenderInfo(sender_id), unregister_callback);
+    }
 #else
-  GetGCMDriver()->Unregister(app_id, unregister_callback);
+    GetGCMDriver()->Unregister(app_id, unregister_callback);
 #endif
+  }
 }
 
-void PushMessagingServiceImpl::DidUnsubscribe(
+void PushMessagingServiceImpl::DidDeleteID(
+    const std::string& app_id,
+    bool was_subscribed,
+    const content::PushMessagingService::UnregisterCallback& callback,
+    InstanceID::Result result) {
+  // DidUnsubscribeInstanceID must be run asynchronously, since it calls
+  // InstanceIDDriver::RemoveInstanceID which deletes the InstanceID itself.
+  // Calling that immediately would cause a use-after-free in our caller.
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::Bind(&PushMessagingServiceImpl::DidUnsubscribeInstanceID,
+                            weak_factory_.GetWeakPtr(), app_id, was_subscribed,
+                            callback, result));
+}
+
+void PushMessagingServiceImpl::DidUnsubscribeInstanceID(
+    const std::string& app_id,
     bool was_subscribed,
     const content::PushMessagingService::UnregisterCallback& callback,
-    gcm::GCMClient::Result result) {
+    InstanceID::Result result) {
+  GetInstanceIDDriver()->RemoveInstanceID(app_id);
+
   if (was_subscribed)
     DecreasePushSubscriptionCount(1, false /* was_pending */);
 
-  // Internal calls pass a null callback.
-  if (callback.is_null())
-    return;
+  DCHECK(!callback.is_null());
 
   if (!was_subscribed) {
     callback.Run(
@@ -653,6 +678,37 @@ void PushMessagingServiceImpl::DidUnsubscribe(
     return;
   }
   switch (result) {
+    case InstanceID::SUCCESS:
+      callback.Run(content::PUSH_UNREGISTRATION_STATUS_SUCCESS_UNREGISTERED);
+      break;
+    case InstanceID::INVALID_PARAMETER:
+    case InstanceID::DISABLED:
+    case InstanceID::SERVER_ERROR:
+    case InstanceID::UNKNOWN_ERROR:
+      callback.Run(content::PUSH_UNREGISTRATION_STATUS_PENDING_SERVICE_ERROR);
+      break;
+    case InstanceID::ASYNC_OPERATION_PENDING:
+    case InstanceID::NETWORK_ERROR:
+      callback.Run(content::PUSH_UNREGISTRATION_STATUS_PENDING_NETWORK_ERROR);
+      break;
+  }
+}
+
+void PushMessagingServiceImpl::DidUnsubscribe(
+    bool was_subscribed,
+    const content::PushMessagingService::UnregisterCallback& callback,
+    gcm::GCMClient::Result gcm_result) {
+  if (was_subscribed)
+    DecreasePushSubscriptionCount(1, false /* was_pending */);
+
+  DCHECK(!callback.is_null());
+
+  if (!was_subscribed) {
+    callback.Run(
+        content::PUSH_UNREGISTRATION_STATUS_SUCCESS_WAS_NOT_REGISTERED);
+    return;
+  }
+  switch (gcm_result) {
     case gcm::GCMClient::SUCCESS:
       callback.Run(content::PUSH_UNREGISTRATION_STATUS_SUCCESS_UNREGISTERED);
       break;
@@ -708,12 +764,23 @@ void PushMessagingServiceImpl::OnContentSettingChanged(
       continue;
     }
 
-    GetSenderId(
-        profile_, app_identifier.origin(),
-        app_identifier.service_worker_registration_id(),
-        base::Bind(
-            &PushMessagingServiceImpl::UnsubscribeBecausePermissionRevoked,
-            weak_factory_.GetWeakPtr(), app_identifier, barrier_closure));
+    bool need_sender_id = false;
+#if defined(OS_ANDROID)
+    need_sender_id =
+        !PushMessagingAppIdentifier::UseInstanceID(app_identifier.app_id());
+#endif
+    if (need_sender_id) {
+      GetSenderId(
+          profile_, app_identifier.origin(),
+          app_identifier.service_worker_registration_id(),
+          base::Bind(
+              &PushMessagingServiceImpl::UnsubscribeBecausePermissionRevoked,
+              weak_factory_.GetWeakPtr(), app_identifier, barrier_closure));
+    } else {
+      UnsubscribeBecausePermissionRevoked(
+          app_identifier, barrier_closure, "" /* sender_id */,
+          true /* success */, true /* not_found */);
+    }
   }
 }
 
@@ -728,8 +795,9 @@ void PushMessagingServiceImpl::UnsubscribeBecausePermissionRevoked(
   // Unsubscribe the PushMessagingAppIdentifier with the push service.
   // It's possible for GetSenderId to have failed and sender_id to be empty, if
   // cookies (and the SW database) for an origin got cleared before permissions
-  // are cleared for the origin. In that case Unsubscribe will just delete the
-  // app identifier to block future messages.
+  // are cleared for the origin. In that case for legacy GCM registrations on
+  // Android, Unsubscribe will just delete the app identifier to block future
+  // messages.
   // TODO(johnme): Auto-unregister before SW DB is cleared
   // (https://crbug.com/402458).
   Unsubscribe(app_identifier.app_id(), sender_id,
@@ -792,6 +860,18 @@ bool PushMessagingServiceImpl::IsPermissionSet(const GURL& origin) {
          blink::WebPushPermissionStatusGranted;
 }
 
+void PushMessagingServiceImpl::GetEncryptionInfoImpl(

[Peter Beverloo, 2016/06/02 15:53:18]

The *Impl suffix is not really appropriate here. Maybe use
GetEncryptionInfoForAppIdentifier()?

[johnme, 2016/06/07 14:16:43]

Done (GetEncryptionInfoForAppId, since it's not an identifier object)

+    const std::string& app_id,
+    const std::string& sender_id,
+    gcm::GCMEncryptionProvider::EncryptionInfoCallback callback) {
+  if (PushMessagingAppIdentifier::UseInstanceID(app_id)) {
+    GetInstanceIDDriver()->GetInstanceID(app_id)->GetEncryptionInfo(
+        NormalizeSenderInfo(sender_id), callback);
+  } else {
+    GetGCMDriver()->GetEncryptionInfo(app_id, callback);
+  }
+}
+
 gcm::GCMDriver* PushMessagingServiceImpl::GetGCMDriver() const {
   gcm::GCMProfileService* gcm_profile_service =
       gcm::GCMProfileServiceFactory::GetForProfile(profile_);
@@ -799,3 +879,12 @@ gcm::GCMDriver* PushMessagingServiceImpl::GetGCMDriver() const {
   CHECK(gcm_profile_service->driver());
   return gcm_profile_service->driver();
 }
+
+instance_id::InstanceIDDriver* PushMessagingServiceImpl::GetInstanceIDDriver()
+    const {
+  instance_id::InstanceIDProfileService* instance_id_profile_service =
+      instance_id::InstanceIDProfileServiceFactory::GetForProfile(profile_);
+  CHECK(instance_id_profile_service);
+  CHECK(instance_id_profile_service->driver());
+  return instance_id_profile_service->driver();
+}