Re: [blink-dev] Intent to Ship Service Worker's claim, Service-Worker-Allowed header, update

12 views
Skip to first unread message

Matt Falkenhagen

unread,
Feb 7, 2015, 9:12:15 AM2/7/15
to Philip Jägenstedt, Kenji Baheux, blink-dev, xiang
Thanks for the questions!

On Sat Feb 07 2015 at 12:57:39 PM Philip Jägenstedt <phi...@opera.com> wrote:
These sound like nice additions, but I have some questions:

1. It looks like claim() isn't implemented quite per spec, which says "If the service worker is not an active worker, return a promise rejected with an "InvalidStateError" exception." Instead, the implementation seems to return a promise that won't ever be resolved in conditions which I'm not sure are equivalent. Will this be fixed before shipping?

The active worker check is performed here in ServiceWorkerVersion::OnClaimClients. The line you link to is unrelated to the check; it's a mitigation for a generic bug when a crash can occur after a worker thread is terminated. More information is on crbug.com/413518 especially this doc.

 
2. Would there be any merit to moving that active worker check into the async part of the algorithm? With media elements, the order of API calls don't matter in some places, so that e.g. one can set src attributes, add/remove source elements or call load() and have the same source be used regardless, by doing the actual selection async. It's a bit complicated, but supposedly makes the API less brittle. Is there a similar situation with claim(), i.e. can there be a situation where the script author knows the worker becomes active shortly after the call to claim() and the only thing standing in the way is that sync check at the top? (If the spec is already in a kind of consistent state in this regard, please ignore me.)

I think typically the script writer will call claim() in the 'activate' event handler, which is called when the worker becomes active. So I don't think the sync check will be a bother.
 
3. It looks ServiceWorkerRegistration.update() isn't actually implemented yet?

Correct. We can make it another intent if you'd like to review the implementation before shipping. But auto-update behavior is already implemented so I don't expect update() to be so tricky.
 
4. In the design doc, Client.url is also in bold, but that's behind the experimental ServiceWorkerClientAttributes flag. That's not a part of this Intent to Ship, right?

I think it's in bold italic, which means it's part of the other Intent to Ship "Service Worker APIs needed to support Notifications and Push API".
Reply all
Reply to author
Forward
0 new messages