Push API: Validate storage before returning cached subscriptions

content/browser/push_messaging/push_messaging_message_filter.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_message_filter.h"
 
 #include <string>
 #include <vector>
 
 #include "base/bind.h"
@@ skipping 33 matching lines @@
 
 namespace {
 
 // Chrome currently does not support the Push API in incognito.
 const char kIncognitoPushUnsupportedMessage[] =
     "Chrome currently does not support the Push API in incognito mode "
     "(https://crbug.com/401439). There is deliberately no way to "
     "feature-detect this, since incognito mode needs to be undetectable by "
     "websites.";
 
-// These UMA methods are only called from IO thread, but it would be acceptable
-// (even though slightly racy) to call them from UI thread as well, see
+// These UMA methods are called from the IO and/or UI threads. Racey but ok, see
 // https://groups.google.com/a/chromium.org/d/msg/chromium-dev/FNzZRJtN2aw/Aw0CWAXJJ1kJ
 void RecordRegistrationStatus(PushRegistrationStatus status) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO) ||
+         BrowserThread::CurrentlyOn(BrowserThread::UI));
   UMA_HISTOGRAM_ENUMERATION("PushMessaging.RegistrationStatus", status,
                             PUSH_REGISTRATION_STATUS_LAST + 1);
 }
-
 void RecordUnregistrationStatus(PushUnregistrationStatus status) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   UMA_HISTOGRAM_ENUMERATION("PushMessaging.UnregistrationStatus", status,
                             PUSH_UNREGISTRATION_STATUS_LAST + 1);
 }
-
 void RecordGetRegistrationStatus(PushGetRegistrationStatus status) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO) ||
+         BrowserThread::CurrentlyOn(BrowserThread::UI));
   UMA_HISTOGRAM_ENUMERATION("PushMessaging.GetRegistrationStatus", status,
                             PUSH_GETREGISTRATION_STATUS_LAST + 1);
 }
 
-// Curries the |success| and |p256dh| parameters over to |callback| and
-// posts a task to invoke |callback| on the IO thread.
-void ForwardEncryptionInfoToIOThreadProxy(
-    const PushMessagingService::EncryptionInfoCallback& callback,
-    bool success,
-    const std::vector<uint8_t>& p256dh,
-    const std::vector<uint8_t>& auth) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
-                          base::Bind(callback, success, p256dh, auth));
+void UnregisterCallbackToClosure(const base::Closure& closure,
+                                 PushUnregistrationStatus status) {
+  DCHECK(!closure.is_null());
+  closure.Run();
 }
 
 // Returns whether |sender_info| contains a valid application server key, that
 // is, a NIST P-256 public key in uncompressed format.
 bool IsApplicationServerKey(const std::string& sender_info) {
   return sender_info.size() == 65 && sender_info[0] == 0x04;
 }
 
 // Returns sender_info if non-empty, otherwise checks if stored_sender_id
 // may be used as a fallback and if so, returns stored_sender_id instead.
