Create ServiceWorkerProviderHost before starting worker

content/common/service_worker/service_worker_provider.mojom

 // Copyright 2017 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 "content/common/service_worker/service_worker_provider_interfaces.mojom";
 import "content/common/service_worker/service_worker_types.mojom";
 
+// A container object carried from the browser to the renderer process.
+// This contains the params for the constructor of ServiceWorkerNetworkProvider.
+// This is passed on launching a service worker.
+struct ServiceWorkerProviderClientInfo {

[falken, 2017/06/16 15:44:47]

Can you help me understand this terminology?

A ServiceWorkerProvider means a ServiceWorkerNetworkProvider (something that
makes resource requests that the browser process wants to intercept).

Adding "Client" is Mojo terminology that means it's on the renderer side or
something? But SWNProvider is already on the renderer side.

It feels like a better name should exist here. 

I would suggest ServiceWorkerNetworkProviderInfo, but this struct is restricted
to SWNP for service worker execution contexts (i.e., "controllers" in
SWProviderHost language, which is pretty bad since SWs don't have be
controllers).

maybe ServiceWorkerNetworkProviderInfoForStartWorker?

[shimazu, 2017/06/19 03:53:30]

I originally thought that "ServiceWorkerProviderClient" is SWNetworkProvider on
the renderer and "ServiceWorkerProviderHost" is SWProviderHost on the
browser-side. Info means the information to be passed on each constructor. 
I agree that better naming should be here, but actually I was confused by the
naming around providers. 
Could you help me understand the variant of providers:
ServiceWorker"Network"Provider, ServiceWorkerProvider"Context" and
ServiceWorkerProvider"Host"?

In the aspect of mojom interface, there are only two concepts:
ServiceWorkerProvider(Client) <-> ServiceWorkerProviderHost. I think no
"Network"Provider in this layer. On the other hand, there are three relevant
classes in //content/renderer and //content/browser: SWNetworkProvider,
SWProviderContext and SWProviderHost. I assume that the counterpart of
SWProviderHost is SWNetworkProvider, but I felt it's a bit difficult to read the
relationship..
I actually tried to avoid "SWNetworkProvider" in the mojom interfaces to
simplify it. 
Do you have any good ideas for it?

[falken, 2017/06/19 05:21:43]

These are great questions. I'm hoping kinuko@ or michaeln@ can provide history
here.

I appreciate trying to simplify the terminology. I think naming is
super-important and we should be consistent and clear and mojofication is the
perfect place to get it right.

I prefer to avoid "Client" to mean something on the renderer, because the word
"Client" is already taken in a lot of places. Particularly for
ServiceWorkerProviderHost itself, we talk about "client" to mean a service
worker client as in ServiceWorkerGlobalScope#clients. We also use it to just
mean a class that uses another class, like ServiceWorkerContextClient.

In most of the Chromium codebase, XXX and XXXHost is a pair that means XXX is on
the renderer, and XXXHost is on the browser process.

Here's my understanding:
ServiceWorkerNetworkProvider: Something on the renderer that attaches to any
execution context that can make resource requests. These are service worker
clients: documents and workers; and service workers themselves. 

ServiceWorkerProviderContext: SWNetworkProvider HAS_A SWProviderContext. This is
an abstract class helper that lets us have different behavior between SW clients
and SW themselves. I think the meaning of "Context" here is to emphasize the
different behavior based on the type execution context (client or SW).

ServiceWorkerProviderHost: The browser-side of ServiceWorkerNetworkProvider. The
naming asymmetry has historical reasons.

I suspect originally NetworkProvider was supposed to just be about resource
requests, but it expanded to include registration association to implement
navigator.serviceWorker.controller and self.registration.

If we're keeping the ServiceWorkerNetworkProvider naming, I still think this
should be ServiceWorkerNetworkProviderInfoForStartWorker because it's the params
to the ServiceWorkerNetworkProvider ctor when we're starting a worker. If we're
going to say ServiceWorkerNetworkProvider is misnamed and should just be
ServiceWorkerProvider, then we could call this
ServiceWorkerProviderInfoForStartWorker and rename SWNP separately.

[shimazu, 2017/06/19 06:00:39]

Thanks, ServiceWorkerProviderInfoForStartWorker sounds good for me. 
Changed the name of the struct to ServiceWorkerProviderInfoForStartWorker.

+  int32 provider_id;
+  ServiceWorkerProviderType type;
+  bool is_parent_frame_secure;

[falken, 2017/06/16 15:44:46]

I don't think |type| and |is_parent_frame_secure| should be needed. If this
struct is only used for starting a service worker, |type| must be CONTROLLER and
|is_parent_frame_secure| has no meaning, since the service worker does not have
a parent frame.

[shimazu, 2017/06/19 03:53:30]

Yes, that's right.
Removed.

+  // |registration| and |attributes| are information about the registration
+  // currently to be launched.

[falken, 2017/06/16 15:44:47]

This terminology is a little inaccurate because the registration doesn't get
launched.

|registration| and |attributes| are information about the service worker's
registration.

[falken, 2017/06/16 15:56:30]

And add "used to populate ServiceWorkerGlobalScope#registration"

[shimazu, 2017/06/19 03:53:30]

Done.

+  ServiceWorkerRegistrationObjectInfo registration;
+  ServiceWorkerVersionAttributes attributes;
+
+  associated ServiceWorkerProviderHost host_ptr_info;
+  associated ServiceWorkerProvider& client_request;
+};
+
 // A container object carried from the renderer to the browser process.
 // This contains the parameters to specify the provider on the browser side.
 // See also comments in
 // content/common/service_worker/service_worker_provider_host_info.h.
 struct ServiceWorkerProviderHostInfo {
   int32 provider_id;
   int32 route_id;
   ServiceWorkerProviderType type;
   bool is_parent_frame_secure;
   associated ServiceWorkerProviderHost& host_request;
   associated ServiceWorkerProvider client_ptr_info;
 };