Inconsistencies in ServiceWorkerClients get and matchAll implementations

[youennf]

I had a look at Chrome, Firefox and Safari implementation of ServiceWorkerClients get and matchAll implementations.
They are not really aligned for fetch event resultingClientId.

For ServiceWorkerClients.get called synchronously using resultingClientId from a navigation fetch event handler:

1. Chrome is waiting for the document to be parsed before resolving the get promise. This is shown in https://wpt.fyi/results/service-workers/service-worker/controlled-iframe-postMessage.https.html where the fetch response is waiting on get promise to resolve.
2. Firefox and Safari are resolving the promise right away (I believe Firefox exposes the client URL about:blank and Safari exposes the request URL)

For ServiceWorkerClients.matchAll called synchronously from a navigation fetch event handler

1. Chrome is not exposing the resultingClientId client
2. Firefox is exposing the resultingClientId client (url is about:blank)
3. Shipping Safari is exposing the resultingClientId client (url is the request URL), though WebKit ToT recently changed to match Chrome to fix some potential issues in case of push event processing.

@asutherland, @yoshisatoyanagisawa, thoughts?

[youennf]

Digging on matchAll, it seems that setting includeUncontrolled: true will align Chrome's results with shipping safari. That is somehow surprising since the document's navigation fetch is being handled by the service worker. My understanding is that the service worker becomes the document's active service worker prior the fetch event.

Digging on get, it seems from testing that Chrome will wait for the service worker to provide a response object, but will not wait to start parsing the response body. This seems somehow consistent with the idea for get to resolve after having enqueued a task on the document's event loop, though I am not really clear when the event loop starts in the document's creation flow.

Only Firefox is exposing about:blank URLs. If calling matchAll or get, the client URL will, at some point migrate from about:blank to the fetch event request URL. I would tend to avoid exposing about:blank and instead expose the request URL.

[yoshisatoyanagisawa]

Chrome is waiting for the document to be parsed before resolving the get promise.

Yes. Chrome wait until ServiceWorkerClient is execution ready.
https://source.chromium.org/chromium/chromium/src/+/main:content/browser/service_worker/service_worker_version.cc;l=1700;drc=fc7ddc185b7f027dd7f4981ba8b2334c69a2e79c
It should be set soon after the commit navigation IPC is sent from the browser process. I understand the timing is just after getting all HTTP headers.
https://source.chromium.org/chromium/chromium/src/+/main:content/renderer/service_worker/service_worker_network_provider_for_frame.cc;l=48;drc=a45502c46d75f210c783e07384138379ea1e46e4

Chrome is not exposing the resultingClientId client

Chrome is explicitly excluding it.
No other controllee case:
https://source.chromium.org/chromium/chromium/src/+/main:content/browser/service_worker/service_worker_client_utils.cc;l=609;drc=a45502c46d75f210c783e07384138379ea1e46e4
With other controllee case:
https://source.chromium.org/chromium/chromium/src/+/main:content/browser/service_worker/service_worker_client_utils.cc;l=617;drc=a45502c46d75f210c783e07384138379ea1e46e4

What I am a bit confused for Chrome implementation is that SetControllerRegistration is called before dispatching the fetch handler.
https://source.chromium.org/chromium/chromium/src/+/main:content/browser/service_worker/service_worker_controllee_request_handler.cc;l=456;drc=a45502c46d75f210c783e07384138379ea1e46e4
Then, if there is other clients, will we see the client with the resulting client id?

@hiroshige-g recently touched the code around this area. Let me listen to your thoughts.

[asutherland]

@youennf The Firefox code is definitely trying to wait for the clients to be execution ready in both cases:

• For matchAll we exclude clients that are not execution ready
• For get Firefox waits for the execution ready promise to resolve.

How are you testing this?

[youennf]

I am testing the following case:

• Call get synchronously from a fetch event handler using resultingClientId.
• Call matchAll synchronously from a fetch event handler.

Would it help if I upload draft WPT tests to make this clearer?

[asutherland]

Would it help if I upload draft WPT tests to make this clearer?

Yes, that would be great, thank you Then I can run them under pernosco and figure out what's going on.

[youennf]

I uploaded web-platform-tests/wpt#49393 to make progress on this issue.

[youennf]

@asutherland, did you get a chance to look at the test?