1757724 - Investigate intermittent "Unregister should return true" failures

Status: NEW

(Core :: DOM: Service Workers, task, P3)

Description

[Jens Stutte [:jstutte]]

We have some intermittents that show the same error on different tests. It is reasonable to assume that there is a common cause, so let's investigate that here.

Comment 1

[Jens Stutte [:jstutte]]

We are probably seeing this abort but do not know why.

Comment 2

[Jens Stutte [:jstutte]]

(In reply to Jens Stutte [:jstutte] from comment #0)

We have some intermittents that show the same error on different tests. It is reasonable to assume that there is a common cause, so let's investigate that here.

At least we have a common copy&paste pattern here:

  function unregister() {
    return registration.unregister().then(function(result) {
      ok(result, "Unregister should return true.");
    }, function(e) {
      dump("Unregistering the SW failed: " + e + "\n");
    });
  }

That does not necessarily mean we have a common root cause, but seeing those intermittents randomly on different tests seems to second that interpretation.

(In reply to Jens Stutte [:jstutte] from comment #1)

We are probably seeing this abort but do not know why.

Actually reading the code better I think we are seeing this case, as otherwise we should have seen the dump("Unregistering the SW failed"), IIUC.

So GetRegistration can fail if:

• PrincipalToScopeKey fails which sounds unlikely? This seems to be just some string operations that should always give the same result?
• if we have no RegistrationDataPerPrincipal object in mRegistrationInfos for our aScopeKey
• if that RegistrationDataPerPrincipal object does not contain a registration for our aScope

I see no way in, for example, test_controller.html how we could end up calling unregister twice (which would lead to such a situation). It is less clear to me if there are other possible code paths that might lead to unregister or if there could be an order of events getting us to the unregister before we even finished the registration.

Comment 3

[Jens Stutte [:jstutte]]

Oh, I forgot: asuth suggested talking yesterday:

A historical problem we’ve seen with SW tests is when the same scope is used by a preceding test and we end up with the tests overlapping at all. This could lead to something weird maybe?

At least the scope './file_blob_response/' appears to be used only in one single test (that is among the intermittents), so this seems not to be the case here.

Comment 4

[Eden Chuang[:edenchuang]]

I took a look at the test case test_controller.html.

https://searchfox.org/mozilla-central/rev/840881e1232f664a58b39caaae6284c7bcf121df/dom/serviceworkers/test/controller/index.html#54

might be the reason for Unregister() twice.

I confirmed that the principal and scope for Unregister() in controller/index.html and in test_controller.html are the same

And it seems we reuse the existing ServiceWorkerRegistrationInfo while Register().
https://searchfox.org/mozilla-central/rev/840881e1232f664a58b39caaae6284c7bcf121df/dom/serviceworkers/ServiceWorkerRegisterJob.cpp#30-42