Note, chrome's behavior matches a previous version of the spec.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9190eb27531ee100bcb83450c9c32c5568531afd

commit 9190eb27531ee100bcb83450c9c32c5568531afd
Author: mike <mike@mikepennisi.com>
Date: Thu May 18 03:30:18 2017

Upstream service worker "register" tests to WPT

**register-default-scope**

This test exists in both WPT and the Chromium project tree. Because they
differ only in implementation details of the testing infrastructure, the
Chromium-specific version may be removed without affecting test coverage
in the Chromium project.

**register-link-element**

This test was previously re-located to the Web Platform Tests project
[1], but its corresponding "expectations" file was mistakenly persisted.
Remove that file.

[1] See commit e46007715a723c2219f49db6199f1af09cbbc7db in the Chromium
    project

**register-same-scope-different-script-url**

This test exists in both WPT and the Chromium project tree. The WPT
version is slightly weaker because it accounts for a condition that is
not possible according to the latest version of the Service Workers
specification [2].

> # Install
>
> [...]
>
> 17. Run the Update Registration State algorithm passing registration,
>     "installing" and null as the arguments.
> 18. Run the Update Worker State algorithm passing registration’s
>     waiting worker and installed as the arguments.
> 19. Invoke Finish Job with job.
> 20. Wait for all the tasks queued by Update Worker State invoked in
>     this algorithm have executed.
> 21. Invoke Try Activate with registration.

Due to step 20, worker activation does not occur until all prior tasks
(which includes microtasks such as the Promise handler microtask created
in the test's body) have executed.

Remove the conditional logic from the WPT version and remove the
Chromium test file.

[2] https://w3c.github.io/ServiceWorker/#installation-algorithm,
    retrieved on 2017-05-16

**register-wait-forever-in-install-worker**

The Web Platform Tests project maintains a more rigorous version of this
test, but because that version currently fails in Chromium, this version
cannot be removed without negatively effecting test coverage.

Re-name the test file and add an in-line comment to document the test's
deprecated status.

**register-foreign-fetch-error**

Update URLs to suitable values for the Web Platform Tests project.

BUG= 688116 , 723037
R=falken@chromium.org

Review-Url: https://codereview.chromium.org/2892473003
Cr-Commit-Position: refs/heads/master@{#472636}

[add] https://crrev.com/9190eb27531ee100bcb83450c9c32c5568531afd/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/register-foreign-fetch-errors.https.html
[modify] https://crrev.com/9190eb27531ee100bcb83450c9c32c5568531afd/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/register-same-scope-different-script-url.https.html
[rename] https://crrev.com/9190eb27531ee100bcb83450c9c32c5568531afd/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/resources/register-foreign-fetch-errors-worker.js
[rename] https://crrev.com/9190eb27531ee100bcb83450c9c32c5568531afd/third_party/WebKit/LayoutTests/http/tests/serviceworker/chromium.register-wait-forever-in-install-worker.html
[delete] https://crrev.com/8aaa656f06ce8379a55bf81fa4cdca123ff43485/third_party/WebKit/LayoutTests/http/tests/serviceworker/register-default-scope.html
[delete] https://crrev.com/8aaa656f06ce8379a55bf81fa4cdca123ff43485/third_party/WebKit/LayoutTests/http/tests/serviceworker/register-foreign-fetch-errors.html
[delete] https://crrev.com/8aaa656f06ce8379a55bf81fa4cdca123ff43485/third_party/WebKit/LayoutTests/http/tests/serviceworker/register-link-element-expected.txt
[delete] https://crrev.com/8aaa656f06ce8379a55bf81fa4cdca123ff43485/third_party/WebKit/LayoutTests/http/tests/serviceworker/register-same-scope-different-script-url.html

@falken, I'd like to investigate this issue, can I own it?

Yes, I think to fix the test the problem is DoomInstallingWorkerIfNeeded(). You should just be able to remove this function and the test should pass.

I'm a bit worried though about the lack of any controls for how long a registration job lasts. If for whatever reason something gets stuck, the user will be stuck forever and can't register/update/unregister a new service worker at that scope.

It may be reasonable to set a 30 minute timeout or so on any active job.

Hi falken

I found there is timer in ServiceWorkerVersion::OnTimeoutTimer for stopping the service worker if time out.
I used the code of the test case(wait-forever-in-install-worker.js), to check if the waitforever SW would be stopped, the result is yes.

And after changing the content of sw.js to correct, the sw is installed normally.

Does above means we don't need another timer as you mentioned?

However, I also found the code "if (GetTickDuration(start_time_) > start_limit) " will never be executed, I think there should be a issue? 

The code never run because start_time_ is always null, it was actually already set in StartTimeoutTimer, but cleared before the timer start. I am investigating why the sw still be stopped in this case.

Thanks for investigating this. Yes, I expected that the service worker itself would timeout if was running for too long. However I still think the register job queue itself should have its own timeout, in case something somewhere else gets stuck (like reading from the db).

It's kind of a redundant safety mechanism, but if this job queue gets stuck the user can never update or unregister their service worker, so it'd negate a "kill switch" mechanism when a site tries to back out off service workers.

I'm interested in how start_time_ is cleared before timer start. My brief look at the code can't find where that would happen, except in cases it is cleared because DevTools is opened.

>I'm interested in how start_time_ is cleared before timer start.

My debugging is that the code should be:
StartWork --> DidEnsureLiveRegistrationForStartWorker --> add RecordStartWorkerResult to callbacks --> StartWorkerInternal --> StartTimeoutTimer[here set start_time_ and add OnTimeoutTimer] --> OnStarted --> FinishStartWorker --> RecordStartWorkerResult [here start_time_ is cleared] --> OnTimeoutTimer [here the code "if (GetTickDuration(start_time_) > start_limit) " never be executed]

However, if the start_time_ is *not* cleared, the chromium would crashed, because of the DCHECK to EmbeddedWorkerStatus in OnTimeoutTimer.

I am not understand below two questions yet:
1) in the current implementation, how the timeout timer make the forever pending SW to be unregistered?
2) why the DCHECK in OnTimeoutTimer is to check EmbeddedWorkerStatus::STARTING||STOPPING instead of RUNNING statue?