@@ skipping 28 matching lines @@
 };
 
 // Inner core of this message filter which lives on the UI thread.
 class PushMessagingMessageFilter::Core {
  public:
   Core(const base::WeakPtr<PushMessagingMessageFilter>& io_parent,
        int render_process_id);
 
   // Public Register methods on UI thread --------------------------------------
 
+  // Callback called on UI thread.
+  void SubscribeDidGetInfoOnUI(const RegisterData& data,
+                               const std::string& push_subscription_id,
+                               const std::string& sender_id,
+                               bool is_valid,
+                               const std::vector<uint8_t>& p256dh,
+                               const std::vector<uint8_t>& auth);
+
   // Called via PostTask from IO thread.
   void RegisterOnUI(const RegisterData& data);
 
   // Public Unregister methods on UI thread ------------------------------------
 
   // Called via PostTask from IO thread.
   void UnregisterFromService(int request_id,
                              int64_t service_worker_registration_id,
                              const GURL& requesting_origin,
                              const std::string& sender_id);
 
+  // Public GetSubscription methods on UI thread -------------------------------
+
+  // Callback called on UI thread.
+  void GetSubscriptionDidGetInfoOnUI(int request_id,
+                                     const GURL& origin,
+                                     int64_t service_worker_registration_id,
+                                     const GURL& endpoint,
+                                     const std::string& sender_info,
+                                     bool is_valid,
+                                     const std::vector<uint8_t>& p256dh,
+                                     const std::vector<uint8_t>& auth);
+
+  // Callback called on UI thread.
+  void GetSubscriptionDidUnsubscribe(
+      int request_id,
+      PushGetRegistrationStatus get_status,
+      PushUnregistrationStatus unsubscribe_status);
+
   // Public GetPermission methods on UI thread ---------------------------------
 
   // Called via PostTask from IO thread.
   void GetPermissionStatusOnUI(const GURL& requesting_origin,
                                bool user_visible,
                                int request_id);
 
   // Public helper methods on UI thread ----------------------------------------
 
-  // Called via PostTask from IO thread. The |io_thread_callback| callback
-  // will be invoked on the IO thread.
-  void GetEncryptionInfoOnUI(
+  // Called via PostTask from IO thread. |callback| will be run on UI thread.
+  void GetSubscriptionInfoOnUI(
       const GURL& origin,
       int64_t service_worker_registration_id,
       const std::string& sender_id,
-      const PushMessagingService::EncryptionInfoCallback& io_thread_callback);
+      const std::string& push_subscription_id,
+      const PushMessagingService::SubscriptionInfoCallback& callback);
 
   // Called (directly) from both the UI and IO threads.
   bool is_incognito() const { return is_incognito_; }
 
   // Returns a push messaging service. May return null.
   PushMessagingService* service();
 
+  // Returns a weak ptr. Must only be called from the outer class's constructor.
+  base::WeakPtr<Core> GetWeakPtrFromIOParentConstructor();
+
  private:
   friend struct BrowserThread::DeleteOnThread<BrowserThread::UI>;
   friend class base::DeleteHelper<Core>;
 
   ~Core();
 
   // Private Register methods on UI thread -------------------------------------
 
   void DidRequestPermissionInIncognito(const RegisterData& data,
                                        blink::mojom::PermissionStatus status);
@@ skipping 55 matching lines @@
     : BrowserMessageFilter(PushMessagingMsgStart),
       service_worker_context_(service_worker_context),
       weak_factory_io_to_io_(this) {
   // Although this class is used only on the IO thread, it is constructed on UI.
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   // Normally, it would be unsafe to obtain a weak pointer from the UI thread,
   // but it's ok in the constructor since we can't be destroyed before our
   // constructor finishes.
   ui_core_.reset(
       new Core(weak_factory_io_to_io_.GetWeakPtr(), render_process_id));
+  ui_core_weak_ptr_ = ui_core_->GetWeakPtrFromIOParentConstructor();
 
   PushMessagingService* service = ui_core_->service();
   service_available_ = !!service;
 
   if (service_available_) {
     default_endpoint_ = service->GetEndpoint(false /* standard_protocol */);
     web_push_protocol_endpoint_ =
         service->GetEndpoint(true /* standard_protocol */);
   }
 }
@@ skipping 57 matching lines @@
                  weak_factory_io_to_io_.GetWeakPtr(), data));
 }
 
 void PushMessagingMessageFilter::DidCheckForExistingRegistration(
     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& push_subscription_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(data, PUSH_REGISTRATION_STATUS_NO_SENDER_ID);
       return;
     }
     if (fixed_sender_id != stored_sender_id) {
       SendSubscriptionError(data, PUSH_REGISTRATION_STATUS_SENDER_ID_MISMATCH);
       return;
     }
