InstallEvent.addRoutes is resolving/rejecting the returned promise synchronously #1742

Activity

1. yoshisatoyanagisawa commented on Jan 21, 2025

   

   yoshisatoyanagisawa

   on Jan 21, 2025

   Collaborator

   More actions

   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.
2. youennf commented on Jan 22, 2025

   

   youennf

   on Jan 22, 2025

   Author

   More actions

   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.
3. yoshisatoyanagisawa commented on Jan 22, 2025

   

   yoshisatoyanagisawa

   on Jan 22, 2025

   Collaborator

   More actions

   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.
4. yoshisatoyanagisawa commented on Feb 4, 2025

   

   yoshisatoyanagisawa

   on Feb 4, 2025

   Collaborator

   More actions

   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?