Implement some follow-ups after mojofiction of Push-messaging.

content/browser/push_messaging/push_messaging_manager.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/browser/push_messaging/push_messaging_manager.h"
 
 #include <string>
 #include <vector>
 
 #include "base/bind.h"
@@ skipping 97 matching lines @@
 
 }  // namespace
 
 struct PushMessagingManager::RegisterData {
   RegisterData();
   RegisterData(const RegisterData& other) = default;
   bool FromDocument() const;
   GURL requesting_origin;
   int64_t service_worker_registration_id;
   PushSubscriptionOptions options;
+  SubscribeCallback callback;

[Peter Beverloo, 2017/02/23 13:37:39]

This will be copied, but since the callback's bind state is ref counted that's
fine.

[ke.he, 2017/02/24 03:24:30]

Thanks!

   // The following member should only be read if FromDocument() is true.
   int render_frame_id;
 };
 
 // Inner core of the PushMessagingManager which lives on the UI thread.
 class PushMessagingManager::Core {
  public:
   Core(const base::WeakPtr<PushMessagingManager>& io_parent,
        int render_process_id);
 
   // Public Register methods on UI thread --------------------------------------
 
   // Called via PostTask from IO thread.
-  void RegisterOnUI(const SubscribeCallback& callback,
-                    const RegisterData& data);
+  void RegisterOnUI(const RegisterData& data);
 
   // Public Unregister methods on UI thread ------------------------------------
 
   // Called via PostTask from IO thread.
   void UnregisterFromService(const UnsubscribeCallback& callback,
                              int64_t service_worker_registration_id,
                              const GURL& requesting_origin,
                              const std::string& sender_id);
 
   // Public GetPermission methods on UI thread ---------------------------------
@@ skipping 20 matching lines @@
   PushMessagingService* service();
 
  private:
   friend struct BrowserThread::DeleteOnThread<BrowserThread::UI>;
   friend class base::DeleteHelper<Core>;
 
   ~Core();
 
   // Private Register methods on UI thread -------------------------------------
 
-  void DidRequestPermissionInIncognito(const SubscribeCallback& callback,
-                                       const RegisterData& data,
+  void DidRequestPermissionInIncognito(const RegisterData& data,
                                        blink::mojom::PermissionStatus status);
 
-  void DidRegister(const SubscribeCallback& callback,
-                   const RegisterData& data,
+  void DidRegister(const RegisterData& data,
                    const std::string& push_registration_id,
                    const std::vector<uint8_t>& p256dh,
                    const std::vector<uint8_t>& auth,
                    PushRegistrationStatus status);
 
   // Private Unregister methods on UI thread -----------------------------------
 
   void DidUnregisterFromService(const UnsubscribeCallback& callback,
                                 int64_t service_worker_registration_id,
                                 PushUnregistrationStatus unregistration_status);
@@ skipping 70 matching lines @@
                                      const PushSubscriptionOptions& options,
                                      const SubscribeCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   // TODO(mvanouwerkerk): Validate arguments?
   RegisterData data;
 
   // Will be ChildProcessHost::kInvalidUniqueID in requests from Service Worker.
   data.render_frame_id = render_frame_id;
 
   data.service_worker_registration_id = service_worker_registration_id;
+  data.callback = callback;
   data.options = options;
 
   ServiceWorkerRegistration* service_worker_registration =
       service_worker_context_->GetLiveRegistration(
           data.service_worker_registration_id);
   if (!service_worker_registration ||
       !service_worker_registration->active_version()) {
-    SendSubscriptionError(callback, data,
-                          PUSH_REGISTRATION_STATUS_NO_SERVICE_WORKER);
+    SendSubscriptionError(data, PUSH_REGISTRATION_STATUS_NO_SERVICE_WORKER);
     return;
   }
   data.requesting_origin = service_worker_registration->pattern().GetOrigin();
 
   DCHECK(!(data.options.sender_info.empty() && data.FromDocument()));
 
   service_worker_context_->GetRegistrationUserData(
       data.service_worker_registration_id,
       {kPushRegistrationIdServiceWorkerKey, kPushSenderIdServiceWorkerKey},
       base::Bind(&PushMessagingManager::DidCheckForExistingRegistration,
-                 weak_factory_io_to_io_.GetWeakPtr(), callback, data));
+                 weak_factory_io_to_io_.GetWeakPtr(), data));
 }
 
 void PushMessagingManager::DidCheckForExistingRegistration(
-    const SubscribeCallback& callback,
     const RegisterData& data,
     const std::vector<std::string>& push_registration_id_and_sender_id,
     ServiceWorkerStatusCode service_worker_status) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   if (service_worker_status == SERVICE_WORKER_OK) {
     DCHECK_EQ(2u, push_registration_id_and_sender_id.size());
     const auto& push_registration_id = push_registration_id_and_sender_id[0];
     const auto& stored_sender_id = push_registration_id_and_sender_id[1];
     std::string fixed_sender_id =
         FixSenderInfo(data.options.sender_info, stored_sender_id);
     if (fixed_sender_id.empty()) {
-      SendSubscriptionError(callback, data,
-                            PUSH_REGISTRATION_STATUS_NO_SENDER_ID);
+      SendSubscriptionError(data, PUSH_REGISTRATION_STATUS_NO_SENDER_ID);
       return;
     }
     if (fixed_sender_id != stored_sender_id) {
-      SendSubscriptionError(callback, data,
-                            PUSH_REGISTRATION_STATUS_SENDER_ID_MISMATCH);
+      SendSubscriptionError(data, PUSH_REGISTRATION_STATUS_SENDER_ID_MISMATCH);
       return;
     }
-    auto callback_ui = base::Bind(&PushMessagingManager::DidGetEncryptionKeys,
-                                  weak_factory_io_to_io_.GetWeakPtr(), callback,
-                                  data, push_registration_id);
+    auto callback = base::Bind(&PushMessagingManager::DidGetEncryptionKeys,
+                               weak_factory_io_to_io_.GetWeakPtr(), data,
+                               push_registration_id);
     BrowserThread::PostTask(
         BrowserThread::UI, FROM_HERE,
         base::Bind(&Core::GetEncryptionInfoOnUI,
                    base::Unretained(ui_core_.get()), data.requesting_origin,
                    data.service_worker_registration_id, fixed_sender_id,
-                   callback_ui));
+                   callback));
     return;
   }
   // TODO(johnme): The spec allows the register algorithm to reject with an
   // AbortError when accessing storage fails. Perhaps we should do that if
   // service_worker_status != SERVICE_WORKER_ERROR_NOT_FOUND instead of
   // attempting to do a fresh registration?
   // https://w3c.github.io/push-api/#widl-PushRegistrationManager-register-Promise-PushRegistration
   if (!data.options.sender_info.empty()) {
-    BrowserThread::PostTask(
-        BrowserThread::UI, FROM_HERE,
-        base::Bind(&Core::RegisterOnUI, base::Unretained(ui_core_.get()),
-                   callback, data));
+    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                            base::Bind(&Core::RegisterOnUI,
+                                       base::Unretained(ui_core_.get()), data));
   } else {
     // There is no existing registration and the sender_info passed in was
     // empty, but perhaps there is a stored sender id we can use.
     service_worker_context_->GetRegistrationUserData(
         data.service_worker_registration_id, {kPushSenderIdServiceWorkerKey},
         base::Bind(&PushMessagingManager::DidGetSenderIdFromStorage,
-                   weak_factory_io_to_io_.GetWeakPtr(), callback, data));
+                   weak_factory_io_to_io_.GetWeakPtr(), data));
   }
 }
 
 void PushMessagingManager::DidGetEncryptionKeys(
-    const SubscribeCallback& callback,
     const RegisterData& data,
     const std::string& push_registration_id,
     bool success,
     const std::vector<uint8_t>& p256dh,
     const std::vector<uint8_t>& auth) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   if (!success) {
-    SendSubscriptionError(callback, data,
+    SendSubscriptionError(data,
                           PUSH_REGISTRATION_STATUS_PUBLIC_KEY_UNAVAILABLE);
     return;
   }
 
-  SendSubscriptionSuccess(callback, data,
-                          PUSH_REGISTRATION_STATUS_SUCCESS_FROM_CACHE,
+  SendSubscriptionSuccess(data, PUSH_REGISTRATION_STATUS_SUCCESS_FROM_CACHE,
                           push_registration_id, p256dh, auth);
 }
 
 void PushMessagingManager::DidGetSenderIdFromStorage(
-    const SubscribeCallback& callback,
     const RegisterData& data,
     const std::vector<std::string>& stored_sender_id,
     ServiceWorkerStatusCode service_worker_status) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   if (service_worker_status != SERVICE_WORKER_OK) {
-    SendSubscriptionError(callback, data,
-                          PUSH_REGISTRATION_STATUS_NO_SENDER_ID);
+    SendSubscriptionError(data, PUSH_REGISTRATION_STATUS_NO_SENDER_ID);
     return;
   }
   DCHECK_EQ(1u, stored_sender_id.size());
   // We should only be here because no sender info was supplied to subscribe().
   DCHECK(data.options.sender_info.empty());
   std::string fixed_sender_id =
       FixSenderInfo(data.options.sender_info, stored_sender_id[0]);
   if (fixed_sender_id.empty()) {
-    SendSubscriptionError(callback, data,
-                          PUSH_REGISTRATION_STATUS_NO_SENDER_ID);
+    SendSubscriptionError(data, PUSH_REGISTRATION_STATUS_NO_SENDER_ID);
     return;
   }
   RegisterData mutated_data = data;
   mutated_data.options.sender_info = fixed_sender_id;
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       base::Bind(&Core::RegisterOnUI, base::Unretained(ui_core_.get()),
-                 callback, mutated_data));
+                 mutated_data));
 }
 
 void PushMessagingManager::Core::RegisterOnUI(
-    const SubscribeCallback& callback,
     const PushMessagingManager::RegisterData& data) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   PushMessagingService* push_service = service();
   if (!push_service) {
     if (!is_incognito()) {
       // This might happen if InstanceIDProfileService::IsInstanceIDEnabled
       // returns false because the Instance ID kill switch was enabled.
       // TODO(johnme): Might be better not to expose the API in this case.
       BrowserThread::PostTask(
           BrowserThread::IO, FROM_HERE,
           base::Bind(&PushMessagingManager::SendSubscriptionError, io_parent_,
-                     callback, data,
-                     PUSH_REGISTRATION_STATUS_SERVICE_NOT_AVAILABLE));
+                     data, PUSH_REGISTRATION_STATUS_SERVICE_NOT_AVAILABLE));
     } else {
       // Prevent websites from detecting incognito mode, by emulating what would
       // have happened if we had a PushMessagingService available.
       if (!data.FromDocument() || !data.options.user_visible_only) {
         // Throw a permission denied error under the same circumstances.
         BrowserThread::PostTask(
             BrowserThread::IO, FROM_HERE,
             base::Bind(&PushMessagingManager::SendSubscriptionError, io_parent_,
-                       callback, data,
+                       data,
                        PUSH_REGISTRATION_STATUS_INCOGNITO_PERMISSION_DENIED));
       } else {
         RenderFrameHost* render_frame_host =
             RenderFrameHost::FromID(render_process_id_, data.render_frame_id);
         WebContents* web_contents =
             WebContents::FromRenderFrameHost(render_frame_host);
         if (web_contents) {
           web_contents->GetMainFrame()->AddMessageToConsole(
               CONSOLE_MESSAGE_LEVEL_ERROR, kIncognitoPushUnsupportedMessage);
           // Request push messaging permission (which will fail, since
           // notifications aren't supported in incognito), so the website can't
           // detect whether incognito is active.
           web_contents->GetBrowserContext()
               ->GetPermissionManager()
               ->RequestPermission(
                   PermissionType::PUSH_MESSAGING, render_frame_host,
                   data.requesting_origin, false /* user_gesture */,
                   base::Bind(&PushMessagingManager::Core::
                                  DidRequestPermissionInIncognito,
-                             weak_factory_ui_to_ui_.GetWeakPtr(), callback,
-                             data));
+                             weak_factory_ui_to_ui_.GetWeakPtr(), data));
         }
       }
     }
     return;
   }
 
   if (data.FromDocument()) {
     push_service->SubscribeFromDocument(
         data.requesting_origin, data.service_worker_registration_id,
         render_process_id_, data.render_frame_id, data.options,
         base::Bind(&Core::DidRegister, weak_factory_ui_to_ui_.GetWeakPtr(),
-                   callback, data));
+                   data));
   } else {
     push_service->SubscribeFromWorker(
         data.requesting_origin, data.service_worker_registration_id,
         data.options,
         base::Bind(&Core::DidRegister, weak_factory_ui_to_ui_.GetWeakPtr(),
-                   callback, data));
+                   data));
   }
 }
 
 void PushMessagingManager::Core::DidRequestPermissionInIncognito(
-    const SubscribeCallback& callback,
     const RegisterData& data,
     blink::mojom::PermissionStatus status) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   // Notification permission should always be denied in incognito.
   DCHECK_EQ(blink::mojom::PermissionStatus::DENIED, status);
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
-      base::Bind(&PushMessagingManager::SendSubscriptionError, io_parent_,
-                 callback, data,
+      base::Bind(&PushMessagingManager::SendSubscriptionError, io_parent_, data,
                  PUSH_REGISTRATION_STATUS_INCOGNITO_PERMISSION_DENIED));
 }
 
 void PushMessagingManager::Core::DidRegister(
-    const SubscribeCallback& callback,
     const RegisterData& data,
     const std::string& push_registration_id,
     const std::vector<uint8_t>& p256dh,
     const std::vector<uint8_t>& auth,
     PushRegistrationStatus status) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   if (status == PUSH_REGISTRATION_STATUS_SUCCESS_FROM_PUSH_SERVICE) {
     BrowserThread::PostTask(
         BrowserThread::IO, FROM_HERE,
         base::Bind(&PushMessagingManager::PersistRegistrationOnIO, io_parent_,
-                   callback, data, push_registration_id, p256dh, auth));
+                   data, push_registration_id, p256dh, auth));
   } else {
     BrowserThread::PostTask(
         BrowserThread::IO, FROM_HERE,
         base::Bind(&PushMessagingManager::SendSubscriptionError, io_parent_,
-                   callback, data, status));
+                   data, status));
   }
 }
 
 void PushMessagingManager::PersistRegistrationOnIO(
-    const SubscribeCallback& callback,
     const RegisterData& data,
     const std::string& push_registration_id,
     const std::vector<uint8_t>& p256dh,
     const std::vector<uint8_t>& auth) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   service_worker_context_->StoreRegistrationUserData(
       data.service_worker_registration_id, data.requesting_origin,
       {{kPushRegistrationIdServiceWorkerKey, push_registration_id},
        {kPushSenderIdServiceWorkerKey, data.options.sender_info}},
       base::Bind(&PushMessagingManager::DidPersistRegistrationOnIO,
-                 weak_factory_io_to_io_.GetWeakPtr(), callback, data,
+                 weak_factory_io_to_io_.GetWeakPtr(), data,
                  push_registration_id, p256dh, auth));
 }
 
 void PushMessagingManager::DidPersistRegistrationOnIO(
-    const SubscribeCallback& callback,
     const RegisterData& data,
     const std::string& push_registration_id,
     const std::vector<uint8_t>& p256dh,
     const std::vector<uint8_t>& auth,
     ServiceWorkerStatusCode service_worker_status) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   if (service_worker_status == SERVICE_WORKER_OK) {
-    SendSubscriptionSuccess(callback, data,
+    SendSubscriptionSuccess(data,
                             PUSH_REGISTRATION_STATUS_SUCCESS_FROM_PUSH_SERVICE,
                             push_registration_id, p256dh, auth);
   } else {
     // TODO(johnme): Unregister, so PushMessagingServiceImpl can decrease count.
-    SendSubscriptionError(callback, data,
-                          PUSH_REGISTRATION_STATUS_STORAGE_ERROR);
+    SendSubscriptionError(data, PUSH_REGISTRATION_STATUS_STORAGE_ERROR);
   }
 }
 
 void PushMessagingManager::SendSubscriptionError(
-    const SubscribeCallback& callback,
     const RegisterData& data,
     PushRegistrationStatus status) {
   // Only called from IO thread, but would be safe to call from UI thread.
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  callback.Run(status, base::nullopt /* endpoint */,
-               base::nullopt /* options */, base::nullopt /* p256dh */,
-               base::nullopt /* auth */);
+  DCHECK(data.callback);

[Peter Beverloo, 2017/02/23 13:37:39]

nit: I wouldn't add this DCHECK, it's really uncommon to DCHECK that a callback
is non-null. I think we can rely on it? (The Run() method will also check this.)

[ke.he, 2017/02/24 03:24:30]

Done. (and elsewhere)

+  data.callback.Run(status, base::nullopt /* endpoint */,
+                    base::nullopt /* options */, base::nullopt /* p256dh */,
+                    base::nullopt /* auth */);
   RecordRegistrationStatus(status);
 }
 
 void PushMessagingManager::SendSubscriptionSuccess(
-    const SubscribeCallback& callback,
     const RegisterData& data,
     PushRegistrationStatus status,
     const std::string& push_subscription_id,
     const std::vector<uint8_t>& p256dh,
     const std::vector<uint8_t>& auth) {
   // Only called from IO thread, but would be safe to call from UI thread.
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   if (!service_available_) {
     // This shouldn't be possible in incognito mode, since we've already checked
     // that we have an existing registration. Hence it's ok to throw an error.
     DCHECK(!ui_core_->is_incognito());
-    SendSubscriptionError(callback, data,
-                          PUSH_REGISTRATION_STATUS_SERVICE_NOT_AVAILABLE);
+    SendSubscriptionError(data, PUSH_REGISTRATION_STATUS_SERVICE_NOT_AVAILABLE);
     return;
   }
 
   const GURL endpoint = CreateEndpoint(
       IsApplicationServerKey(data.options.sender_info), push_subscription_id);
 
-  callback.Run(status, endpoint, data.options, p256dh, auth);
+  DCHECK(data.callback);
+  data.callback.Run(status, endpoint, data.options, p256dh, auth);
 
   RecordRegistrationStatus(status);
 }
 
 // Unsubscribe methods on both IO and UI threads, merged in order of use from
 // PushMessagingManager and Core.
 // -----------------------------------------------------------------------------
 
 void PushMessagingManager::Unsubscribe(int64_t service_worker_registration_id,
                                        const UnsubscribeCallback& callback) {
@@ skipping 331 matching lines @@
 PushMessagingService* PushMessagingManager::Core::service() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   RenderProcessHost* process_host =
       RenderProcessHost::FromID(render_process_id_);
   return process_host
              ? process_host->GetBrowserContext()->GetPushMessagingService()
              : nullptr;
 }
 
 }  // namespace content