-    auto callback = base::Bind(
-        &PushMessagingMessageFilter::DidGetEncryptionKeys,
-        weak_factory_io_to_io_.GetWeakPtr(), data, push_registration_id);
     BrowserThread::PostTask(
         BrowserThread::UI, FROM_HERE,
-        base::Bind(&Core::GetEncryptionInfoOnUI,
+        base::Bind(&Core::GetSubscriptionInfoOnUI,
                    base::Unretained(ui_core_.get()), data.requesting_origin,
                    data.service_worker_registration_id, fixed_sender_id,
-                   callback));
+                   push_subscription_id,
+                   base::Bind(&Core::SubscribeDidGetInfoOnUI, ui_core_weak_ptr_,
+                              data, push_subscription_id, fixed_sender_id)));
     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()), 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(&PushMessagingMessageFilter::DidGetSenderIdFromStorage,
                    weak_factory_io_to_io_.GetWeakPtr(), data));
   }
 }
 
-void PushMessagingMessageFilter::DidGetEncryptionKeys(
+void PushMessagingMessageFilter::Core::SubscribeDidGetInfoOnUI(
     const RegisterData& data,
-    const std::string& push_registration_id,
-    bool success,
+    const std::string& push_subscription_id,
+    const std::string& sender_id,
+    bool is_valid,
     const std::vector<uint8_t>& p256dh,
     const std::vector<uint8_t>& auth) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  if (!success) {
-    SendSubscriptionError(
-        data, PUSH_REGISTRATION_STATUS_PUBLIC_KEY_UNAVAILABLE);
-    return;
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  if (is_valid) {
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(&PushMessagingMessageFilter::SendSubscriptionSuccess,
+                   io_parent_, data,
+                   PUSH_REGISTRATION_STATUS_SUCCESS_FROM_CACHE,
+                   push_subscription_id, p256dh, auth));
+  } else {
+    // Uh-oh! Although there was a cached subscription in the Service Worker
+    // database, it did not have matching counterparts in the
+    // PushMessagingAppIdentifier map and/or GCM Store. Unsubscribe and
+    // re-subscribe to fix this inconsistency.
+
+    // Consider this subscription attempt to have failed. The re-subscribe will
+    // be logged to UMA as a separate subscription attempt.
+    RecordRegistrationStatus(PUSH_REGISTRATION_STATUS_STORAGE_CORRUPT);
+
+    PushMessagingService* push_service = service();
+    DCHECK(push_service);  // NOT_FOUND response can only come from service.

[Peter Beverloo, 2017/03/20 23:50:13]

Core::GetSubscriptionInfoOnUI specifically returns is_valid==false for cases
where there isn't a Push Service. That seems to be in conflict with this
comment?

[johnme, 2017/03/30 18:36:38]

Hmm yeah. In practice it shouldn't be possible to have a stored push
subscription in a profile that doesn't have a push service though. I kept the
DCHECK (well, NOTREACHED), but added a fallback for release builds that returns
PUSH_REGISTRATION_STATUS_PUBLIC_KEY_UNAVAILABLE, in case some refactoring or
edge case (kill switch?) makes that stop being true.

+    push_service->Unsubscribe(
+        PUSH_UNREGISTRATION_REASON_SUBSCRIBE_STORAGE_CORRUPT,
+        data.requesting_origin, data.service_worker_registration_id, sender_id,
+        base::Bind(&UnregisterCallbackToClosure,
+                   base::Bind(IgnoreResult(&BrowserThread::PostTask),
+                              BrowserThread::IO, FROM_HERE,
+                              base::Bind(&PushMessagingMessageFilter::
+                                             DidCheckForExistingRegistration,
+                                         io_parent_, data,
+                                         // push_registration_id_and_sender_id
+                                         std::vector<std::string>(),
+                                         SERVICE_WORKER_ERROR_NOT_FOUND))));

[Peter Beverloo, 2017/03/20 23:50:13]

This is deep. Like, literally. Can we maybe make it slightly less deep by using
anonymous functions to bridge?

[johnme, 2017/03/30 18:36:38]

Done.

   }
-
-  SendSubscriptionSuccess(data, PUSH_REGISTRATION_STATUS_SUCCESS_FROM_CACHE,
-                          push_registration_id, p256dh, auth);
 }
 
 void PushMessagingMessageFilter::DidGetSenderIdFromStorage(
     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(data, PUSH_REGISTRATION_STATUS_NO_SENDER_ID);
     return;
@@ skipping 242 matching lines @@
     DCHECK(!is_incognito());
     BrowserThread::PostTask(
         BrowserThread::IO, FROM_HERE,
         base::Bind(&PushMessagingMessageFilter::DidUnregister, io_parent_,
                    request_id,
                    PUSH_UNREGISTRATION_STATUS_SERVICE_NOT_AVAILABLE));
     return;
   }
 
   push_service->Unsubscribe(
-      requesting_origin, service_worker_registration_id, sender_id,
+      PUSH_UNREGISTRATION_REASON_JAVASCRIPT_API, requesting_origin,
+      service_worker_registration_id, sender_id,
       base::Bind(&Core::DidUnregisterFromService,
                  weak_factory_ui_to_ui_.GetWeakPtr(), request_id,
                  service_worker_registration_id));
 }
 
 void PushMessagingMessageFilter::Core::DidUnregisterFromService(
     int request_id,
     int64_t service_worker_registration_id,
     PushUnregistrationStatus unregistration_status) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
