wpt service-workers tests unstable with e10s

RESOLVED FIXED in Firefox 44

Status

Testing
web-platform-tests
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: jgraham, Assigned: baku)

Tracking

(Blocks: 1 bug)

unspecified
mozilla44
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(e10s+, firefox44 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

2 years ago
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

2 years ago
We should investigate these failures as part of our test improvements on aurora if possible.
Blocks: 1059784

Updated

2 years ago
tracking-e10s: --- → +
(Reporter)

Updated

2 years ago
Blocks: 1206011
(Reporter)

Updated

2 years ago
No longer blocks: 1206011
Related: https://github.com/slightlyoff/ServiceWorker/issues/746
(Assignee)

Comment 3

2 years ago
Created attachment 8664846 [details] [diff] [review]
patch

This is a first patch to fix the crash we have in this WPT: unregister-then-register-new-script.https.html
Attachment #8664846 - Flags: review?(nsm.nikhil)
(Assignee)

Updated

2 years ago
Assignee: nobody → amarchesini
Status: NEW → ASSIGNED
(Assignee)

Comment 4

2 years ago
Created attachment 8664886 [details] [diff] [review]
patch
Attachment #8664846 - Attachment is obsolete: true
Attachment #8664846 - Flags: review?(nsm.nikhil)
Attachment #8664886 - Flags: review?(nsm.nikhil)
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().
Attachment #8664886 - Flags: review?(nsm.nikhil) → review+
(Assignee)

Comment 6

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

Let's see if it's green enough.

Comment 7

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/414686e07ce3
(Assignee)

Updated

2 years ago
Keywords: leave-open
https://hg.mozilla.org/mozilla-central/rev/414686e07ce3
(Assignee)

Comment 9

2 years ago
Created attachment 8668007 [details] [diff] [review]
patch 2

I cannot reproduce this issue anymore. I guess we can re-enable the test.
Ben, what do you think?
Attachment #8668007 - Flags: review?(bkelly)

Comment 10

2 years ago
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.
Attachment #8668007 - Flags: review?(bkelly) → review+

Comment 11

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/188f0abba61a
(Assignee)

Updated

2 years ago
Keywords: leave-open
https://hg.mozilla.org/mozilla-central/rev/188f0abba61a
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox44: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in before you can comment on or make changes to this bug.