Mojoify the notificationclick and notificationclose events

content/renderer/service_worker/service_worker_context_client.cc

 // Copyright 2015 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/service_worker/service_worker_context_client.h"
 
 #include <memory>
 #include <utility>
 
 #include "base/lazy_instance.h"
@@ skipping 190 matching lines @@
       IDMap<std::unique_ptr<blink::WebServiceWorkerClientsCallbacks>>;
   using ClaimClientsCallbacksMap =
       IDMap<std::unique_ptr<blink::WebServiceWorkerClientsClaimCallbacks>>;
   using ClientCallbacksMap =
       IDMap<std::unique_ptr<blink::WebServiceWorkerClientCallbacks>>;
   using SkipWaitingCallbacksMap =
       IDMap<std::unique_ptr<blink::WebServiceWorkerSkipWaitingCallbacks>>;
   using SyncEventCallbacksMap = IDMap<std::unique_ptr<const SyncCallback>>;
   using PaymentRequestEventCallbacksMap =
       IDMap<std::unique_ptr<const PaymentRequestEventCallback>>;
+  using NotificationClickEventCallbacksMap =
+      IDMap<std::unique_ptr<const DispatchNotificationClickEventCallback>>;
+  using NotificationCloseEventCallbacksMap =
+      IDMap<std::unique_ptr<const DispatchNotificationCloseEventCallback>>;
   using PushEventCallbacksMap =
       IDMap<std::unique_ptr<const DispatchPushEventCallback>>;
   using FetchEventCallbacksMap = IDMap<std::unique_ptr<const FetchCallback>>;
   using ExtendableMessageEventCallbacksMap =
       IDMap<std::unique_ptr<const DispatchExtendableMessageEventCallback>>;
   using NavigationPreloadRequestsMap = IDMap<
       std::unique_ptr<ServiceWorkerContextClient::NavigationPreloadRequest>>;
 
   explicit WorkerContextData(ServiceWorkerContextClient* owner)
       : event_dispatcher_binding(owner),
@@ skipping 17 matching lines @@
 
   // Pending callbacks for ClaimClients().
   ClaimClientsCallbacksMap claim_clients_callbacks;
 
   // Pending callbacks for Background Sync Events.
   SyncEventCallbacksMap sync_event_callbacks;
 
   // Pending callbacks for Payment Request Events.
   PaymentRequestEventCallbacksMap payment_request_event_callbacks;
 
+  // Pending callbacks for Notification Click Events.
+  NotificationClickEventCallbacksMap notification_click_event_callbacks;
+
+  // Pending callbacks for Notification Close Events.
+  NotificationCloseEventCallbacksMap notification_close_event_callbacks;
+
   // Pending callbacks for Push Events.
   PushEventCallbacksMap push_event_callbacks;
 
   // Pending callbacks for Fetch Events.
   FetchEventCallbacksMap fetch_event_callbacks;
 
   // Pending callbacks for Extendable Message Events.
   ExtendableMessageEventCallbacksMap message_event_callbacks;
 
   // Pending navigation preload requests.