@@ skipping 53 matching lines @@
     int request_id,
     int64_t service_worker_registration_id,
     const std::vector<std::string>& push_subscription_id_and_sender_info,
     ServiceWorkerStatusCode service_worker_status) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   PushGetRegistrationStatus get_status =
       PUSH_GETREGISTRATION_STATUS_STORAGE_ERROR;
   switch (service_worker_status) {
     case SERVICE_WORKER_OK: {
       DCHECK_EQ(2u, push_subscription_id_and_sender_info.size());
+      const std::string& push_subscription_id =
+          push_subscription_id_and_sender_info[0];
+      const std::string& sender_info = push_subscription_id_and_sender_info[1];
 
       if (!service_available_) {
         // Return not found in incognito mode, so websites can't detect it.
         get_status =
             ui_core_->is_incognito()
                 ? PUSH_GETREGISTRATION_STATUS_INCOGNITO_REGISTRATION_NOT_FOUND
                 : PUSH_GETREGISTRATION_STATUS_SERVICE_NOT_AVAILABLE;
         break;
       }
 
       ServiceWorkerRegistration* registration =
           service_worker_context_->GetLiveRegistration(
               service_worker_registration_id);
       const GURL origin = registration->pattern().GetOrigin();
 
-      const bool uses_standard_protocol =
-          IsApplicationServerKey(push_subscription_id_and_sender_info[1]);
-      const GURL endpoint = CreateEndpoint(
-          uses_standard_protocol, push_subscription_id_and_sender_info[0]);
-
-      auto callback =
-          base::Bind(&PushMessagingMessageFilter::DidGetSubscriptionKeys,
-                     weak_factory_io_to_io_.GetWeakPtr(), request_id, endpoint,
-                     push_subscription_id_and_sender_info[1]);
+      const bool uses_standard_protocol = IsApplicationServerKey(sender_info);
+      const GURL endpoint =
+          CreateEndpoint(uses_standard_protocol, push_subscription_id);
 
       BrowserThread::PostTask(
           BrowserThread::UI, FROM_HERE,
-          base::Bind(&Core::GetEncryptionInfoOnUI,
+          base::Bind(&Core::GetSubscriptionInfoOnUI,
                      base::Unretained(ui_core_.get()), origin,
-                     service_worker_registration_id,
-                     push_subscription_id_and_sender_info[1], callback));
+                     service_worker_registration_id, sender_info,
+                     push_subscription_id,
+                     base::Bind(&Core::GetSubscriptionDidGetInfoOnUI,
+                                ui_core_weak_ptr_, request_id, origin,
+                                service_worker_registration_id, endpoint,
+                                sender_info)));
 
       return;
     }
     case SERVICE_WORKER_ERROR_NOT_FOUND: {
       get_status = PUSH_GETREGISTRATION_STATUS_REGISTRATION_NOT_FOUND;
       break;
     }
     case SERVICE_WORKER_ERROR_FAILED: {
       get_status = PUSH_GETREGISTRATION_STATUS_STORAGE_ERROR;
       break;
@@ skipping 18 matching lines @@
       NOTREACHED() << "Got unexpected error code: " << service_worker_status
                    << " " << ServiceWorkerStatusToString(service_worker_status);
       get_status = PUSH_GETREGISTRATION_STATUS_STORAGE_ERROR;
       break;
     }
   }
   Send(new PushMessagingMsg_GetSubscriptionError(request_id, get_status));
   RecordGetRegistrationStatus(get_status);
 }
 
