InstallEvent.addRoutes is resolving/rejecting the returned promise synchronously

[youennf]

addRoutes currently returns a promise without using the ability to resolve/reject the promise asynchronously.
Looking at Chromium implementation, it seems that the promise is resolved asynchronously (probably after storing the route out of process).

The spec could be written as follows:

1. Validate parameters synchronously, reject as needed (like current spec)
2. If parameters are valid, In parallel, try storing the new route
3. If storing the route fails, queue a task to reject the promise
4. Otherwise, queue a task to resolve the promise

[yoshisatoyanagisawa]

In the Chromium implementation, Step 6 in https://w3c.github.io/ServiceWorker/#register-router-method is actually done in the browser process because the routing actually happens in the browser process for a main resource navigation. Since the browser process do not trust the renderer process (and there is mismatch of a regular expression implementation between them), the browser process verifies Step 4 and 5.

However, as you pointed out, the specification is written like as-if Step 6 is a synchronous step. I am afraid if it can be too much implementation details, but I am happy to update the specification.

[youennf]

Let's first wait on #1744, we can then put up a PR for this one.

If the browser process is doing the validation checks, is it needed to also do them in the render process as well?
Given this is observable from JS, we need to make a conscious decision here.

In specs term, it means that either we do the checks synchronously (and implementation will actually do them twice), or we run these checks inside an in parallel block.

I don't have a strong preference here.

[yoshisatoyanagisawa]

Let's first wait on #1744, we can then put up a PR for this one.

Sure.

If the browser process is doing the validation checks, is it needed to also do them in the render process as well? Given this is observable from JS, we need to make a conscious decision here.

Yeah, I also had the same question when I implemented this, and decided this way upon the following reasons:

1. especially for JavaScript code, the renderer can access more information easier than the browser process, and it might be more easy to give a precise feedback.
2. I prefer a quick feedback than a lazy feedback. Since a frequent problem should be a syntax error, telling mistakes synchronously should be better than asynchronously.
3. the install event is rarely executed. Maybe either of just after register() or update. It is also be an asynchronous process except the web developers explicitly asks. I supposed it fine to take time for its execution.
4. considering the time to set up the rules and evaluate the rules, which is already explained in the specification, web developers may not set too large rules. Then, the verification may not take so long time to execute.
5. considering the time to write to persistent storage for future navigations, verifying rules should be light-weight.

Note that in Chromium, verifying failure in the browser process will bring the renderer to be killed except for the case caused by the regular expression implementation difference.

In specs term, it means that either we do the checks synchronously (and implementation will actually do them twice), or we run these checks inside an in parallel block.

I suppose the time to verify is negligibly small and I currently do not come up with use case that the difference matters, but I prefer to keep it synchronous except for the persistent storage update.

[yoshisatoyanagisawa]

Except for the rejection due to syntax error, promise will be resolved after the list of router rules in serviceWorker has been updated.
Are there anything we need to address with this issue?