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)
Core
DOM: Service Workers
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.
Assignee | ||
Comment 1•6 years ago
|
||
Work-in-progress.
Assignee | ||
Comment 2•6 years ago
|
||
Attachment #8983213 -
Attachment is obsolete: true
Assignee | ||
Comment 3•6 years ago
|
||
Assignee | ||
Comment 4•6 years ago
|
||
Attachment #8983567 -
Attachment is obsolete: true
Assignee | ||
Comment 5•6 years ago
|
||
Assignee | ||
Comment 6•6 years ago
|
||
Assignee | ||
Comment 7•6 years ago
|
||
Assignee | ||
Comment 8•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=31fd33e36ef32883071ec6be4ac37d41836d120c
Assignee | ||
Comment 9•6 years ago
|
||
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.
Updated•6 years ago
|
Priority: -- → P2
Assignee | ||
Comment 10•6 years ago
|
||
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)
Assignee | ||
Comment 11•6 years ago
|
||
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)
Assignee | ||
Comment 12•6 years ago
|
||
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)
Assignee | ||
Comment 13•6 years ago
|
||
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)
Assignee | ||
Comment 14•6 years ago
|
||
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)
Assignee | ||
Comment 15•6 years ago
|
||
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)
Updated•6 years ago
|
Attachment #8983565 -
Flags: review?(bugmail) → review+
Updated•6 years ago
|
Attachment #8983809 -
Flags: review?(bugmail) → review+
Updated•6 years ago
|
Attachment #8983853 -
Flags: review?(bugmail) → review+
Updated•6 years ago
|
Attachment #8983889 -
Flags: review?(bugmail) → review+
Updated•6 years ago
|
Attachment #8983912 -
Flags: review?(bugmail) → review+
Updated•6 years ago
|
Attachment #8983923 -
Flags: review?(bugmail) → review+
Comment 16•6 years ago
|
||
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
Comment 17•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d5e35711fa7b https://hg.mozilla.org/mozilla-central/rev/15b6dcb4a6cf https://hg.mozilla.org/mozilla-central/rev/3b91713af124 https://hg.mozilla.org/mozilla-central/rev/8e2ac5d19e24 https://hg.mozilla.org/mozilla-central/rev/ba3ec8751abc https://hg.mozilla.org/mozilla-central/rev/ee50bd36be6e
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox62:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
You need to log in
before you can comment on or make changes to this bug.
Description
•