@@ skipping 146 matching lines @@
 
 void ServiceWorkerContextClient::OnMessageReceived(
     int thread_id,
     int embedded_worker_id,
     const IPC::Message& message) {
   CHECK_EQ(embedded_worker_id_, embedded_worker_id);
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(ServiceWorkerContextClient, message)
     IPC_MESSAGE_HANDLER(ServiceWorkerMsg_ActivateEvent, OnActivateEvent)
     IPC_MESSAGE_HANDLER(ServiceWorkerMsg_InstallEvent, OnInstallEvent)
-    IPC_MESSAGE_HANDLER(ServiceWorkerMsg_NotificationClickEvent,
-                        OnNotificationClickEvent)
-    IPC_MESSAGE_HANDLER(ServiceWorkerMsg_NotificationCloseEvent,
-                        OnNotificationCloseEvent)
     IPC_MESSAGE_HANDLER(ServiceWorkerMsg_DidGetClient, OnDidGetClient)
     IPC_MESSAGE_HANDLER(ServiceWorkerMsg_DidGetClients, OnDidGetClients)
     IPC_MESSAGE_HANDLER(ServiceWorkerMsg_OpenWindowResponse,
                         OnOpenWindowResponse)
     IPC_MESSAGE_HANDLER(ServiceWorkerMsg_OpenWindowError,
                         OnOpenWindowError)
     IPC_MESSAGE_HANDLER(ServiceWorkerMsg_FocusClientResponse,
                         OnFocusClientResponse)
     IPC_MESSAGE_HANDLER(ServiceWorkerMsg_NavigateClientResponse,
                         OnNavigateClientResponse)
@@ skipping 143 matching lines @@
 
 void ServiceWorkerContextClient::willDestroyWorkerContext(
     v8::Local<v8::Context> context) {
   // At this point WillStopCurrentWorkerThread is already called, so
   // worker_task_runner_->RunsTasksOnCurrentThread() returns false
   // (while we're still on the worker thread).
   proxy_ = NULL;
 
   // Aborts all the pending events callbacks.
   AbortPendingEventCallbacks(context_->sync_event_callbacks);
+  AbortPendingEventCallbacks(context_->notification_click_event_callbacks);
+  AbortPendingEventCallbacks(context_->notification_close_event_callbacks);
   AbortPendingEventCallbacks(context_->push_event_callbacks);
   AbortPendingEventCallbacks(context_->fetch_event_callbacks);
   AbortPendingEventCallbacks(context_->message_event_callbacks);
 
   // We have to clear callbacks now, as they need to be freed on the
   // same thread.
   context_.reset();
 
   // This also lets the message filter stop dispatching messages to
   // this client.
@@ skipping 124 matching lines @@
   DCHECK(callback);
   callback->Run(EventResultToStatus(result),
                 base::Time::FromDoubleT(event_dispatch_time));
   context_->fetch_event_callbacks.Remove(fetch_event_id);
 }
 
 void ServiceWorkerContextClient::didHandleNotificationClickEvent(
     int request_id,
     blink::WebServiceWorkerEventResult result,
     double event_dispatch_time) {
-  Send(new ServiceWorkerHostMsg_NotificationClickEventFinished(
-      GetRoutingID(), request_id, result,
-      base::Time::FromDoubleT(event_dispatch_time)));
+  const DispatchNotificationClickEventCallback* callback =
+      context_->notification_click_event_callbacks.Lookup(request_id);
+  DCHECK(callback);
+
+  callback->Run(result == blink::WebServiceWorkerEventResultCompleted
+                    ? SERVICE_WORKER_OK
+                    : SERVICE_WORKER_ERROR_EVENT_WAITUNTIL_REJECTED,

[shimazu, 2017/02/10 08:09:29]

You can use "EventResultToStatus(result)" here.

[Peter Beverloo, 2017/02/10 16:41:06]

Done.

+                base::Time::FromDoubleT(event_dispatch_time));
+
+  context_->notification_click_event_callbacks.Remove(request_id);
 }
 
 void ServiceWorkerContextClient::didHandleNotificationCloseEvent(
     int request_id,
     blink::WebServiceWorkerEventResult result,
     double event_dispatch_time) {
-  Send(new ServiceWorkerHostMsg_NotificationCloseEventFinished(
-      GetRoutingID(), request_id, result,
-      base::Time::FromDoubleT(event_dispatch_time)));
+  const DispatchNotificationCloseEventCallback* callback =
+      context_->notification_close_event_callbacks.Lookup(request_id);
+  DCHECK(callback);
+
+  callback->Run(result == blink::WebServiceWorkerEventResultCompleted
+                    ? SERVICE_WORKER_OK
+                    : SERVICE_WORKER_ERROR_EVENT_WAITUNTIL_REJECTED,

[shimazu, 2017/02/10 08:09:29]

ditto

[Peter Beverloo, 2017/02/10 16:41:06]

Done.

+                base::Time::FromDoubleT(event_dispatch_time));
+
+  context_->notification_close_event_callbacks.Remove(request_id);
 }
 
 void ServiceWorkerContextClient::didHandlePushEvent(
     int request_id,
     blink::WebServiceWorkerEventResult result,
     double event_dispatch_time) {
   const DispatchPushEventCallback* callback =
       context_->push_event_callbacks.Lookup(request_id);
   DCHECK(callback);
   callback->Run(result == blink::WebServiceWorkerEventResultCompleted
@@ skipping 275 matching lines @@
   webRequest.setClientId(blink::WebString::fromUTF8(request.client_id));
   webRequest.setIsReload(request.is_reload);
   if (request.fetch_type == ServiceWorkerFetchType::FOREIGN_FETCH) {
     proxy_->dispatchForeignFetchEvent(fetch_event_id, webRequest);
   } else {
     proxy_->dispatchFetchEvent(fetch_event_id, webRequest,
                                navigation_preload_sent);
   }
 }
 
-void ServiceWorkerContextClient::OnNotificationClickEvent(
-    int request_id,
+void ServiceWorkerContextClient::DispatchNotificationClickEvent(
     const std::string& notification_id,
     const PlatformNotificationData& notification_data,
     int action_index,
-    const base::NullableString16& reply) {
+    const base::Optional<base::string16>& reply,
+    const DispatchNotificationClickEventCallback& callback) {
   TRACE_EVENT0("ServiceWorker",
-               "ServiceWorkerContextClient::OnNotificationClickEvent");
+               "ServiceWorkerContextClient::DispatchNotificationClickEvent");
+
+  int request_id = context_->notification_click_event_callbacks.Add(
+      base::MakeUnique<DispatchNotificationClickEventCallback>(callback));
+
+  blink::WebString web_reply;
+  if (reply)
+    web_reply = blink::WebString::fromUTF16(reply.value());
+
   proxy_->dispatchNotificationClickEvent(
       request_id, blink::WebString::fromUTF8(notification_id),
-      ToWebNotificationData(notification_data), action_index,
-      blink::WebString::fromUTF16(reply));
+      ToWebNotificationData(notification_data), action_index, web_reply);

[shimazu, 2017/02/10 08:09:29]

I think you can use blink.mojom.Notification directly instead of
ToWebNotificationData. Using PlatformNotificationData is really straightforward,
but I'd like to have less [Native]s and like to remove _messages.h.
ConsoleMessage might be a good reference for type-mapping to a blink type.
https://cs.chromium.org/chromium/src/third_party/WebKit/public/web/console_me...

Creating another patch to introduce the typemap is also possible.
WDYT?

[Peter Beverloo, 2017/02/10 16:41:06]

This relates to the layering issue that we spoke about last week.

Keeping WebNotificationData as the type used in Blink is an interesting idea.
The type map configuration would be a bit tricky due to the mojo variants;

  - We need WebNotificationData (WTF) -> PlatformNotificationData (STL) for
Blink to Chromium.
  - We need PlatformNotificationData (STL) -> WebNotificationData (STL) for
Chromium to Blink. (Since we receive the mojo call in //content.)

Then, when onion soup comes into play, we also need PlatformNotificationData
(STL) -> WebNotificationData (WTF).

I quite like that idea - thanks! I will have to spend some time figuring out
whether the Mojo bindings like having asymmetrical conversions, so I would like
to do that in a separate patch if that's OK with you.

[shimazu, 2017/02/13 00:38:56]

Ah, sorry, I didn't understand accurately what you mentioned. The point here is
NotificationDatabaseData which is a container used under browser/ contains
PlatformNotificationData, so changing the type of argument for
DispatchNotificationClickEvent() doesn't seem reasonable.
In that case, using [Native] makes sense. 

Thanks for your kind explanation! :)

 }
 
-void ServiceWorkerContextClient::OnNotificationCloseEvent(
-    int request_id,
+void ServiceWorkerContextClient::DispatchNotificationCloseEvent(
     const std::string& notification_id,
-    const PlatformNotificationData& notification_data) {
+    const PlatformNotificationData& notification_data,
+    const DispatchNotificationCloseEventCallback& callback) {
   TRACE_EVENT0("ServiceWorker",
-               "ServiceWorkerContextClient::OnNotificationCloseEvent");
+               "ServiceWorkerContextClient::DispatchNotificationCloseEvent");
+
+  int request_id = context_->notification_close_event_callbacks.Add(
+      base::MakeUnique<DispatchNotificationCloseEventCallback>(callback));
+
   proxy_->dispatchNotificationCloseEvent(
       request_id, blink::WebString::fromUTF8(notification_id),
       ToWebNotificationData(notification_data));
 }
 
 void ServiceWorkerContextClient::DispatchPushEvent(
     const PushEventPayload& payload,
     const DispatchPushEventCallback& callback) {
   TRACE_EVENT0("ServiceWorker",
                "ServiceWorkerContextClient::DispatchPushEvent");
@@ skipping 216 matching lines @@
 }
 
 base::WeakPtr<ServiceWorkerContextClient>
 ServiceWorkerContextClient::GetWeakPtr() {
   DCHECK(worker_task_runner_->RunsTasksOnCurrentThread());
   DCHECK(context_);
   return context_->weak_factory.GetWeakPtr();
 }
 
 }  // namespace content