Closed Bug 1187350 Opened 4 years ago Closed 4 years ago

service worker update() method should return a promise

Categories

(Core :: DOM: Service Workers, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla43
Tracking Status
firefox43 --- fixed

People

(Reporter: bkelly, Assigned: nsm)

References

Details

Attachments

(2 files)

Recent change to the spec:

  https://github.com/slightlyoff/ServiceWorker/issues/311

Note, I'm marking this v1 since the issue is considered "milestone 1" for service workers.  We could reasonably delay this, though.
This is a rough sketch for the fix.  There are two big things missing from this:

* We may end up releasing a promise on the wrong thread.  We would need to pass in an event target and pipe it all the way to release stuff on the correct thread.
* We should probably pass a pointer to the global object to ServiceWorkerUpdateFinishCallback and get rid of ServiceWorkerResolveWindowPromiseOnUpdateCallback::mWindow.  Plus, ServiceWorkerResolveWindowPromiseOnUpdaeCallback::UpdateFailed(const ErrorEventInit& aErrorDesc) should probably move whole-sale to the base class.

If someone wants to finish this up, please go ahead!
Bug 1187350 - update() should return a Promise. r?ehsan,catalinb
Attachment #8648319 - Flags: review?(catalin.badea392)
Assignee: nobody → nsm.nikhil
Comment on attachment 8648319 [details]
MozReview Request: Bug 1187350 - update() should return a Promise. r?ehsan,catalinb

https://reviewboard.mozilla.org/r/16201/#review14537

lgtm, just one comment

::: dom/workers/ServiceWorkerRegistration.cpp:374
(Diff revision 1)
> +    : mPromiseProxy(PromiseWorkerProxy::Create(aWorkerPrivate, aWorkerPromise))

This is not safe since PromiseWorkerProxy::Create can fail and there are no null checks when mPromiseProxy is used on the main thread. You should check this on the worker thread and don't dispatch the runnable if it failed.
Attachment #8648319 - Flags: review?(catalin.badea392) → review+
Ehsan, could i get dom peer sign off on the webidl? Thanks!
Flags: needinfo?(ehsan)
Comment on attachment 8648319 [details]
MozReview Request: Bug 1187350 - update() should return a Promise. r?ehsan,catalinb

https://reviewboard.mozilla.org/r/16201/#review14993

Ship It!
Attachment #8648319 - Flags: review+
Flags: needinfo?(ehsan)
url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/780809cead8968a0a90e3f913aa568dfa889a5f7
changeset:  780809cead8968a0a90e3f913aa568dfa889a5f7
user:       Nikhil Marathe <nsm.nikhil@gmail.com>
date:       Fri Aug 14 15:06:00 2015 -0700
description:
Bug 1187350 - update() should return a Promise. r=ehsan,catalinb
https://hg.mozilla.org/mozilla-central/rev/780809cead89
Status: NEW → RESOLVED
Closed: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in before you can comment on or make changes to this bug.