OnTimeoutTimer is trying to detect a worker that got stuck starting up. So start_time holds the time the worker started starting up. Once the worker finsihed starting up (FinishStartWorker -> RecordStartWorkerResult), the worker is not stuck starting up, so we clear start_time.

That's why it's checking that, if start_time is not cleared, the worker is still STARTING or STOPPING.

Are you sure the service worker you're looking at is stalled in starting, or if it's stalled in the install event? For the test wait-forever-in-install-worker.js, I think what's happening is the worker finishes starting up and then it gets the install event. It's the install event that's timing out. ServiceWorkerVersion::StartRequest() gives the request a kRequestTimeout (5 min) timeout by default. 

Thanks for clarification.
You are right, the service worker is stalled in the install event, the status is always "installing".

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ddf5acab903eaca3a63d65b0558c6bf7680864d8

commit ddf5acab903eaca3a63d65b0558c6bf7680864d8
Author: xzhan96 <xiaofeng.zhang@intel.com>
Date: Thu Jul 27 11:47:19 2017

Pass the WPT: register-wait-forever-in-install-worker.https.html

This test failure is because of spec changes. Current version requires
that algorithm execution should hold if the job queue still contains
the previously-scheduled "register" job.

This patch actually reverts crrev.com/817643002, but adds a timer to
control the front job of each queue, in case somewhere gets stuck,
like reading from the db, so that can't register/update/unregister a
new service worker at that scope.

BUG=723037

