1290116 - service worker unregister() should wait for active worker to go idle before clearing the registration

Status: RESOLVED FIXED

(Core :: DOM: Service Workers, defect)

Description

[Sam Ruby]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:47.0) Gecko/20100101 Firefox/47.0
Build ID: 20160606113944

Steps to reproduce:

Attempted to use Basic Authentication with Service Workers.  I started with the offline-fallback demo in serviceworke.rs and added a .htaccess and htpasswd file.  Complete instructions here:

https://github.com/rubys/serviceworker-cookbook/tree/authenticated-offline-fallack/offline-fallback#reproduction-instructions-apache-httpd

Diff:

https://github.com/mozilla/serviceworker-cookbook/commit/554a5b86a378bc4783668388f3e3551eac112c3a


Actual results:

Got a Corrupted Content Error:

https://raw.githubusercontent.com/rubys/serviceworker-cookbook/authenticated-offline-fallack/offline-fallback/CorruptedContentError-firefox.png


Expected results:

Should have seen "Yay, you are online!" page.

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

I can reproduce.  I didn't use apache, though.  I used this SimpleHTTPServer python script:

https://gist.githubusercontent.com/fxsjy/5465353/raw/c5e5b66f5b5b35247560d19fd2ea5251f88e34b7/SimpleAuthServer.py

I don't think this is actually related to the basic auth.  The failure only occurs when pressing the "View Complete Demo Again" button.  This does an unregister() and reload() without waiting like this:

    navigator.serviceWorker.getRegistration().then(function(registration) {
      registration.unregister();
      window.location.reload();
    });

If I change this code to look like this:

    navigator.serviceWorker.getRegistration().then(function(registration) {
      if (registration) {
        return registration.unregister();
      }
    }).then(_ => {
      window.location.reload();
    });

Then I can no longer reproduce.  It seems we have some unsafe race when reloading before the unregister() promise completes.

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

With the original code I see one of four results:

1) "Service worker change, registered the service worker" shown on the page and the web console shows "[oninstall] Cached offline page 'http://localhost:8080/offline.html'".
2) "An active service worker controller was found, no need to register" shown on the page and no logs in the web console.
3) Content corrupted page with this in the web console "Failed to load ‘http://localhost:8080/?1470016121244’. A ServiceWorker intercepted the request and encountered an unexpected error."
4) Only once I saw the same as (1), but also an error about the service worker script fetch failing.

With the code modifications in comment 1 I consistently get only result (1).

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

