[ServiceWorker] Convert ping-pong IPC into mojo interface ServiceWorkerEventDispatcher

content/common/service_worker/service_worker_event_dispatcher.mojom

 // Copyright 2016 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.
 
 module content.mojom;
 
 import "components/payments/content/payment_app.mojom";
 import "content/common/url_loader.mojom";
 import "mojo/common/string16.mojom";
 import "mojo/common/time.mojom";
@@ skipping 55 matching lines @@
   DispatchSyncEvent(string tag,
                     blink.mojom.BackgroundSyncEventLastChance last_chance)
       => (blink.mojom.ServiceWorkerEventStatus status,
           mojo.common.mojom.Time dispatch_event_time);
   DispatchPaymentRequestEvent(payments.mojom.PaymentAppRequest app_request)
       => (blink.mojom.ServiceWorkerEventStatus status,
           mojo.common.mojom.Time dispatch_event_time);
   DispatchExtendableMessageEvent(ExtendableMessageEvent event)
       => (blink.mojom.ServiceWorkerEventStatus status,
           mojo.common.mojom.Time dispatch_event_time);
+  Ping() => ();

[falken, 2017/03/21 06:54:04]

Ping() doesn't really belong here, because it's not an event. shimazu@ do you
have plans about making new interfaces for all the messages, or perhaps changing
this interface name?

Also, should we have a comment like the messages.h file had?

[shimazu, 2017/03/21 07:59:27]

My understanding is that Ping() is a method for pinging to the SWEventDispatcher
itself. I feel it's okay as is even though Ping is not an event, but I also
agree that Ping() might be confusing because it's not an event.
How about changing the method name like CheckIfResponsible() to clarify that?
In that case, I think it's better to add a blank line before this method.

 
+1 to having a comment.

[falken, 2017/03/21 14:00:45]

Hm, I think of Ping() as pinging the service worker itself. I'm not sure
ServiceWorkerEventDispatcher is the right name if it's just the interface for
IPCs sent from the browser to the service worker. Are all the existing legacy
IPC messages going to be added to ServiceWorkerEventDispatcher? How many of them
are not about dispatching events? If there are many, maybe
ServiceWorkerEventDispatcher should be renamed something like
ServiceWorkerDispatcher.

[shimazu, 2017/03/22 01:32:29]

I totally agree with that. ServiceWorkerEventDisptacher is the interface
representing the context of a service worker used from the browser process.

However, Ping() will be the only method except for Dispatch*Event
ServiceWorkerEventDispatcher will have. 
// Link to the design doc
https://docs.google.com/document/d/1xW-NXKmqKu7pQ0hTNz9rpdGXgNaItyonOySVigXdq...

ServiceWorkerEventDisptacher is possible to be considered as a counterpart of
ServiceWorkerGlobalScopeHost in the desgin doc, but I feel
ServiceWorkerEventDispatcher is clearer than "ServiceWorkerGlobalScope" or
"ServiceWorkerDispatcher" (but both have been already taken) because the
interface will have only Dispatch*Event except for Ping.

What do you think?

[falken, 2017/03/22 02:15:56]

OK, if it's the only exception I think we don't need to worry about it. I'd like
a comment above Ping() explaining what it is though, something like:

// Pings the service worker to check if it is responsive. If the callback is not
called within a certain period of time, the browser will terminate the worker.
Unlike the other functions in this interface, Ping() does not dispatch an event.

I slightly prefer Ping() over CheckIfResponsive() to align with the
ServiceWorkerVersion::Ping() naming.

 };