bkelly's theory is that this is a race caused by dom.serviceWorkers.idle_timeout being set to 0 in tests. Examples of test failures: https://treeherder.mozilla.org/#/jobs?repo=try&revision=807d47b26115&selectedJob=21934258 https://treeherder.mozilla.org/#/jobs?repo=try&revision=807d47b26115&selectedJob=21932025 IRC conversation about this: 16:07 <bkelly> till: this may be an issue with this pref setting: ["dom.serviceWorkers.idle_timeout", 0], 16:07 <bkelly> till: I wonder if its terminating the service worker before it can finish the registration 16:09 <till> bkelly: hmm, but why wouldn't that affect my local testing? 16:09 <bkelly> till: it would be a race... 16:09 <till> bkelly: or the DOM Promise implementation, for that matter 16:09 <till> ok, same answer 16:09 <bkelly> till: it could be considered a bug... but we don't ship that low of an idle timeout 16:09 <till> bkelly: but we use it in mochitests? 16:10 <bkelly> till: we also use it here: test_unresolved_fetch_interception.html

This patch just reorders things so our KeepAliveHandler is resolved after the handler provided by the code initiating the event. This way we can make sure the worker is not terminated before the caller gets the correct success/failure status from running the event. https://treeherder.mozilla.org/#/jobs?repo=try&revision=2c7070ee1b0f

Comment on attachment 8760303 [details] [diff] [review] Hold service worker alive until event dispatching code is notified. r=asuth Review of attachment 8760303 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for the excellent comment! It warms my heart whenever I see a comment explaining why something is happening. I sanity-checked the unchanged call-site (1), and confirm that SendPushSubscriptionChangeEventRunnable internally doesn't care what happens to its promise and its nullptr aWaitUntilPromise arg now works for aPromiseHandler because such things are untyped. However, should you be doing "Unused <<" for that usage since the return value is uncheck, or is that idiom only for use in certain situations?

(In reply to Andrew Sutherland [:asuth] from comment #3) > aPromiseHandler because such things are untyped. However, should you be > doing "Unused <<" for that usage since the return value is uncheck, or is > that idiom only for use in certain situations? I've only really seen "Unused <<" for cases where we have MOZ_MUST_USE static checking enabled for the return value.

Try looks fairly green. It appears to have passed the failing tests in the SM promise try as well. Going to land.

Pushed by bkelly@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/4fd4188b0bff Hold service worker alive until event dispatching code is notified. r=asuth

Comment on attachment 8760303 [details] [diff] [review] Hold service worker alive until event dispatching code is notified. r=asuth Review of attachment 8760303 [details] [diff] [review]: ----------------------------------------------------------------- This already landed, but yes, I can confirm that it fixes the issue with SpiderMonkey Promises. Thanks for the quick work on this!