Issue 801036  has been merged into this issue.

S13nServiceWorker: make sure the worker is started when ControllerServiceWorker interface ptr is queried or sent by/to the clients.

Currently clients (are going to) have two ways to get the interface ptr to ControllerServiceWorker:

1. Clients can call SWContainerHost::GetControllerServiceWorker() to get the interface ptr. The method is implemented by SWProviderHost in the browser process, and clients usually call it via ControllerServiceWorkerConnector (during subresource loading).

2. Clients can get a new controller ptr when the browser calls SWContainer::SetController() after https://chromium-review.googlesource.com/c/chromium/src/+/742961 lands. The method is called by SWProviderHost in the browser process, and implemented by SWProviderContext in the renderer.

In both cases we should make sure the service worker is running or is starting up, so that the interface ptr is going to work (or connection error happens if the request is dropped).

Here is a quick fix:
https://chromium-review.googlesource.com/c/chromium/src/+/981842

If I understand how SWProviderContext::SetController() works correctly, I think this covers both 1. and 2. in comment 2, as it stores ControllerSerivceWorker interface ptr in ControllerServiceWorkerConnector.

That said I'm not sure this is a reasonable approach because:
- Passing ServiceWorkerMetrics::EventType::UNKNOWN is unfortunate.
- The name GetControllerServiceWorker() doesn't sound doing some additional work like starting a worker.

An alternative approach might be adding a new method to ServiceWorkerContainerHost, e.g. StartControllerServiceWorker(EventType event_type).

Or just changing GetControllerServiceWorker() to take EventType would be enough?

kinuko@, falken@: what do you think? 

Yeah good points. I think something like the patch should do what's needed.

Reg: naming how about if we rename this like EnsureControllerServiceWorker()?

The EventType will always be FETCH for now, but I agree that it's probably better if we could give an event type there.  Either by converting all the EventType into mojo or by defining a small one that just have kFetch to start with.

Thank you for the suggestion. EnsureControllerServiceWorker sounds good to me. Looks like the approach is reasonable so I've updated the CL for actual review.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b722d21967bda4c00eaa2d7e340b507dfe1ae749

commit b722d21967bda4c00eaa2d7e340b507dfe1ae749
Author: Kenichi Ishibashi <bashi@chromium.org>
Date: Thu Mar 29 23:50:11 2018

S13nSW: Make sure SW is started when getting ControllerServiceWorker

When getting a controller service worker, clients expect that the
controller is running. This CL makes sure the controller is running
when a client queries ControllerServiceWorker via
ControllerServiceWorkerConnector.

Note that there is another path to get ControllerServiceWorker, i.e.,
ServiceWorkerProviderHost::GetControllerServiceWorkerPtr(). This CL
doesn't update it yet because we need to change it as an async method.
A follow-up CL will update it.

Bug:  797222 
Change-Id: I9b3df8e87d4c184266083041de6a3e888efd9413
Reviewed-on: https://chromium-review.googlesource.com/981842
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547028}
[modify] https://crrev.com/b722d21967bda4c00eaa2d7e340b507dfe1ae749/content/browser/service_worker/embedded_worker_instance_unittest.cc
[modify] https://crrev.com/b722d21967bda4c00eaa2d7e340b507dfe1ae749/content/browser/service_worker/service_worker_provider_host.cc
[modify] https://crrev.com/b722d21967bda4c00eaa2d7e340b507dfe1ae749/content/browser/service_worker/service_worker_provider_host.h
[modify] https://crrev.com/b722d21967bda4c00eaa2d7e340b507dfe1ae749/content/common/service_worker/service_worker_container.mojom
[modify] https://crrev.com/b722d21967bda4c00eaa2d7e340b507dfe1ae749/content/renderer/service_worker/controller_service_worker_connector.cc
[modify] https://crrev.com/b722d21967bda4c00eaa2d7e340b507dfe1ae749/content/renderer/service_worker/controller_service_worker_connector.h
[modify] https://crrev.com/b722d21967bda4c00eaa2d7e340b507dfe1ae749/content/renderer/service_worker/service_worker_subresource_loader.cc
[modify] https://crrev.com/b722d21967bda4c00eaa2d7e340b507dfe1ae749/content/renderer/service_worker/service_worker_subresource_loader_unittest.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b7661721d3ae6085748873a7495d87af3cbe0825

commit b7661721d3ae6085748873a7495d87af3cbe0825
Author: Kenichi Ishibashi <bashi@chromium.org>
Date: Tue Apr 03 07:02:24 2018

S13nSW: GetControllerServiceWorkerPtr() doesn't ensure the controller is running

The method is similar to EnsureControllerServiceWorker but in some cases we
can't make sure that the controller service worker is running (as existing
comment described). Update the description of the method to state that this
method doesn't guarantee that the controller is running.

The method will be called during navigation when S13nSW is enabled. During
navigation, we need to make sure that the controller is running before
calling this method. Currently we make sure this by calling
ServiceWorkerFetchDispatcher::StartWorker() in the following sequence:

ServiceWorkerControlleeRequestHandler::MaybeCreateLoader()
-> ServiceWorkerURLJobWrapper::ForwardToServiceWorker()
-> ServiceWorkerNavigationLoader::ForwardToServiceWorker()
-> ServiceWorkerFetchDispatcher::Run()
-> ServiceWorkerFetchDispatcher::StartWorker()

After creating a loader, we call GetControllerServiceWorkerPtr() in
ServiceWorkerControlleeReuqestHandler::MaybeCreateSubresourceLoaderParams().

Bug:  797222 
Change-Id: If7ef6dbe2e59c15e2f737f16d68f0c851d889556
Reviewed-on: https://chromium-review.googlesource.com/987832
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547634}
[modify] https://crrev.com/b7661721d3ae6085748873a7495d87af3cbe0825/content/browser/service_worker/service_worker_provider_host.cc
[modify] https://crrev.com/b7661721d3ae6085748873a7495d87af3cbe0825/content/browser/service_worker/service_worker_provider_host.h

Let me close this for now. We covered all cases we have at this point. Issue 827935 tracks what we can do to make sure the worker is running in general.