-
Notifications
You must be signed in to change notification settings - Fork 315
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make Client.postMessage to unloaded client not throw #1293
Conversation
cf834f0 clarified the target object can be null, and it must throw in that case. But #1291 pointed out that we cannot implement that behavior without blocking the service worker process in multi-process browser architectures. This change makes the control return right away if the target client has been unloaded. Fixes #1291.
@wanderview, does https://github.com/w3c/ServiceWorker/pull/1293/files#diff-27b79860afe28f01aed4f1f6228367faR1033 work for you? Chromium checks if the client is available when the request is getting through the browser process: https://cs.chromium.org/chromium/src/content/browser/service_worker/service_worker_version.cc?type=cs&q=ServiceWorkerVersion::PostMessageToClient&l=1155. /cc @mattto |
docs/index.bs
Outdated
1. Let |serializeWithTransferResult| be <a abstract-op>StructuredSerializeWithTransfer</a>(|message|, |transfer|). Rethrow any exceptions. | ||
1. Add a <a>task</a> that runs the following steps to |destination|'s [=ServiceWorkerContainer/client message queue=]: | ||
1. Let |targetClient| be the [=context object=]'s [=Client/service worker client=], or null if it does not exist (i.e. if the [=/service worker client=] has been [=Handle Service Worker Client Unload|unloaded=]). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what "or null if it does not exist" means. Have we defined how to check for existence? If so we should link to that. Also, is it ok to perform this existence check on the main thread?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jakearchibald, I pushed a commit to address these points. PTAL.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
cf834f0
clarified the target object can be null, and it must throw in that case.
But #1291 pointed out that we
cannot implement that behavior without blocking the service worker
process in multi-process browser architectures.
This change makes the control return right away if the target client has
been unloaded.
Fixes #1291.
Preview | Diff