Change-Id: I5c64314628fb025b190ea03a42d85dbacc3a6d94
Reviewed-on: https://chromium-review.googlesource.com/560633
Commit-Queue: Xiaofeng Zhang <xiaofeng.zhang@intel.com>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489901}
[modify] https://crrev.com/ddf5acab903eaca3a63d65b0558c6bf7680864d8/content/browser/service_worker/service_worker_job_coordinator.cc
[modify] https://crrev.com/ddf5acab903eaca3a63d65b0558c6bf7680864d8/content/browser/service_worker/service_worker_job_coordinator.h
[modify] https://crrev.com/ddf5acab903eaca3a63d65b0558c6bf7680864d8/content/browser/service_worker/service_worker_job_unittest.cc
[modify] https://crrev.com/ddf5acab903eaca3a63d65b0558c6bf7680864d8/content/browser/service_worker/service_worker_register_job.cc
[modify] https://crrev.com/ddf5acab903eaca3a63d65b0558c6bf7680864d8/content/browser/service_worker/service_worker_register_job.h
[modify] https://crrev.com/ddf5acab903eaca3a63d65b0558c6bf7680864d8/content/browser/service_worker/service_worker_register_job_base.h
[modify] https://crrev.com/ddf5acab903eaca3a63d65b0558c6bf7680864d8/content/browser/service_worker/service_worker_unregister_job.cc
[modify] https://crrev.com/ddf5acab903eaca3a63d65b0558c6bf7680864d8/content/browser/service_worker/service_worker_unregister_job.h
[delete] https://crrev.com/4bd3e149764cb4f8096bed87950946cf4bcdfe0e/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/register-wait-forever-in-install-worker.https-expected.txt
[delete] https://crrev.com/4bd3e149764cb4f8096bed87950946cf4bcdfe0e/third_party/WebKit/LayoutTests/http/tests/serviceworker/chromium.register-wait-forever-in-install-worker.html

Great!

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/10503e57dda6f97af129ccb4429150163b8dddc5

commit 10503e57dda6f97af129ccb4429150163b8dddc5
Author: xzhan96 <xiaofeng.zhang@intel.com>
Date: Mon Sep 04 03:27:21 2017

Remove the unused wait-forever-in-install-worker.js

This file should be deleted in https://chromium-review.googlesource.com/560633,
because the WPT test register-wait-forever-in-install-worker.https.html can
be passed now.

BUG=723037

Change-Id: I3f02b24127797b1dce999f0eb900866effbf127c
Reviewed-on: https://chromium-review.googlesource.com/646245
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Commit-Queue: Xiaofeng Zhang <xiaofeng.zhang@intel.com>
Cr-Commit-Position: refs/heads/master@{#499430}
[delete] https://crrev.com/413eb50d787707efa55a74ff2bfff0041da959fb/third_party/WebKit/LayoutTests/http/tests/serviceworker/resources/wait-forever-in-install-worker.js

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3b33c617981ec6a854b7558038b19627f384cddd

commit 3b33c617981ec6a854b7558038b19627f384cddd
Author: Matt Falkenhagen <falken@chromium.org>
Date: Fri Sep 15 07:43:10 2017

service worker: Revert registration timeout timer.

Revert the following due to crashes.

Revert "Remove the unused wait-forever-in-install-worker.js"
This reverts commit 10503e57dda6f97af129ccb4429150163b8dddc5.

Revert "Pass the WPT: register-wait-forever-in-install-worker.https.html"
This reverts commit ddf5acab903eaca3a63d65b0558c6bf7680864d8.

Bug: 761251, 723037
Change-Id: I72235453c9465df48f637f293f3e3a122a79b567
Reviewed-on: https://chromium-review.googlesource.com/668260
Reviewed-by: Tsuyoshi Horo <horo@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502189}
[modify] https://crrev.com/3b33c617981ec6a854b7558038b19627f384cddd/content/browser/service_worker/service_worker_job_coordinator.cc
[modify] https://crrev.com/3b33c617981ec6a854b7558038b19627f384cddd/content/browser/service_worker/service_worker_job_coordinator.h
[modify] https://crrev.com/3b33c617981ec6a854b7558038b19627f384cddd/content/browser/service_worker/service_worker_job_unittest.cc
[modify] https://crrev.com/3b33c617981ec6a854b7558038b19627f384cddd/content/browser/service_worker/service_worker_register_job.cc
[modify] https://crrev.com/3b33c617981ec6a854b7558038b19627f384cddd/content/browser/service_worker/service_worker_register_job.h
[modify] https://crrev.com/3b33c617981ec6a854b7558038b19627f384cddd/content/browser/service_worker/service_worker_register_job_base.h
[modify] https://crrev.com/3b33c617981ec6a854b7558038b19627f384cddd/content/browser/service_worker/service_worker_unregister_job.cc
[modify] https://crrev.com/3b33c617981ec6a854b7558038b19627f384cddd/content/browser/service_worker/service_worker_unregister_job.h
[add] https://crrev.com/3b33c617981ec6a854b7558038b19627f384cddd/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/register-wait-forever-in-install-worker.https-expected.txt
[add] https://crrev.com/3b33c617981ec6a854b7558038b19627f384cddd/third_party/WebKit/LayoutTests/http/tests/serviceworker/chromium.register-wait-forever-in-install-worker.html
[add] https://crrev.com/3b33c617981ec6a854b7558038b19627f384cddd/third_party/WebKit/LayoutTests/http/tests/serviceworker/resources/wait-forever-in-install-worker.js

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a0fc7f5c3b60c9b3c22040f8f30ca31b3c7f355c

