Make Web Push use InstanceID tokens instead of GCM registrations

chrome/browser/push_messaging/push_messaging_service_impl.cc

 // 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.
 
 #include "chrome/browser/push_messaging/push_messaging_service_impl.h"
 
 #include <vector>
 
 #include "base/barrier_closure.h"
 #include "base/base64url.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback_helpers.h"
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
 #include "build/build_config.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/permissions/permission_manager.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/push_messaging/push_messaging_app_identifier.h"
 #include "chrome/browser/push_messaging/push_messaging_constants.h"
 #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"
 #include "chrome/common/pref_names.h"
 #include "chrome/grit/generated_resources.h"
 #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"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/permission_type.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/service_worker_context.h"
 #include "content/public/browser/storage_partition.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/child_process_host.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/push_messaging_status.h"
 #include "content/public/common/push_subscription_options.h"
 #include "ui/base/l10n/l10n_util.h"
 
 #if BUILDFLAG(ENABLE_BACKGROUND)
 #include "chrome/browser/background/background_mode_manager.h"
 #endif
 
+using instance_id::InstanceID;
+
 namespace {
+const char kGCMScope[] = "GCM";

[Peter Beverloo, 2016/04/21 13:26:19]

micro nit: blank line after namespace definition.

Please document this constant as well.

[johnme, 2016/05/25 14:11:51]

Done.

+
 const int kMaxRegistrations = 1000000;
 
 // Chrome does not yet support silent push messages, and requires websites to
 // indicate that they will only send user-visible messages.
 const char kSilentPushUnsupportedMessage[] =
     "Chrome currently only supports the Push API for subscriptions that will "
     "result in user-visible messages. You can indicate this by calling "
     "pushManager.subscribe({userVisibleOnly: true}) instead. See "
     "https://goo.gl/yqv4Q4 for more details.";
 
@@ skipping 107 matching lines @@
 #if BUILDFLAG(ENABLE_BACKGROUND)
     if (UseBackgroundMode() && g_browser_process->background_mode_manager()) {
       g_browser_process->background_mode_manager()->UnregisterTrigger(profile_,
                                                                       this);
     }
 #endif  // BUILDFLAG(ENABLE_BACKGROUND)
   }
 }
 
 bool PushMessagingServiceImpl::CanHandle(const std::string& app_id) const {
-  return !PushMessagingAppIdentifier::FindByAppId(profile_, app_id).is_null();
+  return app_id.rfind(kPushMessagingAppIdentifierPrefix, 0) == 0;
 }
 
 void PushMessagingServiceImpl::ShutdownHandler() {
   // Shutdown() should come before and it removes us from the list of app
   // handlers of gcm::GCMDriver so this shouldn't ever been called.
   NOTREACHED();
 }
 
 // OnMessage methods -----------------------------------------------------------
 
@@ skipping 85 matching lines @@
             completion_closure_runner.Release());
       }
 #endif
       break;
     case content::PUSH_DELIVERY_STATUS_INVALID_MESSAGE:
     case content::PUSH_DELIVERY_STATUS_SERVICE_WORKER_ERROR:
       break;
     case content::PUSH_DELIVERY_STATUS_UNKNOWN_APP_ID:
     case content::PUSH_DELIVERY_STATUS_PERMISSION_DENIED:
     case content::PUSH_DELIVERY_STATUS_NO_SERVICE_WORKER:
-      Unsubscribe(app_id, message.sender_id,
-                  base::Bind(&UnregisterCallbackToClosure,
-                             completion_closure_runner.Release()));
+      Unsubscribe(app_id, base::Bind(&UnregisterCallbackToClosure,
+                                     completion_closure_runner.Release()));
       break;
   }
 
   RecordDeliveryStatus(status);
 }
 
 void PushMessagingServiceImpl::DidHandleMessage(
     const std::string& app_id,
     const base::Closure& message_handled_closure) {
   auto in_flight_iterator = in_flight_message_deliveries_.find(app_id);
@@ skipping 71 matching lines @@
         content::CONSOLE_MESSAGE_LEVEL_ERROR, kSilentPushUnsupportedMessage);
 
     SubscribeEndWithError(callback,
                           content::PUSH_REGISTRATION_STATUS_PERMISSION_DENIED);
     return;
   }
 
   // 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(
     const GURL& requesting_origin,
     int64_t service_worker_registration_id,
     const content::PushSubscriptionOptions& options,
     const content::PushMessagingService::RegisterCallback& register_callback) {
   PushMessagingAppIdentifier app_identifier =
       PushMessagingAppIdentifier::Generate(requesting_origin,
                                            service_worker_registration_id);
 
   if (push_subscription_count_ + pending_push_subscription_count_ >=
       kMaxRegistrations) {
     SubscribeEndWithError(register_callback,
                           content::PUSH_REGISTRATION_STATUS_LIMIT_REACHED);
     return;
   }
 
   blink::WebPushPermissionStatus permission_status =
       PushMessagingServiceImpl::GetPermissionStatus(requesting_origin,

[Peter Beverloo, 2016/04/21 13:26:19]

What's up with this call? I.e. why isn't it this->GetPermissionStatus()? Should
probably kill blink::WebPushPermissionStatus too at some point.

[johnme, 2016/05/25 14:11:51]

Weird. Removed the PushMessagingServiceImpl::.

                                                     options.user_visible_only);
 
   if (permission_status != blink::WebPushPermissionStatusGranted) {
     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));
+  DoSubscribe(app_identifier, options, register_callback,
+              blink::mojom::PermissionStatus::GRANTED);
 }
 
 blink::WebPushPermissionStatus PushMessagingServiceImpl::GetPermissionStatus(
     const GURL& origin,
     bool user_visible) {
   if (!user_visible)
     return blink::WebPushPermissionStatusDenied;
 
   // Because the Push API is tied to Service Workers, many usages of the API
   // won't have an embedding origin at all. Only consider the requesting
   // |origin| when checking whether permission to use the API has been granted.
   return ToPushPermission(profile_->GetPermissionManager()->GetPermissionStatus(
       content::PermissionType::PUSH_MESSAGING, origin, origin));
 }
 
 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) /* authorized_entity */,