Sam, please be aware if you require basic auth on the offline.html page itself (or any pre-cached pages fetch'd in the install event handler) they will be cached as failed authorizations.  We don't have a window to associate the authorization with in the install event, so I think you will always get a failed auth for those fetches.

I think this is unrelated to the content corrupted page, though.

Comment 4

[Sam Ruby]

It is not clear what information you need from me.

I will say that my current code base loads the cache entirely from the "client" process (i.e., the browser), and I can verify that the cache contains successful results, and I do NO fetches from the service worker (not precisely true: the code will try a fetch if the object is not in the cache, but I have verified that that path is not being executed). Even so, I still get either content corrupted or failed authorization errors (again, despite verifying that the cache only contains successful responses).

In the case of failed authorization errors, a shift-F5 reload from the server, and in the process throw up a modal dialog prompting for user and password, and after that point the service worker generally behaves, but exiting the browser completely and starting again can cause me to get authentication errors again.

I'll be glad to share my code, but I thought it best to start the conversation with the posted service worker examples.

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

Sorry.  I guess it was more just an fYI about a problem with the example code.  If that does not reflect your real situation, then that is good.  The behavior I was seeing in the example was a failed auth page when the server was offline.

Comment 6

[Ben Kelly [:bkelly, not reviewing]]

So, we are hitting this error on the serviceworke.rs site as well:

Failed to load ‘https://serviceworke.rs/offline-fallback/’. A ServiceWorker intercepted the request and encountered an unexpected error.service-worker.js:23:4

But its not showing the error because about:neterror is deemed non-secure and this is an iframe in a secure document:

Blocked loading mixed active content “about:neterror?e=corruptedContentErrorv2&u=https%3A//serviceworke.rs/offline-fallback/&c=UTF-8&f=regular&d=The%20site%20at%20https%3A//serviceworke.rs/offline-fallback/%20has%20experienced%20a%20network%20protocol%20violation%20that%20cannot%20be%20repaired.”[Learn More]

In localhost http testing I don't think we consider this to be mixed content.

Comment 7

[Ben Kelly [:bkelly, not reviewing]]

Filed bug 1291448 about blocking about:neterror as mixed content.

Comment 8

[Ben Kelly [:bkelly, not reviewing]]

For some reason the RespondWithHandler is being GC'd before the promise is fulfilled.  Currently we cancel the intercepted request in this case:

https://dxr.mozilla.org/mozilla-central/source/dom/workers/ServiceWorkerEvents.cpp#341

We could just reset the channel here, but I'd like to figure out why we are GC'ing the RespondWithHandler first.

Comment 9

[Ben Kelly [:bkelly, not reviewing]]

I believe the problem here is that we unconditionally terminate workers and remove the registration when the last controlling document goes away:

https://dxr.mozilla.org/mozilla-central/source/dom/workers/ServiceWorkerUnregisterJob.cpp#142
https://dxr.mozilla.org/mozilla-central/source/dom/workers/ServiceWorkerManager.cpp#2005

We really need to wait to do this until the active worker is idle.  This is very similar to bug 1170543.

I'll make these entry points check the active worker idle state.  I'll also add this to the spec issue.

Comment 10

[Ben Kelly [:bkelly, not reviewing]]

Attachment: P1 Expose a ServiceWorkerRegistrationInfo::IsIdle() method. r=asuth

Comment 11

[Ben Kelly [:bkelly, not reviewing]]

Attachment: P2 Clean up ServiceWorkerManager::StopControllingADocument(). r=asuth

Comment 12

[Ben Kelly [:bkelly, not reviewing]]

Attachment: P3 Delay removing registration until active worker is idle. r=asuth

With these patches I can no longer trigger the failure locally.  I will write a wpt test that exposes the problem next.

Comment 13

[Ben Kelly [:bkelly, not reviewing]]

Attachment: P4 Verify that unregister waits for active worker to go idle. r=asuth

This fails without the P1, P2, and P3 patches.  It succeeds with the patches.

Comment 14

[Ben Kelly [:bkelly, not reviewing]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=807f79bb9595

Comment 15

[Ben Kelly [:bkelly, not reviewing]]

We should release this with bug 1170543 if we can.

Comment 16

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8777375 [details] [diff] [review]
P1 Expose a ServiceWorkerRegistrationInfo::IsIdle() method. r=asuth

Step one is to expose the "registration is idle" state in a convenient way.  later patches will then use this method.

Comment 17

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8777377 [details] [diff] [review]
P2 Clean up ServiceWorkerManager::StopControllingADocument(). r=asuth

This patch simply cleans up the logic in ServiceWorkerManager::StopControllingADocument() to use our traditional "short circuit return" style.  It has no functional change.

Comment 18

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8777378 [details] [diff] [review]
P3 Delay removing registration until active worker is idle. r=asuth

Do the actual work of delaying registration teardown until after the registration's active worker has gone idle.

Comment 19

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8777396 [details] [diff] [review]
P4 Verify that unregister waits for active worker to go idle. r=asuth

Add a test case verifying we wait to teardown the registration until the active worker is idle.

I put this in activation.https.html because its nearly identical to the other tests there.  Its just a bit of a misnomer because its not about activation but instead about unregister.  It doesn't seem worth the hassle of renaming the test or creating a new test file with duplicate logic.

Comment 20

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 8777396 [details] [diff] [review]
P4 Verify that unregister waits for active worker to go idle. r=asuth

Review of attachment 8777396 [details] [diff] [review]:
-----------------------------------------------------------------

Restating the test for my benefit:
- Step one retracts the controlled iframe document that "active" was controlling.  "active" is now being kept alive by the (post)MessageEvent's waitUntil().  Prior to the code in P3 being introduced that checks for the worker being idle, this would have either resulted in the registration being removed in one of:
  - ServiceWorkerUnregisterJob::Unregister if the document was already gone, or
  - ServiceWorkerManager::StopControllingADocument if the document removal raced the unregister.  For this specific test, it won't be this one because nsDocument::SetScriptGlobalObject should get synchronously called by the remove() and call through to this path prior to the unregister call in the test.
- Step two is gated on the unregister job completing (in step 5, after flagging uninstalling in 4, with step 6 skipping with us assuming that your proposed change of "using the service worker" at https://github.com/slightlyoff/ServiceWorker/issues/916#issuecomment-237272060 will go through).  We wait for the the "waiting" SW to be marked redundant, which will occur when ServiceWorkerRegistrationInfo::Clear() is invoked by SWM::RemoveRegistration().  That happens when we postMessage the "active" SW which resolves its MessageEvent waitUntil() which results in SWM::WorkerIsIdle being invoked.
- Step three is just checking that the Clear() totally happened.

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/db014dc92795
P1 Expose a ServiceWorkerRegistrationInfo::IsIdle() method. r=asuth
https://hg.mozilla.org/integration/mozilla-inbound/rev/c50283f07370
P2 Clean up ServiceWorkerManager::StopControllingADocument(). r=asuth
https://hg.mozilla.org/integration/mozilla-inbound/rev/d6a336a1fa97
P3 Delay removing registration until active worker is idle. r=asuth
https://hg.mozilla.org/integration/mozilla-inbound/rev/3e95eb4821b8
P4 Verify that unregister waits for active worker to go idle. r=asuth

Comment 22

[Sam Ruby]

(In reply to Ben Kelly [:bkelly] from comment #1)
> I can reproduce.  
> I don't think this is actually related to the basic auth.

I do think that what you are fixing is an important bug, but I'm increasingly convinced that the bug I wanted to report on is different than the one being fixed.

Once there is a stable version (preferably in a nightly build, but if necessary I can build it myself - it has been a while, but I used to be able to build firefox on my machine), I'll follow up (either in this bug or a new bug - I care not) with a reproduction scenario, this time using Python's SimpleHTTPServer.

Comment 23

[Ben Kelly [:bkelly, not reviewing]]

(In reply to Sam Ruby from comment #22)
> (In reply to Ben Kelly [:bkelly] from comment #1)
> > I can reproduce.  
> > I don't think this is actually related to the basic auth.
> 
> I do think that what you are fixing is an important bug, but I'm
> increasingly convinced that the bug I wanted to report on is different than
> the one being fixed.
> 
> Once there is a stable version (preferably in a nightly build, but if
> necessary I can build it myself - it has been a while, but I used to be able
> to build firefox on my machine), I'll follow up (either in this bug or a new
> bug - I care not) with a reproduction scenario, this time using Python's
> SimpleHTTPServer.

Please do!  I was not aware you saw another error beyond the corrupted content after pressing "View Complete Demo Again".

If you run into more problems, please file them as a new bug and need-info me.  Thanks!

Comment 24

[Ben Kelly [:bkelly, not reviewing]]

By the way, I think you could test now with the builds from my tinderbox run:

Windows:
http://archive.mozilla.org/pub/firefox/try-builds/[e-mail]-807f79bb95954a90361b418dcf1cd4131aba2377/try-win32/firefox-50.0a1.en-US.win32.zip

Mac:
http://archive.mozilla.org/pub/firefox/try-builds/[e-mail]-807f79bb95954a90361b418dcf1cd4131aba2377/try-macosx64/firefox-50.0a1.en-US.mac.dmg

Linux:
http://archive.mozilla.org/pub/firefox/try-builds/[e-mail]-807f79bb95954a90361b418dcf1cd4131aba2377/try-linux64/firefox-50.0a1.en-US.linux-x86_64.tar.bz2

Can you reproduce the problem with one of these builds?

Comment 25

[Sam Ruby]

Will do once this lands in a build.

Meanwhile, the scenario I wanted to report on is one where the cache is loaded correctly, and the service worker correctly returns that data via event.respondWith many times.  All is well and good until the browser is completely closed and then the user restarts the browser and attempts to visit the page.  The result is a 401 response.  There is no opportunity to provide a user and password, just an unconditional authentication error.  In fact, no request is sent to the server in this scenario.

Because the cache is available to the browser (client) process, I can verify that the data in the cache is still correct.

Comment 26

[Sam Ruby]

I'll try one of those builds either late tonight or tomorrow (I'm East Coast US).

Comment 27

[Ben Kelly [:bkelly, not reviewing]]

(In reply to Sam Ruby from comment #25)
> Will do once this lands in a build.
> 
> Meanwhile, the scenario I wanted to report on is one where the cache is
> loaded correctly, and the service worker correctly returns that data via
> event.respondWith many times.  All is well and good until the browser is
> completely closed and then the user restarts the browser and attempts to
> visit the page.  The result is a 401 response.  There is no opportunity to
> provide a user and password, just an unconditional authentication error.  In
> fact, no request is sent to the server in this scenario.

Yes, I can reproduce this.

> Because the cache is available to the browser (client) process, I can verify
> that the data in the cache is still correct.

At least for the offline-fallback demo its not pulling any data from the Cache.  The service worker just proxies the connection through.

I'll file a new bug.  Thanks.

Comment 28

[Ben Kelly [:bkelly, not reviewing]]

See bug 1291893.

Comment 29

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I had to back this out for w-e10s failures like https://treeherder.mozilla.org/logviewer.html#?job_id=33262919&repo=mozilla-inbound

https://hg.mozilla.org/integration/mozilla-inbound/rev/a0eb2464ca17

Comment 30

[Ben Kelly [:bkelly, not reviewing]]

This does not reproduce locally, but my best guess is we are delaying the unregister because we happen to catch the worker in the activating state while its processing the no-op activate event.

Comment 31

[Ben Kelly [:bkelly, not reviewing]]

(In reply to Ben Kelly [:bkelly] from comment #30)
> This does not reproduce locally, but my best guess is we are delaying the
> unregister because we happen to catch the worker in the activating state
> while its processing the no-op activate event.

Actually, that shouldn't matter.  Because the uninstalling flag should be set.

Except... the spec does not say to ignore registrations with the uninstalling flag set.  I think that is probably a spec bug.  I filed:

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

Let's see if that fixes the issue here.

Comment 32

[Ben Kelly [:bkelly, not reviewing]]

Attachment: P5 Make navigator.serviceWorker.getRegistrations() ignore uninstalling registrations. r=asuth

https://treeherder.mozilla.org/#/jobs?repo=try&revision=879f8583a484

Comment 33

[Ben Kelly [:bkelly, not reviewing]]

Attachment: P6 Add a wpt test case verifying getRegistrations() does not return registrations with a pending uninstall. r=asuth

Since the other test case is racy, lets explicitly test getRegistrations() with the uninstalling flag set.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bca2a724cae8

Comment 34

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8777975 [details] [diff] [review]
P5 Make navigator.serviceWorker.getRegistrations() ignore uninstalling registrations. r=asuth

The try build indicates this fixes the problem with the previous landing.

I also got preliminary spec approval from Jake on IRC.

Basically we getRegistrations() should be doing the same thing as getRegistration().  A registration that has a pending uninstall should not be returned.

Comment 35

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8778002 [details] [diff] [review]
P6 Add a wpt test case verifying getRegistrations() does not return registrations with a pending uninstall. r=asuth

The additional test case verifies that getRegistrations() hides registrations with pending uninstalls.  It does this by holding a controlled frame alive and calling unregister().  In this situation the registration gets the pending uninstall flag, but does not actually go away.

This test fails without the P5 patch.  It succeeds with the P5 patch.

Comment 36

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 8777975 [details] [diff] [review]
P5 Make navigator.serviceWorker.getRegistrations() ignore uninstalling registrations. r=asuth

Review of attachment 8777975 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/workers/ServiceWorkerManager.cpp
@@ +696,5 @@
>      }
>  
>      for (uint32_t i = 0; i < data->mOrderedScopes.Length(); ++i) {
> +      RefPtr<ServiceWorkerRegistrationInfo> info =
> +        data->mInfos.GetWeak(data->mOrderedScopes[i]);

Is there an idiom for using GetWeak() when we know the result will already exist?

Or an idiom change to favor this style since the pointer gets directly returned (and AddRefed by the refptr) rather than requiring a separate declaration line and manually typing getter_AddRefs for the obscured outval?

(I ask because I don't see a lot of GetWeak in dom/workers/ and "Weak" always raises my suspicions.)

Comment 37

[Ben Kelly [:bkelly, not reviewing]]

(In reply to Andrew Sutherland [:asuth] from comment #36)
> > +      RefPtr<ServiceWorkerRegistrationInfo> info =
> > +        data->mInfos.GetWeak(data->mOrderedScopes[i]);
> 
> Is there an idiom for using GetWeak() when we know the result will already
> exist?

Yea, I just wanted to assign to the RefPtr in a single statement.

Comment 38

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/90006dffbe5b
P1 Expose a ServiceWorkerRegistrationInfo::IsIdle() method. r=asuth
https://hg.mozilla.org/integration/mozilla-inbound/rev/74ceee4dc2f4
P2 Clean up ServiceWorkerManager::StopControllingADocument(). r=asuth
https://hg.mozilla.org/integration/mozilla-inbound/rev/ca998a8cbe48
P3 Delay removing registration until active worker is idle. r=asuth
https://hg.mozilla.org/integration/mozilla-inbound/rev/44b50089c079
P4 Verify that unregister waits for active worker to go idle. r=asuth
https://hg.mozilla.org/integration/mozilla-inbound/rev/c9922c014310
P5 Make navigator.serviceWorker.getRegistrations() ignore uninstalling registrations. r=asuth
https://hg.mozilla.org/integration/mozilla-inbound/rev/aa715b19902b
P6 Add a wpt test case verifying getRegistrations() does not return registrations with a pending uninstall. r=asuth

Comment 39

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/35b2715748a3 for 70% odds of WinXP debug timing out in test_subscription_change.html, https://treeherder.mozilla.org/logviewer.html#?job_id=33457497&repo=mozilla-inbound

Comment 40

[Ben Kelly [:bkelly, not reviewing]]

Given the number of backouts here I don't intend to uplift any more.  This can just ride the trains.  (Once I figure out this latest failure.)

Comment 41

[Ben Kelly [:bkelly, not reviewing]]

Attachment: P7 Fixed activation races in push service worker tests. r=kitcambridge

I think the push tests are generally a bit racy in that very few of them wait for the service worker to become active.  Changing the timing could cause these races to finish in different ways.

Let's see if this fixes the problem:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0c8bf9706520

Comment 42

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8778958 [details] [diff] [review]
P7 Fixed activation races in push service worker tests. r=kitcambridge

Review of attachment 8778958 [details] [diff] [review]:
-----------------------------------------------------------------

Kit, this patch makes the dom/push/test mochitests consistently wait for service workers to be active.  We were only waiting in a few cases here before.  I think to avoid races we should generally wait for active workers all the time.  It did not seem like any of these tests were purposefully trying to test push against installing or waiting workers.

Even if this does not fix the failures in the last backout its still probably a good thing to land.

Comment 43

[Lina Butler [:lina]]

Comment on attachment 8778958 [details] [diff] [review]
P7 Fixed activation races in push service worker tests. r=kitcambridge

Review of attachment 8778958 [details] [diff] [review]:
-----------------------------------------------------------------

Ouch. Thanks for fixing these, Ben!

::: dom/push/test/test_utils.js
@@ +235,5 @@
> +      resolve(swr);
> +      return;
> +    }
> +    sw.addEventListener('statechange', function onStateChange(evt) {
> +      if (sw.state === 'activated') {

Just so I understand: this is different than `navigator.serviceWorker.ready` because that resolves if the worker is "activating" or "activated", right?

Comment 44

[Ben Kelly [:bkelly, not reviewing]]

(In reply to Kit Cambridge [:kitcambridge] from comment #43)
> Just so I understand: this is different than `navigator.serviceWorker.ready`
> because that resolves if the worker is "activating" or "activated", right?

No, I think .ready resolves on activated.  Its more to avoid accidentally using a previously active service worker.  If you already have an activated service worker .ready resolves immediately.  So after a .register() you usually want to wait for .installing to really test your new worker.

Comment 45

[Ben Kelly [:bkelly, not reviewing]]

Also, .ready doesn't let you wait for a different scope activation which some of these tests needed.

Comment 46

[Ben Kelly [:bkelly, not reviewing]]

Ugh... After 24 hours I finally got two winxp test results.  One of them is a failure.

Comment 47

[Ben Kelly [:bkelly, not reviewing]]

Although it failed in a different way.  Now its timing out right at the start.  The previous test exercises our "long running SW, kill it with fire" code paths.  I suspect we are leaving a token count non zero there which make the SW seem like its non-idle forever.  This then prevents the previous worker from every being completely evicted.

Comment 48

[Ben Kelly [:bkelly, not reviewing]]

In the winxp failures I get these additional error messages in test_serviceworker_lifetime.html:

 14:16:48     INFO -  [336] WARNING: '!JS::AddPromiseReactions(cx, promiseObj, resolveFunc, rejectFunc)', file c:/builds/moz2_slave/try-w32-d-00000000000000000000/build/src/dom/promise/Promise.cpp, line 824

14:16:48 INFO - [336] WARNING: A runnable was posted to a worker that is already shutting down!: file c:/builds/moz2_slave/try-w32-d-00000000000000000000/build/src/dom/workers/WorkerPrivate.cpp, line 2443

Comment 49

[Ben Kelly [:bkelly, not reviewing]]

Ugh, I think this is a race in worker holder code.  I'm going to file a separate bug.

Comment 50

[Ben Kelly [:bkelly, not reviewing]]

I am fairly confident bug 1293690 is the issue here.  I added some NS_ASSERTION(false) calls to log stacks and managed to trigger the same timing that the winxp automation is hitting.  My patch for the other bug seems to fix it.  Will push a new try build in a bit.

Comment 51

[Ben Kelly [:bkelly, not reviewing]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ec0c6e9fcf6a

Comment 52

[Ben Kelly [:bkelly, not reviewing]]

The try build has all green winxp M(3) results with bug 1293690 in the mix.  Just have to wait for baku to come back from PTO next week to review it.

Comment 53

[Ben Kelly [:bkelly, not reviewing]]

And indeed, I see it hitting the expected error path:

20:02:57     INFO -  [644] WARNING: '!JS::AddPromiseReactions(cx, promiseObj, resolveFunc, rejectFunc)', file c:/builds/moz2_slave/try-w32-d-00000000000000000000/build/src/dom/promise/Promise.cpp, line 824
20:02:57     INFO -  [644] WARNING: '!ref->UseWorkerHolder()', file c:/builds/moz2_slave/try-w32-d-00000000000000000000/build/src/dom/workers/ServiceWorkerPrivate.cpp, line 342
20:02:57     INFO -  [644] WARNING: '!handler', file c:/builds/moz2_slave/try-w32-d-00000000000000000000/build/src/dom/workers/ServiceWorkerPrivate.cpp, line 379
20:02:57     INFO -  [644] WARNING: A runnable was posted to a worker that is already shutting down!: file c:/builds/moz2_slave/try-w32-d-00000000000000000000/build/src/dom/workers/WorkerPrivate.cpp, line 2443

The patches in bug 1293690 make the UseWorkerHolder() call fail here if the worker is already terminated.

Comment 54

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6a4e8c92faae
P1 Expose a ServiceWorkerRegistrationInfo::IsIdle() method. r=asuth
https://hg.mozilla.org/integration/mozilla-inbound/rev/eb7da0dbfc72
P2 Clean up ServiceWorkerManager::StopControllingADocument(). r=asuth
https://hg.mozilla.org/integration/mozilla-inbound/rev/2d5ad5e9ed20
P3 Delay removing registration until active worker is idle. r=asuth
https://hg.mozilla.org/integration/mozilla-inbound/rev/8e9a9ba04ce5
P4 Verify that unregister waits for active worker to go idle. r=asuth
https://hg.mozilla.org/integration/mozilla-inbound/rev/e0b9c25b6125
P5 Make navigator.serviceWorker.getRegistrations() ignore uninstalling registrations. r=asuth
https://hg.mozilla.org/integration/mozilla-inbound/rev/a81b6ce5c25f
P6 Add a wpt test case verifying getRegistrations() does not return registrations with a pending uninstall. r=asuth
https://hg.mozilla.org/integration/mozilla-inbound/rev/bbc14d107fca
P7 Fixed activation races in push service worker tests. r=kitcambridge

Comment 55

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/252ff19cd746 for the build bustage from bug 1293690.

Comment 56

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c0c8cd41219c
P1 Expose a ServiceWorkerRegistrationInfo::IsIdle() method. r=asuth
https://hg.mozilla.org/integration/mozilla-inbound/rev/465601c1b933
P2 Clean up ServiceWorkerManager::StopControllingADocument(). r=asuth
https://hg.mozilla.org/integration/mozilla-inbound/rev/eb891c00bc15
P3 Delay removing registration until active worker is idle. r=asuth
https://hg.mozilla.org/integration/mozilla-inbound/rev/508299ab7a22
P4 Verify that unregister waits for active worker to go idle. r=asuth
https://hg.mozilla.org/integration/mozilla-inbound/rev/cf520760c0df
P5 Make navigator.serviceWorker.getRegistrations() ignore uninstalling registrations. r=asuth
https://hg.mozilla.org/integration/mozilla-inbound/rev/8207a64e1cf2
P6 Add a wpt test case verifying getRegistrations() does not return registrations with a pending uninstall. r=asuth
https://hg.mozilla.org/integration/mozilla-inbound/rev/c3b6a4e0898f
P7 Fixed activation races in push service worker tests. r=kitcambridge

Comment 57

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c0c8cd41219c
https://hg.mozilla.org/mozilla-central/rev/465601c1b933
https://hg.mozilla.org/mozilla-central/rev/eb891c00bc15
https://hg.mozilla.org/mozilla-central/rev/508299ab7a22
https://hg.mozilla.org/mozilla-central/rev/cf520760c0df
https://hg.mozilla.org/mozilla-central/rev/8207a64e1cf2
https://hg.mozilla.org/mozilla-central/rev/c3b6a4e0898f