-void PushMessagingMessageFilter::DidGetSubscriptionKeys(
+void PushMessagingMessageFilter::Core::GetSubscriptionDidGetInfoOnUI(
     int request_id,
+    const GURL& origin,
+    int64_t service_worker_registration_id,
     const GURL& endpoint,
     const std::string& sender_info,
-    bool success,
+    bool is_valid,
     const std::vector<uint8_t>& p256dh,
     const std::vector<uint8_t>& auth) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  if (!success) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  if (is_valid) {
+    PushSubscriptionOptions options;
+    // Chrome rejects subscription requests with userVisibleOnly false, so it
+    // must have been true. TODO(harkness): If Chrome starts accepting silent
+    // push subscriptions with userVisibleOnly false, the bool will need to be
+    // stored.
+    options.user_visible_only = true;
+    options.sender_info = sender_info;
+
+    Send(new PushMessagingMsg_GetSubscriptionSuccess(request_id, endpoint,
+                                                     options, p256dh, auth));
+
+    RecordGetRegistrationStatus(PUSH_GETREGISTRATION_STATUS_SUCCESS);
+  } else {
+    // Uh-oh! Although there was a cached subscription in the Service Worker
+    // database, it did not have matching counterparts in the
+    // PushMessagingAppIdentifier map and/or GCM Store. Unsubscribe to fix this
+    // inconsistency.
     PushGetRegistrationStatus status =
-        PUSH_GETREGISTRATION_STATUS_PUBLIC_KEY_UNAVAILABLE;
+        PUSH_GETREGISTRATION_STATUS_STORAGE_CORRUPT;
 
-    Send(new PushMessagingMsg_GetSubscriptionError(request_id, status));
+    PushMessagingService* push_service = service();
+    DCHECK(push_service);  // NOT_FOUND response can only come from service.
+    push_service->Unsubscribe(
+        PUSH_UNREGISTRATION_REASON_GET_SUBSCRIPTION_STORAGE_CORRUPT, origin,
+        service_worker_registration_id, sender_info,
+        base::Bind(&Core::GetSubscriptionDidUnsubscribe,
+                   weak_factory_ui_to_ui_.GetWeakPtr(), request_id, status));
 
     RecordGetRegistrationStatus(status);
-    return;
   }
+}
 