+          kGCMScope, std::map<std::string, std::string>() /* options */,
+          base::Bind(&PushMessagingServiceImpl::DidSubscribe,
+                     weak_factory_.GetWeakPtr(), app_identifier,
+                     register_callback));
+}
+
 void PushMessagingServiceImpl::SubscribeEnd(
     const content::PushMessagingService::RegisterCallback& callback,
     const std::string& subscription_id,
     const std::vector<uint8_t>& p256dh,
     const std::vector<uint8_t>& auth,
     content::PushRegistrationStatus status) {
   callback.Run(subscription_id, p256dh, auth, status);
 }
 
 void PushMessagingServiceImpl::SubscribeEndWithError(
     const content::PushMessagingService::RegisterCallback& callback,
     content::PushRegistrationStatus status) {
   SubscribeEnd(callback, std::string() /* subscription_id */,
                std::vector<uint8_t>() /* p256dh */,
                std::vector<uint8_t>() /* auth */, status);
 }
 
 void PushMessagingServiceImpl::DidSubscribe(
     const PushMessagingAppIdentifier& app_identifier,
     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(),
           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:
       status = content::PUSH_REGISTRATION_STATUS_SERVICE_ERROR;

[Peter Beverloo, 2016/04/21 13:26:18]

I'm beginning to think we should consider having DLOG(ERROR) calls in aggregated
switch cases like this, which would tremendously help debugging of any sort of
issue. WDYT?

[johnme, 2016/05/25 14:11:51]

Done (hopefully it won't trigger too often; in theory it's possible for sloppy
sites to trigger this in a loop).

       break;
-    case gcm::GCMClient::NETWORK_ERROR:
-    case gcm::GCMClient::TTL_EXCEEDED:
+    case InstanceID::NETWORK_ERROR:
       status = content::PUSH_REGISTRATION_STATUS_NETWORK_ERROR;
       break;
   }
 
   SubscribeEndWithError(callback, status);
 }
 
 void PushMessagingServiceImpl::DidSubscribeWithEncryptionInfo(
     const PushMessagingAppIdentifier& app_identifier,
     const content::PushMessagingService::RegisterCallback& callback,
     const std::string& subscription_id,
     const std::string& p256dh,
     const std::string& auth_secret) {
   if (!p256dh.size()) {
     SubscribeEndWithError(
         callback, content::PUSH_REGISTRATION_STATUS_PUBLIC_KEY_UNAVAILABLE);
     return;
   }
 
   app_identifier.PersistToPrefs(profile_);
 
   IncreasePushSubscriptionCount(1, false /* is_pending */);
 
   SubscribeEnd(callback, subscription_id,
                std::vector<uint8_t>(p256dh.begin(), p256dh.end()),
                std::vector<uint8_t>(auth_secret.begin(), auth_secret.end()),
                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 PushMessagingService::EncryptionInfoCallback& callback) {
   PushMessagingAppIdentifier app_identifier =
       PushMessagingAppIdentifier::FindByServiceWorker(
           profile_, origin, service_worker_registration_id);
 
@@ skipping 14 matching lines @@
 
   callback.Run(success, std::vector<uint8_t>(p256dh.begin(), p256dh.end()),
                std::vector<uint8_t>(auth_secret.begin(), auth_secret.end()));
 }
 
 // Unsubscribe methods ---------------------------------------------------------
 
 void PushMessagingServiceImpl::Unsubscribe(
     const GURL& requesting_origin,
     int64_t service_worker_registration_id,
-    const std::string& sender_id,
     const content::PushMessagingService::UnregisterCallback& callback) {
   PushMessagingAppIdentifier app_identifier =
       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);
     }
     return;
   }
 
-  Unsubscribe(app_identifier.app_id(), sender_id, callback);
+  Unsubscribe(app_identifier.app_id(), callback);
 }
 
 void PushMessagingServiceImpl::Unsubscribe(
     const std::string& app_id,
-    const std::string& sender_id,
     const content::PushMessagingService::UnregisterCallback& callback) {
   // Delete the mapping for this app_id, to guarantee that no messages get
   // delivered in future (even if unregistration fails).
   // TODO(johnme): Instead of deleting these app ids, store them elsewhere, and
   // 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)
     app_identifier.DeleteFromPrefs(profile_);
 
-  const auto& unregister_callback =
+  GetInstanceIDDriver()->GetInstanceID(app_id)->DeleteID(

[Peter Beverloo, 2016/04/21 13:26:18]

Now that we don't call GCMDriver::Unregister(WithSenderId), what gets rid of the
stored encryption keys in the GCM key store?

[johnme, 2016/05/25 14:11:51]

848 lines later... Done (https://codereview.chromium.org/1923953002)

       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);
-  } else {
-    GetGCMDriver()->UnregisterWithSenderId(
-        app_id, NormalizeSenderInfo(sender_id), unregister_callback);
-  }
-#else
-  GetGCMDriver()->Unregister(app_id, unregister_callback);
-#endif
+                 weak_factory_.GetWeakPtr(), app_id, was_registered, callback));
 }
 
 void PushMessagingServiceImpl::DidUnsubscribe(
+    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())

[Peter Beverloo, 2016/04/21 13:26:18]

While not related to your CL, this isn't true anymore. Mind updating this to:

DCHECK(!callback.is_null());

And remove the if-statement from PushMessagingServiceImpl::Unsubscribe() for
bonus points? :-)

[johnme, 2016/05/25 14:11:51]

