Closed Bug 1466681 Opened 6 years ago Closed 6 years ago

Make service worker binding inner interfaces use callbacks instead of MozPromise

Categories

(Core :: DOM: Service Workers, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla62
Tracking Status
firefox62 --- fixed

People

(Reporter: bkelly, Assigned: bkelly)

References

Details

Attachments

(6 files, 2 obsolete files)

9.61 KB, patch
asuth
: review+
Details | Diff | Splinter Review
7.04 KB, patch
asuth
: review+
Details | Diff | Splinter Review
6.14 KB, patch
asuth
: review+
Details | Diff | Splinter Review
7.62 KB, patch
asuth
: review+
Details | Diff | Splinter Review
9.81 KB, patch
asuth
: review+
Details | Diff | Splinter Review
9.82 KB, patch
asuth
: review+
Details | Diff | Splinter Review
Currently the service worker binding layer's inner interfaces are defined in terms of MozPromise.  While this is a nice abstraction it unfortunately introduces an extra runnable dispatch.  It turns out this make it difficult to get everything to update in the correct order when the IPC layer is added.

Therefore, lets convert the inner interfaces to use callbacks instead of MozPromise.
Before flagging for review here I'm going to rebase my other work-in-progress patches to make sure this does indeed help the IPC case.
Priority: -- → P2
Comment on attachment 8983565 [details] [diff] [review]
P1 Make ServiceWorkerContainer::Inner::Register() use callbacks instead of MozPromise. r=asuth

As discussed previously in this bug we need to precisely control when state changes and events fire.  That means that we can't tolerate the extra runnables required by MozPromise thenables.  To avoid this I'm converting the various Inner interfaces to use std::function callbacks instead.

This first patch converts ServiceWorkerContainer::Inner::Register() to use callbacks.  Note, since this is the first patch it also introduces the callback types needed.

Also, I had to an mOuter back-reference from the ServiceWorkerContainerImpl to the binding ServiceWorkerContainer.  This was necessary in order to access the binding object's global to get the event target.  This is needed in order to do a Then() on the MozPromise returned from ServiceWorkerManager.

If you are wondering why we are doing this if the impl class uses a MozPromise anyway, the answer is that there is no MozPromise present internally on the child side when we get the IPC implementation.
Attachment #8983565 - Flags: review?(bugmail)
Comment on attachment 8983809 [details] [diff] [review]
P2 Make ServiceWorkerContainer::Inner::GetRegistration() use callbacks instead of MozPromise. r=asuth

This patch convert ServiceWorkerContainer::Inner::GetRegistration() to use callbacks.  It should be straightforward.
Attachment #8983809 - Flags: review?(bugmail)
Comment on attachment 8983853 [details] [diff] [review]
P3 Make ServiceWorkerContainer::Inner::GetReady() use callbacks instead of MozPromise. r=asuth

This patch converts ServiceWorkerContainer::Inner::GetReady() to use callbacks.
Attachment #8983853 - Flags: review?(bugmail)
Comment on attachment 8983889 [details] [diff] [review]
P4 Make ServiceWorkerContainer::Inner::GetRegistrations() use callbacks instead of MozPromise. r=asuth

This patch make ServiceWorkerContainer::Inner::GetRegistrations() use callbacks.  Note, I had to add another callback type for the list result.
Attachment #8983889 - Flags: review?(bugmail)
Comment on attachment 8983912 [details] [diff] [review]
P5 Make ServiceWorkerRegistration::Inner::Update() use callbacks instead of MozPromise. r=asuth

This patch makes ServiceWorkerRegistration::Inner::Update() use callbacks instead of MozPromise.  Again, it should be straightforward.
Attachment #8983912 - Flags: review?(bugmail)
Comment on attachment 8983923 [details] [diff] [review]
P6 Make ServiceWorkerRegistration::Inner::Unregister() use callbacks instead of MozPromise. r=asuth

This patch makes ServiceWorkerRegistration::Unregister() use callbacks instead of MozPromise.  Note, I added one last callback type for the boolean result.
Attachment #8983923 - Flags: review?(bugmail)
Attachment #8983565 - Flags: review?(bugmail) → review+
Attachment #8983809 - Flags: review?(bugmail) → review+
Attachment #8983853 - Flags: review?(bugmail) → review+
Attachment #8983889 - Flags: review?(bugmail) → review+
Attachment #8983912 - Flags: review?(bugmail) → review+
Attachment #8983923 - Flags: review?(bugmail) → review+
Pushed by bkelly@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d5e35711fa7b
P1 Make ServiceWorkerContainer::Inner::Register() use callbacks instead of MozPromise. r=asuth
https://hg.mozilla.org/integration/mozilla-inbound/rev/15b6dcb4a6cf
P2 Make ServiceWorkerContainer::Inner::GetRegistration() use callbacks instead of MozPromise. r=asuth
https://hg.mozilla.org/integration/mozilla-inbound/rev/3b91713af124
P3 Make ServiceWorkerContainer::Inner::GetReady() use callbacks instead of MozPromise. r=asuth
https://hg.mozilla.org/integration/mozilla-inbound/rev/8e2ac5d19e24
P4 Make ServiceWorkerContainer::Inner::GetRegistrations() use callbacks instead of MozPromise. r=asuth
https://hg.mozilla.org/integration/mozilla-inbound/rev/ba3ec8751abc
P5 Make ServiceWorkerRegistration::Inner::Update() use callbacks instead of MozPromise. r=asuth
https://hg.mozilla.org/integration/mozilla-inbound/rev/ee50bd36be6e
P6 Make ServiceWorkerRegistration::Inner::Unregister() use callbacks instead of MozPromise. r=asuth
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: