Noemí Freire (:noemi) Reporter
	Description • 9 years ago

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.

	Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)
	Comment 1 • 9 years ago

One of the spec issues affecting this test https://github.com/slightlyoff/ServiceWorker/issues/746

	Noemí Freire (:noemi) Reporter
	Comment 2 • 9 years ago

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

	Noemí Freire (:noemi) Reporter
	Comment 3 • 9 years ago

Per comment 2, changing the subject and the dependency accordingly

	(no longer active)
	Comment 4 • 9 years ago

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!

	Anne (:annevk)
	Comment 5 • 9 years ago

Nikhil's suggestion was accepted.

	Ben Kelly [:bkelly, not reviewing] Assignee
	Comment 6 • 9 years ago

I'll start looking at this tomorrow.

	Ben Kelly [:bkelly, not reviewing] Assignee
	Comment 7 • 9 years ago

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.

	Ben Kelly [:bkelly, not reviewing] Assignee
	Comment 8 • 9 years ago

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().

	Ben Kelly [:bkelly, not reviewing] Assignee
	Comment 9 • 9 years ago

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.

	Pulsebot
	Comment 10 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/50b8122fd926

	Ben Kelly [:bkelly, not reviewing] Assignee
	Comment 11 • 9 years ago

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.

	Carsten Book [:Tomcat]
	Comment 12 • 9 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/50b8122fd926

	Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)
	Comment 13 • 9 years ago

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.

	Carsten Book [:Tomcat]
	Comment 14 • 9 years ago
bugherder uplift

https://hg.mozilla.org/releases/mozilla-aurora/rev/ffdc3e33fc9e