Prohibit ServiceWorker event handler update after the initialization. #1161
Conversation
|
@jakearchibald @LeszekSwirski |
To understand the effect of whatwg/dom#1161, let me add use counters to count updates of handlers prohibited by this proposal. Bug: 1347319 Change-Id: Ic6a19a512222dabca8e52416fdab9629ce9a70ad Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4259225 Commit-Queue: Yoshisato Yanagisawa <yyanagisawa@chromium.org> Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Reviewed-by: Minoru Chikamune <chikamune@chromium.org> Reviewed-by: Shunya Shishido <sisidovski@chromium.org> Cr-Commit-Position: refs/heads/main@{#1106099}
To understand the effect of whatwg/dom#1161, let me add use counters to count updates of handlers prohibited by this proposal. (cherry picked from commit c49519f) Bug: 1347319 Change-Id: Ic6a19a512222dabca8e52416fdab9629ce9a70ad Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4259225 Commit-Queue: Yoshisato Yanagisawa <yyanagisawa@chromium.org> Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Reviewed-by: Minoru Chikamune <chikamune@chromium.org> Reviewed-by: Shunya Shishido <sisidovski@chromium.org> Cr-Original-Commit-Position: refs/heads/main@{#1106099} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4273346 Auto-Submit: Yoshisato Yanagisawa <yyanagisawa@chromium.org> Cr-Commit-Position: refs/branch-heads/5563@{#663} Cr-Branched-From: 3ac59a6-refs/heads/main@{#1097615}
|
Did you look into the history as to why this was not throwing? It's not clear to me why we are revisiting that. |
|
We're currently looking into this because of investigations into skipping the SW startup for trivial/simple handlers (in addition to the existing behaviour of skipping it for no handler). The thinking is that the possibility of dynamic changes to the set of listeners of an event will make any static analysis of the definition of "trivial" near impossible. We weren't aware of the previous discussion, sorry about that and thanks for bringing it up. We'll catch up on the history there and will see if we have any new thoughts, arguments or evidence. |
|
Past discussion is here #371 (comment) If the developer adds a service worker global event for "install", "activate", "fetch", "push" after the initial execution of the service worker, they're going to get behavior they don't expect. Worse, it might work as they expect in testing, but sometimes go wrong for real users. The goal here is to avoid this case. Right now we show a console warning, which is ok, but could it be worth tightening the rules here so issues are found in development? Option 1: ThrowI'd forgotten that we'd discussed that previously. We could limit the throwing to a subset of event types, but it seems that folks we still be against that. Option 2: IgnoreInstead of throwing, the call could be ignored. This would cause it to fail in dev, and (hopefully) cause the developer to see the console warning, and amend their code. Again, this could be limited to particular event types. In both cases, the behaviour should be applied to both event listeners and event handlers. |
|
I prefer not changing this. If we got to do this again I'd probably push harder for service workers not using events, but now that they do use events they should stick to the event model as close as possible. Having certain known "types" be ignored or throw exceptions very much goes against that. Contrived, I know, but you might very well have a custom |
This is a very early stage proposal on the specification change. I hope to listen to what you think of the change.
I will update the following checklist afterwords.
(See WHATWG Working Mode: Changes for more details.)
Preview | Diff