1205676 - wpt service-workers tests unstable with e10s

Status: RESOLVED FIXED

(Testing :: web-platform-tests, defect)

Description

[James Graham [:jgraham]]

testing/web-platform/mozilla/meta/service-workers/service-worker/fetch-request-css-base-url.https.html.ini
testing/web-platform/mozilla/meta/service-workers/service-worker/unregister-then-register-new-script.https.html.ini

Disabled for now

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

We should investigate these failures as part of our test improvements on aurora if possible.

Comment 2

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Related: https://github.com/slightlyoff/ServiceWorker/issues/746

Comment 3

[Andrea Marchesini [:baku]]

Attachment: patch

This is a first patch to fix the crash we have in this WPT: unregister-then-register-new-script.https.html

Comment 4

[Andrea Marchesini [:baku]]

Attachment: patch

Comment 5

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Comment on attachment 8664886 [details] [diff] [review]
patch

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

::: dom/workers/ServiceWorkerManager.cpp
@@ +4633,5 @@
>  ServiceWorkerManager::MaybeRemoveRegistration(ServiceWorkerRegistrationInfo* aRegistration)
>  {
>    MOZ_ASSERT(aRegistration);
>    nsRefPtr<ServiceWorkerInfo> newest = aRegistration->Newest();
> +  if (!newest && HasScope(aRegistration->mPrincipal, aRegistration->mScope)) {

While you are here, we also need to call aRegistration->Clear() before calling RemoveRegistration().

Comment 6

[Andrea Marchesini [:baku]]

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

Let's see if it's green enough.

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/414686e07ce3

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/414686e07ce3

Comment 9

[Andrea Marchesini [:baku]]

Attachment: patch 2

I cannot reproduce this issue anymore. I guess we can re-enable the test.
Ben, what do you think?

Comment 10

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8668007 [details] [diff] [review]
patch 2

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

Ok.  Note, I will need to change or remove this test in the near future.

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/188f0abba61a

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/188f0abba61a