Closed
Bug 1113957
Opened 10 years ago
Closed 10 years ago
ServiceWorker unregistration should also use the FIFO job queue
Categories
(Core :: DOM: Workers, defect)
Tracking
()
RESOLVED
FIXED
mozilla38
People
(Reporter: nsm, Assigned: nsm)
References
Details
Attachments
(2 files, 1 obsolete file)
(deleted),
patch
|
baku
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
baku
:
review+
|
Details | Diff | Splinter Review |
Otherwise register('/blah', ...).then(function(swr) { swr.unregister(); }) has no-op behaviour due to the SW still being in the installing stage.
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → nsm.nikhil
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8553348 -
Flags: review?(amarchesini)
Comment 2•10 years ago
|
||
Comment on attachment 8553348 [details] [diff] [review] ServiceWorker unregistration uses job queue Review of attachment 8553348 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/workers/ServiceWorkerManager.cpp @@ +1446,5 @@ > + { > + AssertIsOnMainThread(); > + nsCOMPtr<nsIRunnable> r = > + NS_NewRunnableMethod(this, &ServiceWorkerUnregisterJob::UnregisterAndDone); > + DebugOnly<nsresult> ok = NS_DispatchToMainThread(r); MOZ_ALWAYS_TRUE(NS_SUCCEEDED(NS_DispatchToMainThread(r))); It's exactly like yours. Up to you. Lately I prefer this way to write this check. ::: dom/workers/test/serviceworkers/test_unregister.html @@ -86,5 @@ > > function runTest() { > simpleRegister() > - .then(function(v) { > - info("simpleRegister() promise resolved"); Why have you removed all this useful info messages?
Attachment #8553348 -
Flags: review?(amarchesini) → review+
Assignee | ||
Comment 3•10 years ago
|
||
(In reply to Andrea Marchesini (:baku) from comment #2) > Comment on attachment 8553348 [details] [diff] [review] > ServiceWorker unregistration uses job queue > > Review of attachment 8553348 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: dom/workers/ServiceWorkerManager.cpp > @@ +1446,5 @@ > > + { > > + AssertIsOnMainThread(); > > + nsCOMPtr<nsIRunnable> r = > > + NS_NewRunnableMethod(this, &ServiceWorkerUnregisterJob::UnregisterAndDone); > > + DebugOnly<nsresult> ok = NS_DispatchToMainThread(r); > > MOZ_ALWAYS_TRUE(NS_SUCCEEDED(NS_DispatchToMainThread(r))); > > It's exactly like yours. Up to you. Lately I prefer this way to write this > check. > > ::: dom/workers/test/serviceworkers/test_unregister.html > @@ -86,5 @@ > > > > function runTest() { > > simpleRegister() > > - .then(function(v) { > > - info("simpleRegister() promise resolved"); > > Why have you removed all this useful info messages? They were debugging messages to ensure the promises were getting resolved. I never intended to land them in the first place. They are just noise unless a test is consistently failing/timing out.
Assignee | ||
Comment 4•10 years ago
|
||
/r/2921 - Bug 1113957 - ServiceWorker unregistration uses job queue. r=baku /r/2923 - Bug 1081293 - Silently ignore lack of ServiceWorkerManager when shutting down ServiceWorkerContainer. Pull down these commits: hg pull review -r 1c5b76b58327ba8c6c536d18af43f9788ce477ee
Attachment #8553889 -
Flags: review?(amarchesini)
Assignee | ||
Updated•10 years ago
|
Attachment #8553889 -
Attachment is obsolete: true
Attachment #8553889 -
Flags: review?(amarchesini)
Assignee | ||
Comment 6•10 years ago
|
||
Attachment #8553920 -
Flags: review?(amarchesini)
Assignee | ||
Updated•10 years ago
|
Attachment #8553348 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8553348 -
Attachment is obsolete: false
Assignee | ||
Comment 7•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/67d15be29a07
Comment 8•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/67d15be29a07
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Updated•10 years ago
|
Attachment #8553920 -
Flags: review?(amarchesini) → review+
Assignee | ||
Updated•10 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 9•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/237edb7e4ae5
https://hg.mozilla.org/mozilla-central/rev/237edb7e4ae5
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•