Partial implementation of subscription restrictions.

content/renderer/push_messaging/push_messaging_dispatcher.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 "content/renderer/push_messaging/push_messaging_dispatcher.h"
 
 #include "base/strings/utf_string_conversions.h"
 #include "content/child/service_worker/web_service_worker_registration_impl.h"
 #include "content/common/push_messaging_messages.h"
 #include "content/renderer/manifest/manifest_manager.h"
@@ skipping 26 matching lines @@
   IPC_END_MESSAGE_MAP()
   return handled;
 }
 
 void PushMessagingDispatcher::subscribe(
     blink::WebServiceWorkerRegistration* service_worker_registration,
     const blink::WebPushSubscriptionOptions& options,
     blink::WebPushSubscriptionCallbacks* callbacks) {
   DCHECK(service_worker_registration);
   DCHECK(callbacks);
-  RenderFrameImpl::FromRoutingID(routing_id())
-      ->manifest_manager()
-      ->GetManifest(base::Bind(
-          &PushMessagingDispatcher::DoSubscribe, base::Unretained(this),
-          service_worker_registration, options, callbacks));
+  // If a developer provided an application server key in |options|, skip
+  // fetching the manifest.
+  if (options.applicationServerKey.isEmpty()) {
+    RenderFrameImpl::FromRoutingID(routing_id())
+        ->manifest_manager()
+        ->GetManifest(base::Bind(
+            &PushMessagingDispatcher::DidGetManifest, base::Unretained(this),
+            service_worker_registration, options, callbacks));
+  } else {

[Peter Beverloo, 2016/02/23 14:28:29]

nit: Chrome prefers early returns, so you could rewrite this as:

===============
  if (options.applicationServerKey.isEmpty()) {
    ...
    return;
  }

  ...
===============

[harkness, 2016/02/26 11:44:26]

Actually, in this case, I feel like the different format makes the code less
readable. Do you feel strongly about the change?

[Peter Beverloo, 2016/02/26 16:02:32]

No :-)

+    PushSubscriptionOptions content_options;
+    content_options.user_visible_only = options.userVisibleOnly;
+    content_options.sender_info = options.applicationServerKey.utf8();
+    DoSubscribe(service_worker_registration, content_options, callbacks);
+  }
+}
+
+void PushMessagingDispatcher::DidGetManifest(
+    blink::WebServiceWorkerRegistration* service_worker_registration,
+    const blink::WebPushSubscriptionOptions& options,
+    blink::WebPushSubscriptionCallbacks* callbacks,
+    const Manifest& manifest) {
+  int request_id = subscription_callbacks_.Add(callbacks);
+  // Get the sender_info from the manifest since it wasn't provided by
+  // the caller.
+  if (manifest.IsEmpty()) {
+    OnSubscribeFromDocumentError(
+        request_id, PUSH_REGISTRATION_STATUS_MANIFEST_EMPTY_OR_MISSING);
+    return;
+  }
+
+  PushSubscriptionOptions content_options;
+  content_options.user_visible_only = options.userVisibleOnly;
+  content_options.sender_info =

[Peter Beverloo, 2016/02/23 14:28:29]

nit: Since this has got a default value of empty already, you could get rid of
the ternary by writing this as:

===============
  if (!manifest.gcm_sender_id.is_null()) {
    content_options.sender_info =
        base::UTF16ToUTF8(manifest.gcm_sender_id.string());
  }
===============

[harkness, 2016/02/26 11:44:26]

Done.

+      manifest.gcm_sender_id.is_null()
+          ? std::string()
+          : base::UTF16ToUTF8(manifest.gcm_sender_id.string());
+
+  DoSubscribe(service_worker_registration, content_options, callbacks);
 }
 
 void PushMessagingDispatcher::DoSubscribe(
     blink::WebServiceWorkerRegistration* service_worker_registration,
-    const blink::WebPushSubscriptionOptions& options,
-    blink::WebPushSubscriptionCallbacks* callbacks,
-    const Manifest& manifest) {
+    const PushSubscriptionOptions& options,
+    blink::WebPushSubscriptionCallbacks* callbacks) {
   int request_id = subscription_callbacks_.Add(callbacks);
   int64_t service_worker_registration_id =
       static_cast<WebServiceWorkerRegistrationImpl*>(
           service_worker_registration)
           ->registration_id();
 
-  if (manifest.IsEmpty()) {
-    OnSubscribeFromDocumentError(
-        request_id, PUSH_REGISTRATION_STATUS_MANIFEST_EMPTY_OR_MISSING);
-    return;
-  }
-
-  std::string sender_id =
-      manifest.gcm_sender_id.is_null()
-          ? std::string()
-          : base::UTF16ToUTF8(manifest.gcm_sender_id.string());
-  if (sender_id.empty()) {
+  if (options.sender_info.empty()) {
     OnSubscribeFromDocumentError(request_id,
                                  PUSH_REGISTRATION_STATUS_NO_SENDER_ID);
     return;
   }
-
   Send(new PushMessagingHostMsg_SubscribeFromDocument(
-      routing_id(), request_id,
-      manifest.gcm_sender_id.is_null()
-          ? std::string()
-          : base::UTF16ToUTF8(manifest.gcm_sender_id.string()),
-      options.userVisibleOnly, service_worker_registration_id));
+      routing_id(), request_id, options, service_worker_registration_id));
 }
 
 void PushMessagingDispatcher::OnSubscribeFromDocumentSuccess(
     int32_t request_id,
     const GURL& endpoint,
     const std::vector<uint8_t>& p256dh,
     const std::vector<uint8_t>& auth) {
   blink::WebPushSubscriptionCallbacks* callbacks =
       subscription_callbacks_.Lookup(request_id);
   DCHECK(callbacks);
@@ skipping 17 matching lines @@
           : blink::WebPushError::ErrorTypeAbort;
 
   callbacks->onError(blink::WebPushError(
       error_type,
       blink::WebString::fromUTF8(PushRegistrationStatusToString(status))));
 
   subscription_callbacks_.Remove(request_id);
 }
 
 }  // namespace content