Partial implementation of subscription restrictions.

content/browser/push_messaging/push_messaging_message_filter.cc

Index: content/browser/push_messaging/push_messaging_message_filter.cc
diff --git a/content/browser/push_messaging/push_messaging_message_filter.cc b/content/browser/push_messaging/push_messaging_message_filter.cc
index 844b7242e7a8be252064b4677b0600e4de07de11..be6a601566d0178e53c8457937af66b69bb05f93 100644
--- a/content/browser/push_messaging/push_messaging_message_filter.cc
+++ b/content/browser/push_messaging/push_messaging_message_filter.cc
@@ -9,6 +9,7 @@
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
+#include "base/command_line.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/metrics/histogram.h"
@@ -27,12 +28,14 @@
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/child_process_host.h"
 #include "content/public/common/console_message_level.h"
+#include "content/public/common/content_switches.h"
 #include "content/public/common/push_messaging_status.h"
 #include "third_party/WebKit/public/platform/modules/push_messaging/WebPushPermissionStatus.h"
 
 namespace content {
 
 const char kPushSenderIdServiceWorkerKey[] = "push_sender_id";
+const char kPushSenderKeyServiceWorkerKey[] = "push_sender_public_key";
 const char kPushRegistrationIdServiceWorkerKey[] = "push_registration_id";
 
 namespace {
@@ -250,8 +253,7 @@ bool PushMessagingMessageFilter::OnMessageReceived(
 void PushMessagingMessageFilter::OnSubscribeFromDocument(
     int render_frame_id,
     int request_id,
-    const std::string& sender_id,
-    bool user_visible,
+    const PushSubscriptionOptions& options,
     int64_t service_worker_registration_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   // TODO(mvanouwerkerk): Validate arguments?
@@ -259,7 +261,7 @@ void PushMessagingMessageFilter::OnSubscribeFromDocument(
   data.request_id = request_id;
   data.service_worker_registration_id = service_worker_registration_id;
   data.render_frame_id = render_frame_id;
-  data.user_visible = user_visible;
+  data.user_visible = options.user_visible_only;
 
   ServiceWorkerRegistration* service_worker_registration =
       service_worker_context_->GetLiveRegistration(
@@ -271,22 +273,29 @@ void PushMessagingMessageFilter::OnSubscribeFromDocument(
   }
   data.requesting_origin = service_worker_registration->pattern().GetOrigin();
 
+  if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableExperimentalWebPlatformFeatures))
+    DCHECK(!options.using_public_key);
+
+  const std::string key = options.using_public_key
+                              ? kPushSenderKeyServiceWorkerKey
+                              : kPushSenderIdServiceWorkerKey;

[Peter Beverloo, 2016/02/18 11:39:43]

Why would we store the values differently?

Theoretically a Web developer can already pass a applicationServerKey in the
gcm_sender_id field of the manifest (which is of type string). I don't know why
that wouldn't have to work?

[harkness, 2016/02/22 15:40:43]

Done.

   service_worker_context_->StoreRegistrationUserData(
-      service_worker_registration_id, data.requesting_origin,
-      kPushSenderIdServiceWorkerKey, sender_id,
-      base::Bind(&PushMessagingMessageFilter::DidPersistSenderId,
-                 weak_factory_io_to_io_.GetWeakPtr(), data, sender_id));
+      service_worker_registration_id, data.requesting_origin, key,
+      options.sender_info,
+      base::Bind(&PushMessagingMessageFilter::DidPersistSenderInfo,
+                 weak_factory_io_to_io_.GetWeakPtr(), data, options));
 }
 
 void PushMessagingMessageFilter::OnSubscribeFromWorker(
     int request_id,
     int64_t service_worker_registration_id,
-    bool user_visible) {
+    const PushSubscriptionOptions& options) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   RegisterData data;
   data.request_id = request_id;
   data.service_worker_registration_id = service_worker_registration_id;
-  data.user_visible = user_visible;
+  data.user_visible = options.user_visible_only;
 
   ServiceWorkerRegistration* service_worker_registration =
       service_worker_context_->GetLiveRegistration(
@@ -297,34 +306,35 @@ void PushMessagingMessageFilter::OnSubscribeFromWorker(
   }
   data.requesting_origin = service_worker_registration->pattern().GetOrigin();
 
-  // This sender_id will be ignored; instead it will be fetched from storage.
-  CheckForExistingRegistration(data, std::string() /* sender_id */);
+  // This sender_info in options will be ignored; instead it will be fetched

[Peter Beverloo, 2016/02/18 11:39:43]

This is not correct anymore, if |options.sender_info| is set the developer
subscribed with an explicit applicationServerKey, and we should be using that.

This raises an interesting question that I asked on the GitHub issue as well:
what if it's different? :-)

[harkness, 2016/02/22 15:40:43]

I updated the comment to be correct. My assumption in the implementation is that
I will only look in the registration if I don't already have a sender_info.
However, that means that if there's a bad sender_info in the registration, I'm
not updating it. Essentially, that would mean that the only way to get rid of an
outdated registration would be to unregister and re-register with the new valid
ID. The only drawback we could do would be to overwrite any time there is a
register, but that's a lot of extra IO work for us to do if we do it on every
registration.

+  // from storage.
+  CheckForExistingRegistration(data, PushSubscriptionOptions());
 }
 
-void PushMessagingMessageFilter::DidPersistSenderId(
+void PushMessagingMessageFilter::DidPersistSenderInfo(
     const RegisterData& data,
-    const std::string& sender_id,
+    const PushSubscriptionOptions& options,
     ServiceWorkerStatusCode service_worker_status) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   if (service_worker_status != SERVICE_WORKER_OK)
     SendSubscriptionError(data, PUSH_REGISTRATION_STATUS_STORAGE_ERROR);
   else
-    CheckForExistingRegistration(data, sender_id);
+    CheckForExistingRegistration(data, options);
 }
 
 void PushMessagingMessageFilter::CheckForExistingRegistration(
     const RegisterData& data,
-    const std::string& sender_id) {
+    const PushSubscriptionOptions& options) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   service_worker_context_->GetRegistrationUserData(
       data.service_worker_registration_id, kPushRegistrationIdServiceWorkerKey,
       base::Bind(&PushMessagingMessageFilter::DidCheckForExistingRegistration,
-                 weak_factory_io_to_io_.GetWeakPtr(), data, sender_id));
+                 weak_factory_io_to_io_.GetWeakPtr(), data, options));
 }
 
 void PushMessagingMessageFilter::DidCheckForExistingRegistration(
     const RegisterData& data,
-    const std::string& sender_id,
+    const PushSubscriptionOptions& options,
     const std::string& push_registration_id,
     ServiceWorkerStatusCode service_worker_status) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
@@ -349,12 +359,20 @@ void PushMessagingMessageFilter::DidCheckForExistingRegistration(
     BrowserThread::PostTask(
         BrowserThread::UI, FROM_HERE,
         base::Bind(&Core::RegisterOnUI, base::Unretained(ui_core_.get()), data,
-                   sender_id));
+                   options.sender_info));
   } else {
-    service_worker_context_->GetRegistrationUserData(
-        data.service_worker_registration_id, kPushSenderIdServiceWorkerKey,
-        base::Bind(&PushMessagingMessageFilter::DidGetSenderIdFromStorage,
-                   weak_factory_io_to_io_.GetWeakPtr(), data));
+    if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
+            switches::kEnableExperimentalWebPlatformFeatures))
+      service_worker_context_->GetRegistrationUserData(
+          data.service_worker_registration_id, kPushSenderIdServiceWorkerKey,
+          base::Bind(&PushMessagingMessageFilter::DidGetSenderIdFromStorage,
+                     weak_factory_io_to_io_.GetWeakPtr(), data));
+    else
+      // Only check for the sender public key if the feature is enabled.
+      service_worker_context_->GetRegistrationUserData(
+          data.service_worker_registration_id, kPushSenderKeyServiceWorkerKey,
+          base::Bind(&PushMessagingMessageFilter::DidGetSenderKeyFromStorage,
+                     weak_factory_io_to_io_.GetWeakPtr(), data));
   }
 }
 
@@ -375,6 +393,25 @@ void PushMessagingMessageFilter::DidGetEncryptionKeys(
                           push_registration_id, p256dh, auth);
 }
 
+void PushMessagingMessageFilter::DidGetSenderKeyFromStorage(
+    const RegisterData& data,
+    const std::string& sender_key,
+    ServiceWorkerStatusCode service_worker_status) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  if (service_worker_status != SERVICE_WORKER_OK)
+    // A failure might just mean that the service worker is registered with a
+    // sender id instead of the public key. Try to get that data.
+    service_worker_context_->GetRegistrationUserData(
+        data.service_worker_registration_id, kPushSenderIdServiceWorkerKey,
+        base::Bind(&PushMessagingMessageFilter::DidGetSenderIdFromStorage,
+                   weak_factory_io_to_io_.GetWeakPtr(), data));
+  else
+    BrowserThread::PostTask(
+        BrowserThread::UI, FROM_HERE,
+        base::Bind(&Core::RegisterOnUI, base::Unretained(ui_core_.get()), data,
+                   sender_key));
+}
+
 void PushMessagingMessageFilter::DidGetSenderIdFromStorage(
     const RegisterData& data,
     const std::string& sender_id,
@@ -392,7 +429,7 @@ void PushMessagingMessageFilter::DidGetSenderIdFromStorage(
 
 void PushMessagingMessageFilter::Core::RegisterOnUI(
     const PushMessagingMessageFilter::RegisterData& data,
-    const std::string& sender_id) {
+    const std::string& sender_info) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   PushMessagingService* push_service = service();
   if (!push_service) {
@@ -438,16 +475,18 @@ void PushMessagingMessageFilter::Core::RegisterOnUI(
     return;
   }
 
+  PushSubscriptionOptions options;
+  options.user_visible_only = data.user_visible;
+  options.sender_info = sender_info;
   if (data.FromDocument()) {
     push_service->SubscribeFromDocument(
-        data.requesting_origin, data.service_worker_registration_id, sender_id,
-        render_process_id_, data.render_frame_id, data.user_visible,
+        data.requesting_origin, data.service_worker_registration_id,
+        render_process_id_, data.render_frame_id, options,
         base::Bind(&Core::DidRegister, weak_factory_ui_to_ui_.GetWeakPtr(),
                    data));
   } else {
     push_service->SubscribeFromWorker(
-        data.requesting_origin, data.service_worker_registration_id, sender_id,
-        data.user_visible,
+        data.requesting_origin, data.service_worker_registration_id, options,
         base::Bind(&Core::DidRegister, weak_factory_ui_to_ui_.GetWeakPtr(),
                    data));
   }