commit a0fc7f5c3b60c9b3c22040f8f30ca31b3c7f355c
Author: Matt Falkenhagen <falken@chromium.org>
Date: Mon Sep 25 05:16:28 2017

M62: service worker: Revert registration timeout timer.

Revert the following due to crashes.

Revert "Remove the unused wait-forever-in-install-worker.js"
This reverts commit 10503e57dda6f97af129ccb4429150163b8dddc5.

Revert "Pass the WPT: register-wait-forever-in-install-worker.https.html"
This reverts commit ddf5acab903eaca3a63d65b0558c6bf7680864d8.

TBR=falken@chromium.org

(cherry picked from commit 3b33c617981ec6a854b7558038b19627f384cddd)

Bug: 761251, 723037
Change-Id: I72235453c9465df48f637f293f3e3a122a79b567
Reviewed-on: https://chromium-review.googlesource.com/668260
Reviewed-by: Tsuyoshi Horo <horo@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#502189}
Reviewed-on: https://chromium-review.googlesource.com/680772
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/branch-heads/3202@{#420}
Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
[modify] https://crrev.com/a0fc7f5c3b60c9b3c22040f8f30ca31b3c7f355c/content/browser/service_worker/service_worker_job_coordinator.cc
[modify] https://crrev.com/a0fc7f5c3b60c9b3c22040f8f30ca31b3c7f355c/content/browser/service_worker/service_worker_job_coordinator.h
[modify] https://crrev.com/a0fc7f5c3b60c9b3c22040f8f30ca31b3c7f355c/content/browser/service_worker/service_worker_job_unittest.cc
[modify] https://crrev.com/a0fc7f5c3b60c9b3c22040f8f30ca31b3c7f355c/content/browser/service_worker/service_worker_register_job.cc
[modify] https://crrev.com/a0fc7f5c3b60c9b3c22040f8f30ca31b3c7f355c/content/browser/service_worker/service_worker_register_job.h
[modify] https://crrev.com/a0fc7f5c3b60c9b3c22040f8f30ca31b3c7f355c/content/browser/service_worker/service_worker_register_job_base.h
[modify] https://crrev.com/a0fc7f5c3b60c9b3c22040f8f30ca31b3c7f355c/content/browser/service_worker/service_worker_unregister_job.cc
[modify] https://crrev.com/a0fc7f5c3b60c9b3c22040f8f30ca31b3c7f355c/content/browser/service_worker/service_worker_unregister_job.h
[add] https://crrev.com/a0fc7f5c3b60c9b3c22040f8f30ca31b3c7f355c/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/register-wait-forever-in-install-worker.https-expected.txt
[add] https://crrev.com/a0fc7f5c3b60c9b3c22040f8f30ca31b3c7f355c/third_party/WebKit/LayoutTests/http/tests/serviceworker/chromium.register-wait-forever-in-install-worker.html

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/292f7e3e4d44f4a2f00939a436f39a7b1a5b6c79

commit 292f7e3e4d44f4a2f00939a436f39a7b1a5b6c79
Author: Matt Falkenhagen <falken@chromium.org>
Date: Tue Sep 26 06:47:30 2017

M62: Fix bad merge from a0fc7f5c3b60c9b3c22040

Bug: 761251, 723037, 768201
Change-Id: Ie4c76689cf557224abefb003d509b470547edb4d
TBR: abdulsyed
Reviewed-on: https://chromium-review.googlesource.com/683999
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/branch-heads/3202@{#445}
Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
[modify] https://crrev.com/292f7e3e4d44f4a2f00939a436f39a7b1a5b6c79/content/browser/service_worker/service_worker_job_unittest.cc

Reopened because the fix is reverted as it causes crash.