-  PushSubscriptionOptions options;
-  // Chrome rejects subscription requests with userVisibleOnly false, so it must
-  // have been true. TODO(harkness): If Chrome starts accepting silent push
-  // subscriptions with userVisibleOnly false, the bool will need to be stored.
-  options.user_visible_only = true;
-  options.sender_info = sender_info;
-
-  Send(new PushMessagingMsg_GetSubscriptionSuccess(request_id, endpoint,
-                                                   options, p256dh, auth));
-
-  RecordGetRegistrationStatus(PUSH_GETREGISTRATION_STATUS_SUCCESS);
+void PushMessagingMessageFilter::Core::GetSubscriptionDidUnsubscribe(
+    int request_id,
+    PushGetRegistrationStatus get_status,
+    PushUnregistrationStatus unsubscribe_status) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  Send(new PushMessagingMsg_GetSubscriptionError(request_id, get_status));
 }
 
 // GetPermission methods on both IO and UI threads, merged in order of use from
 // PushMessagingMessageFilter and Core.
 // -----------------------------------------------------------------------------
 
 void PushMessagingMessageFilter::OnGetPermissionStatus(
     int request_id,
     int64_t service_worker_registration_id,
     bool user_visible) {
@@ skipping 37 matching lines @@
     return;
   }
   Send(new PushMessagingMsg_GetPermissionStatusSuccess(request_id,
                                                        permission_status));
 }
 
 // Helper methods on both IO and UI threads, merged from
 // PushMessagingMessageFilter and Core.
 // -----------------------------------------------------------------------------
 
-void PushMessagingMessageFilter::Core::GetEncryptionInfoOnUI(
+void PushMessagingMessageFilter::Core::GetSubscriptionInfoOnUI(
     const GURL& origin,
     int64_t service_worker_registration_id,
     const std::string& sender_id,
-    const PushMessagingService::EncryptionInfoCallback& io_thread_callback) {
+    const std::string& push_subscription_id,
+    const PushMessagingService::SubscriptionInfoCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   PushMessagingService* push_service = service();
-  if (push_service) {
-    push_service->GetEncryptionInfo(
-        origin, service_worker_registration_id, sender_id,
-        base::Bind(&ForwardEncryptionInfoToIOThreadProxy, io_thread_callback));
+  if (!push_service) {
+    callback.Run(false /* is_valid */, std::vector<uint8_t>() /* p256dh */,
+                 std::vector<uint8_t>() /* auth */);
     return;
   }
 
-  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
-                          base::Bind(io_thread_callback, false /* success */,
-                                     std::vector<uint8_t>() /* p256dh */,
-                                     std::vector<uint8_t>() /* auth */));
+  push_service->GetSubscriptionInfo(origin, service_worker_registration_id,
+                                    sender_id, push_subscription_id, callback);
 }
 
 void PushMessagingMessageFilter::Core::Send(IPC::Message* message) {
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       base::Bind(&PushMessagingMessageFilter::SendIPC, io_parent_,
                  base::Passed(base::WrapUnique(message))));
 }
 
 void PushMessagingMessageFilter::SendIPC(
@@ skipping 12 matching lines @@
 
 PushMessagingService* PushMessagingMessageFilter::Core::service() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   RenderProcessHost* process_host =
       RenderProcessHost::FromID(render_process_id_);
   return process_host
              ? process_host->GetBrowserContext()->GetPushMessagingService()
              : nullptr;
 }
 
+base::WeakPtr<PushMessagingMessageFilter::Core>
+PushMessagingMessageFilter::Core::GetWeakPtrFromIOParentConstructor() {

[Peter Beverloo, 2017/03/20 23:50:13]

Would you mind summarising in two sentences or so why it's important to have
this method and cache the WeakPtr?

[johnme, 2017/03/30 18:36:38]

weak_factory_ui_to_ui_ should not be public, as it's only safe to use on the UI
thread, and the outer class lives on the IO thread. The constructor of the outer
class exceptionally runs on the UI thread, so can safely grab a weak ptr, hence
this public wrapper method that DCHECKs it's called on UI thread.

I added the following comment to ui_core_weak_ptr_ which hopefully the purpose
clearer:
  // Can be used on the IO thread as the |this| parameter when binding a
  // callback that will be called on the UI thread (an IO -> UI -> UI chain).

And clarified the comment on GetWeakPtrFromIOParentConstructor:
  // Returns a weak ptr. Must only be called on the UI thread (and hence can
  // only be called from the outer class's constructor).

+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  return weak_factory_ui_to_ui_.GetWeakPtr();
+}
+
 }  // namespace content