Done.

     return;
 
   if (!was_subscribed) {
     callback.Run(
         content::PUSH_UNREGISTRATION_STATUS_SUCCESS_WAS_NOT_REGISTERED);
     return;
   }
   switch (result) {
-    case gcm::GCMClient::SUCCESS:
+    case InstanceID::SUCCESS:
       callback.Run(content::PUSH_UNREGISTRATION_STATUS_SUCCESS_UNREGISTERED);
       break;
-    case gcm::GCMClient::INVALID_PARAMETER:
-    case gcm::GCMClient::GCM_DISABLED:
-    case gcm::GCMClient::SERVER_ERROR:
-    case gcm::GCMClient::UNKNOWN_ERROR:
+    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 gcm::GCMClient::ASYNC_OPERATION_PENDING:
-    case gcm::GCMClient::NETWORK_ERROR:
-    case gcm::GCMClient::TTL_EXCEEDED:
+    case InstanceID::ASYNC_OPERATION_PENDING:
+    case InstanceID::NETWORK_ERROR:
       callback.Run(content::PUSH_UNREGISTRATION_STATUS_PENDING_NETWORK_ERROR);
       break;
   }
 }
 
 // OnContentSettingChanged methods ---------------------------------------------
 
 void PushMessagingServiceImpl::OnContentSettingChanged(
     const ContentSettingsPattern& primary_pattern,
     const ContentSettingsPattern& secondary_pattern,
@@ skipping 23 matching lines @@
         !primary_pattern.Matches(app_identifier.origin())) {
       barrier_closure.Run();
       continue;
     }
 
     if (IsPermissionSet(app_identifier.origin())) {
       barrier_closure.Run();
       continue;
     }
 
-    GetSenderId(
-        profile_, app_identifier.origin(),
-        app_identifier.service_worker_registration_id(),
-        base::Bind(
-            &PushMessagingServiceImpl::UnsubscribeBecausePermissionRevoked,
-            weak_factory_.GetWeakPtr(), app_identifier, barrier_closure));
+    UnsubscribeBecausePermissionRevoked(app_identifier, barrier_closure);

[Peter Beverloo, 2016/04/21 13:26:19]

Can we coalesce UnsubscribeBecausePermissionRevoked() into this method? It's
only three additional statements.

[johnme, 2016/05/25 14:11:51]

Not any more - I had to re-add the async GetSenderId call.

   }
 }
 
 void PushMessagingServiceImpl::UnsubscribeBecausePermissionRevoked(
     const PushMessagingAppIdentifier& app_identifier,
-    const base::Closure& closure,
-    const std::string& sender_id,
-    bool success,
-    bool not_found) {
+    const base::Closure& closure) {
   base::Closure barrier_closure = base::BarrierClosure(2, closure);
 
-  // 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.
-  // TODO(johnme): Auto-unregister before SW DB is cleared
-  // (https://crbug.com/402458).

[Peter Beverloo, 2016/04/21 13:26:19]

Now that we no longer rely on the sender_id I think this will actually enable us
to do something sensible in response to
ServiceWorkerContextObserver::OnRegistrationDeleted() and
ServiceWorkerContextObserver::OnStorageWiped() \o/

[johnme, 2016/05/25 14:11:51]

Yup - the future is bright! Though unfortunately, we still need the sender_id to
unsubscribe legacy GCM registrations on Android, so this isn't quite as simple.

-  Unsubscribe(app_identifier.app_id(), sender_id,
+  Unsubscribe(app_identifier.app_id(),
               base::Bind(&UnregisterCallbackToClosure, barrier_closure));
 
   // Clear the associated service worker push registration id.
   ClearPushSubscriptionID(profile_, app_identifier.origin(),
                           app_identifier.service_worker_registration_id(),
                           barrier_closure);
 }
 
 void PushMessagingServiceImpl::SetContentSettingChangedCallbackForTesting(
     const base::Closure& callback) {
@@ skipping 46 matching lines @@
          blink::WebPushPermissionStatusGranted;
 }
 
 gcm::GCMDriver* PushMessagingServiceImpl::GetGCMDriver() const {
   gcm::GCMProfileService* gcm_profile_service =
       gcm::GCMProfileServiceFactory::GetForProfile(profile_);
   CHECK(gcm_profile_service);
   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();
+}