Closed Bug 1186856 Opened 6 years ago Closed 6 years ago

"Harness status: OK" + failing test when running "unregister-then-register-new-script.https.html" test

Categories

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

defect
Not set
normal

Tracking

(firefox42 wontfix, firefox43 wontfix, firefox44 fixed, firefox45 fixed)

RESOLVED FIXED
mozilla45
Tracking Status
firefox42 --- wontfix
firefox43 --- wontfix
firefox44 --- fixed
firefox45 --- fixed

People

(Reporter: noemi, Assigned: bkelly)

References

Details

Attachments

(1 file)

Test run such as |./mach web-platform-tests _mozilla/service-workers/service-worker/unregister-then-register-new-script.https.html|

Actual Result: 3 "File not found" issues
1- Firefox can't find the file at https://web-platform.test:8443/_mozilla/service-workers/service-worker/resources/scope/unregister-then-register-new-script-that-404s.
2- Firefox can't find the file at https://web-platform.test:8443/_mozilla/service-workers/service-worker/resources/scope/unregister-then-register-new-script-that-exists.
3- Firefox can't find the file at https://web-platform.test:8443/_mozilla/service-workers/service-worker/resources/scope/unregister-then-register-reject-install-worker.
Component: DOM: Service Workers → web-platform-tests
Product: Core → Testing
Hi,


just checked on m-c (fcef8ded8221 revision), please see below the results:

Summary

Harness status: OK

Found 3 tests
1 Pass
2 Fail
Details

*Result
Fail	
*Test Name
Registering a new script URL while an unregistered registration is in use	
*Message
assert_unreached: unexpected rejection: wait_for_state must be passed a ServiceWorker Reached unreachable code
unreached_rejection/<@https://web-platform.test:8443/_mozilla/service-workers/service-worker/resources/test-helpers.sub.js:45:7
Test.prototype.step@https://web-platform.test:8443/resources/testharness.js:1380:20
Test.prototype.step_func/<@https://web-platform.test:8443/resources/testharness.js:1404:1
promise callback*@https://web-platform.test:8443/_mozilla/service-workers/service-worker/unregister-then-register-new-script.https.html:15:1
Test.prototype.step@https://web-platform.test:8443/resources/testharness.js:1380:20
async_test@https://web-platform.test:8443/resources/testharness.js:513:13
@https://web-platform.test:8443/_mozilla/service-workers/service-worker/unregister-then-register-new-script.https.html:8:1

*Result
Pass	
*Test Name
Registering a new script URL that 404s does not resurrect an unregistered registration	

*Result
Fail	
*Test Name
Registering a new script URL that fails to install does not resurrect an unregistered registration
*Message
assert_unreached: unexpected rejection: wait_for_state must be passed a ServiceWorker Reached unreachable code
unreached_rejection/<@https://web-platform.test:8443/_mozilla/service-workers/service-worker/resources/test-helpers.sub.js:45:7
Test.prototype.step@https://web-platform.test:8443/resources/testharness.js:1380:20
Test.prototype.step_func/<@https://web-platform.test:8443/resources/testharness.js:1404:1
promise callback*@https://web-platform.test:8443/_mozilla/service-workers/service-worker/unregister-then-register-new-script.https.html:121:1
Test.prototype.step@https://web-platform.test:8443/resources/testharness.js:1380:20
async_test@https://web-platform.test:8443/resources/testharness.js:513:13
@https://web-platform.test:8443/_mozilla/service-workers/service-worker/unregister-then-register-new-script.https.html:8:1
Per comment 2, changing the subject and the dependency accordingly
Blocks: 1180731
No longer blocks: 1179806
Summary: "File not found" issue when running "unregister-then-register-new-script.https.html" test → "Harness status: OK" + failing test when running "unregister-then-register-new-script.https.html" test
This seems pretty important for compat, and we need to fix it before v1.

Anne, do you mind informing us about the result of discussing this in the F2F this week?  Thanks!
Flags: needinfo?(annevk)
Nikhil's suggestion was accepted.
Flags: needinfo?(annevk)
Blocks: 1189023
No longer blocks: 1180731
I'll start looking at this tomorrow.
Assignee: nobody → bkelly
Status: NEW → ASSIGNED
This is definitely a bug and we should try to get it in 44, but its not so critical we should block v1 for it.
Blocks: ServiceWorkers-postv1
No longer blocks: ServiceWorkers-v1
This catches up to the change in:

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

The Register Algorithm now unconditionally removes the uninstalling flag in step 4.2.1.

Implementing this lets us pass the first case since run-to-completion is no longer violated.  I then had to update the expectations in the other two cases to understand that uninstallation is always aborted by .register().
Attachment #8689725 - Flags: review?(josh)
Attachment #8689725 - Flags: review?(josh) → review+
Note, it seems we don't early return from .update() when uninstalling is set.  I wrote a follow-up bug to deal with that.  See bug 1226384.
Comment on attachment 8689725 [details] [diff] [review]
ServiceWorker .register() should always stop current registration from uninstalling. r=jdm

Approval Request Comment
[Feature/regressing bug #]: Service workers
[User impact if declined]: This fixes a bug when a service worker is being both unregistered and registered at the same time.  It also fixes the WPT test to match the latest spec definition.
[Describe test coverage new/current, TreeHerder]: WPT test included.
[Risks and why]: Minimal.  Only affects service workers.
[String/UUID change made/needed]: None.
Attachment #8689725 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/50b8122fd926
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Comment on attachment 8689725 [details] [diff] [review]
ServiceWorker .register() should always stop current registration from uninstalling. r=jdm

This change has been on Nightly for a few days, seems safe to uplift to Aurora44. It's always good to see new automated tests.
Attachment #8689725 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.