Move push event dispatching out of ServiceWorkerVersion.

content/browser/push_messaging/push_messaging_router.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_router.h"
 
 #include <string>
 
 #include "base/bind.h"
 #include "content/browser/service_worker/service_worker_context_wrapper.h"
 #include "content/browser/service_worker/service_worker_registration.h"
 #include "content/browser/service_worker/service_worker_storage.h"
+#include "content/common/service_worker/service_worker_messages.h"
 #include "content/common/service_worker/service_worker_status_code.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/storage_partition.h"
 
 namespace content {
 
 namespace {
 
 void RunDeliverCallback(
     const PushMessagingRouter::DeliverMessageCallback& deliver_message_callback,
     PushDeliveryStatus delivery_status) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       base::Bind(deliver_message_callback, delivery_status));
 }
 
 }  // namespace
 
 // static
 void PushMessagingRouter::DeliverMessage(
     BrowserContext* browser_context,
@@ skipping 48 matching lines @@
   ServiceWorkerVersion* version = service_worker_registration->active_version();
   DCHECK(version);
 
   // Hold on to the service worker registration in the callback to keep it
   // alive until the callback dies. Otherwise the registration could be
   // released when this method returns - before the event is delivered to the
   // service worker.
   base::Callback<void(ServiceWorkerStatusCode)> dispatch_event_callback =
       base::Bind(&PushMessagingRouter::DeliverMessageEnd,
                  deliver_message_callback, service_worker_registration);
-
-  version->DispatchPushEvent(dispatch_event_callback, data);
+  version->RunAfterStartWorker(

[johnme, 2016/01/22 14:35:37]

Nit: If you made DispatchSimpleEvent automatically handle the
RunAfterStartWorker part, this could be even simpler:

ServiceWorkerVersion
    ::DispatchSimpleEvent<ServiceWorkerMsg_PushEvent,
                          ServiceWorkerHostMsg_PushEventFinished,
                          std::string>(
    version, ServiceWorkerMetrics::EventType::PUSH,
    dispatch_event_callback, data);

[Marijn Kruisselbrink, 2016/01/23 00:02:19]

A prerequisite for that merge would be to merge RunAfterStartWorker with
StartRequest. And if/when those two are merged (as in StartRequest can function
without an already running worker) there really is no benefit anymore in merging
StartRequest with DispatchEvent since then you could just synchronously call one
after the other while SWVersion does all the correct queueing.
That is something I'd love to do, but I'd prefer to wait until after I finished
migrating all the events to use StartRequest so I can first simplify the request
tracking in SWVersion to not have to care about multiple request types anymore.

+      base::Bind(base::IgnoreResult(
+                     &ServiceWorkerVersion::DispatchSimpleEvent<
+                         ServiceWorkerMsg_PushEvent,
+                         ServiceWorkerHostMsg_PushEventFinished, std::string>),
+                 version, ServiceWorkerMetrics::EventType::PUSH,
+                 dispatch_event_callback, data),
+      dispatch_event_callback);
 }
 
 // static
 void PushMessagingRouter::DeliverMessageEnd(
     const DeliverMessageCallback& deliver_message_callback,
     const scoped_refptr<ServiceWorkerRegistration>& service_worker_registration,
     ServiceWorkerStatusCode service_worker_status) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   // TODO(mvanouwerkerk): UMA logging.
   PushDeliveryStatus delivery_status =
@@ skipping 27 matching lines @@
     case SERVICE_WORKER_ERROR_MAX_VALUE:
       NOTREACHED() << "Got unexpected error code: " << service_worker_status
                    << " " << ServiceWorkerStatusToString(service_worker_status);
       delivery_status = PUSH_DELIVERY_STATUS_SERVICE_WORKER_ERROR;
       break;
   }
   RunDeliverCallback(deliver_message_callback, delivery_status);
 }
 
 }  // namespace content