[ServiceWorker] Mojofy PushEvent of Service Worker.

content/renderer/service_worker/service_worker_context_client.cc

Index: content/renderer/service_worker/service_worker_context_client.cc
diff --git a/content/renderer/service_worker/service_worker_context_client.cc b/content/renderer/service_worker/service_worker_context_client.cc
index 7226478159e96bbad5ccde8c7ad661adb51dee32..4edec49847d85ce67482f9d6b829b3181d7ccc3f 100644
--- a/content/renderer/service_worker/service_worker_context_client.cc
+++ b/content/renderer/service_worker/service_worker_context_client.cc
@@ -179,6 +179,8 @@ struct ServiceWorkerContextClient::WorkerContextData {
   using SkipWaitingCallbacksMap =
       IDMap<std::unique_ptr<blink::WebServiceWorkerSkipWaitingCallbacks>>;
   using SyncEventCallbacksMap = IDMap<std::unique_ptr<const SyncCallback>>;
+  using PushEventCallbacksMap =
+      IDMap<std::unique_ptr<const DispatchPushEventCallback>>;
   using FetchEventCallbacksMap = IDMap<std::unique_ptr<const FetchCallback>>;
   using ExtendableMessageEventCallbacksMap =
       IDMap<std::unique_ptr<const DispatchExtendableMessageEventCallback>>;
@@ -211,6 +213,9 @@ struct ServiceWorkerContextClient::WorkerContextData {
   // Pending callbacks for Background Sync Events.
   SyncEventCallbacksMap sync_event_callbacks;
 
+  // Pending callbacks for Push Events.
+  PushEventCallbacksMap push_event_callbacks;
+
   // Pending callbacks for Fetch Events.
   FetchEventCallbacksMap fetch_event_callbacks;
 
@@ -406,7 +411,6 @@ void ServiceWorkerContextClient::OnMessageReceived(
                         OnNotificationClickEvent)
     IPC_MESSAGE_HANDLER(ServiceWorkerMsg_NotificationCloseEvent,
                         OnNotificationCloseEvent)
-    IPC_MESSAGE_HANDLER(ServiceWorkerMsg_PushEvent, OnPushEvent)
     IPC_MESSAGE_HANDLER(ServiceWorkerMsg_DidGetClient, OnDidGetClient)
     IPC_MESSAGE_HANDLER(ServiceWorkerMsg_DidGetClients, OnDidGetClients)
     IPC_MESSAGE_HANDLER(ServiceWorkerMsg_OpenWindowResponse,
@@ -574,6 +578,12 @@ void ServiceWorkerContextClient::willDestroyWorkerContext(
        !it.IsAtEnd(); it.Advance()) {
     it.GetCurrentValue()->Run(SERVICE_WORKER_ERROR_ABORT, base::Time::Now());
   }
+  // Aborts the all pending push event callbacks.
+  for (WorkerContextData::PushEventCallbacksMap::iterator it(
+           &context_->push_event_callbacks);
+       !it.IsAtEnd(); it.Advance()) {
+    it.GetCurrentValue()->Run(SERVICE_WORKER_ERROR_ABORT, base::Time::Now());
+  }
   // Aborts the all pending fetch event callbacks.
   for (WorkerContextData::FetchEventCallbacksMap::iterator it(
            &context_->fetch_event_callbacks);
@@ -761,9 +771,18 @@ void ServiceWorkerContextClient::didHandlePushEvent(
     int request_id,
     blink::WebServiceWorkerEventResult result,
     double event_dispatch_time) {
-  Send(new ServiceWorkerHostMsg_PushEventFinished(
-      GetRoutingID(), request_id, result,
-      base::Time::FromDoubleT(event_dispatch_time)));
+  const DispatchPushEventCallback* callback =
+      context_->push_event_callbacks.Lookup(request_id);
+  if (!callback)
+    return;

[Peter Beverloo, 2016/12/13 13:31:32]

Should this have a LOG(WARN)? Maybe even a DCHECK? In which scenario would we
end up acknowledging that a push event that doesn't exist anymore has finished?

[xiaofengzhang, 2016/12/15 01:30:49]

Done.

+  if (result == blink::WebServiceWorkerEventResultCompleted) {

[Peter Beverloo, 2016/12/13 13:31:32]

I notice that this reverses the logic for determining the |status|.
Previously[1] we would always return |SERVICE_WORKER_OK|, and only return
|SERVICE_WORKER_ERROR_EVENT_WAITUNTIL_REJECTED| when the |result| was set to
|blink::WebServiceWorkerEventResultRejected|.

Since there are only two entries in the |WebServiceWorkerEventResult| enum
that's probably OK, but it becomes significant if a third entry ever gets added.
The best option would be to use a switch() so that we get compile time
verification, but that may end up being verbose. I'll defer to the OWNERS of
this file.

[1]
https://cs.chromium.org/chromium/src/content/browser/service_worker/service_w...

+    callback->Run(SERVICE_WORKER_OK,
+                  base::Time::FromDoubleT(event_dispatch_time));
+  } else {
+    callback->Run(SERVICE_WORKER_ERROR_EVENT_WAITUNTIL_REJECTED,
+                  base::Time::FromDoubleT(event_dispatch_time));

[Peter Beverloo, 2016/12/13 13:31:32]

nit: prefer avoiding multiple calls to the same method. Something like the
following ought to work:

    ServiceWorkerStatusCode status =
        result == blink::WebServiceWorkerEventResultCompleted
            ? SERVICE_WORKER_OK
            : SERVICE_WORKER_ERROR_EVENT_WAITUNTIL_REJECTED;

    callback->Run(status, ...);

[xiaofengzhang, 2016/12/15 01:30:49]

Done. Thanks :-)

+  }
+  context_->push_event_callbacks.Remove(request_id);
 }
 
 void ServiceWorkerContextClient::didHandleSyncEvent(
@@ -1063,10 +1082,14 @@ void ServiceWorkerContextClient::OnNotificationCloseEvent(
       ToWebNotificationData(notification_data));
 }
 
-void ServiceWorkerContextClient::OnPushEvent(int request_id,
-                                             const PushEventPayload& payload) {
+void ServiceWorkerContextClient::DispatchPushEvent(
+    const PushEventPayload& payload,
+    const DispatchPushEventCallback& callback) {
   TRACE_EVENT0("ServiceWorker",
-               "ServiceWorkerContextClient::OnPushEvent");
+               "ServiceWorkerContextClient::DispatchPushEvent");
+  int request_id = context_->push_event_callbacks.Add(
+      base::MakeUnique<DispatchPushEventCallback>(callback));
+
   // Only set data to be a valid string if the payload had decrypted data.
   blink::WebString data;
   if (!payload.is_null)