Thank you for reporting this.

As you mentioned, WorkerThread::TerminateAllWorkersForTesting() is an irregular code path to terminate workers. On the path, there is no chance that thread owners (e.g., WebSharedWorkerImpl, WebEmbeddedWorkerImpl) detect thread termination, and that results in such the failure.

Looks like the easiest fix is to remove the DCHECK from the dtor of WebEmbeddedWorkerImpl, but I think it could hide real bugs. I'll consider other possible ways, for example, to make WebEmbeddedWorkerImpl observe WorkerThreadLifecycleContext to detect thread termination.

I'm not sure if the leak detector needs to stop workers forcefully.
It makes sense to trigger GC on the target page for stabilizing the result, but I don't understand how stopping workers helps stabilizing the result since workers and pages doesn't share a context.
Also, I'm worried that forceful termination may break the page's behavior, so it may fall into a bad state. If the page could continue to work after the leak detection, it won't work if we terminate all of workers. 
Is it okay for that use case?

// Changed the type since it seems more like a feature request than an existing bug.

Let me hand over this to shimazu@ because I'll be OOO 2018Q1.

Just another idea to terminate threads via proper code path:

1) WorkerThread provides WorkerThreadOwner interface like this:

class WorkerThreadOwner {
 public:
  void WorkerThreadOwner(WorkerThread* thread) {
    thread->RegisterOwner(this);
  }

  virtual void ~WorkerThreadOwner() {
    thread->UnregisterOwner(this);
  }

  virtual void RequestToTerminate();
};

2) Each thread owner or something implements the interface:

class WebEmbeddedWorkerImpl : public WorkerThreadOwner {
 public:
  void RequestToTerminate() override {
    // Do service-worker-specific cleanup.

    // ..., and then terminate the thread.
    thread->Terminate();
  }
};

3) LeakDetector terminates the threads via WorkerThreadOwner:

void WorkerThread::TerminateAllWorkersForTesting() {
  // ...

  for (auto* owner : GetRegisteredOwners())
    owner->RequestToTerminate();

  // ...
}

As we chatted offline yesterday, we have two ways to terminate workers gracefully. 
- Terminate (service|shared) workers from the regular path (e.g. using DevTools API)
- Add some codes to terminate (service|shared|dedicated|...) workers living on the same process

I think it's simpler to call DevTools API than add codes only for the leak detector if the leak detector bot is already using DevTools API, but I'm not quite familiar with how it works.

Updated summary of this issue and changed the owner for the investigation around that.
If you think we need to add the termination codes, feel free to pass this to me again:) 

At the end of 2017, we've talked how to terminate the workers appropriately.
The LeakDetector lets the frame navigates to about:blank, so SharedWorkers and DedicatedWorkers should be terminated afterwards, and ServiceWorker termination can be triggered by DevTools before triggering leak detection via DevTools protocol.
Thus, what we need here is some logic to wait for termination of all worker threads. I'll start to think more of it.

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

commit b4811d08e6d426510bee52ff0817939d083602bd
Author: Makoto Shimazu <shimazu@chromium.org>
Date: Fri Jan 12 06:55:14 2018

Clean termination of service workers when testing

Currently WorkerThread::TerminateAllWorkersForTesting() immediately calls
WorkerThread::Terminate(), and it hits DCHECK(asked_to_terminate_) in the dtor
of WebEmbeddedWorkerImpl. This patch fixes the cleanup procedure by adding
TerminateForTesting which can be overriden by each WorkerThread variants. As the
result, service workers can be terminated through the regular path.

Bug:  795665 
Change-Id: Ic343d61832bc085148aac8ad8d416a02d72e2adb
Reviewed-on: https://chromium-review.googlesource.com/861587
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Commit-Queue: Makoto Shimazu <shimazu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528899}
[modify] https://crrev.com/b4811d08e6d426510bee52ff0817939d083602bd/third_party/WebKit/Source/core/workers/WorkerThread.cpp
[modify] https://crrev.com/b4811d08e6d426510bee52ff0817939d083602bd/third_party/WebKit/Source/core/workers/WorkerThread.h
[modify] https://crrev.com/b4811d08e6d426510bee52ff0817939d083602bd/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerGlobalScopeProxy.cpp
[modify] https://crrev.com/b4811d08e6d426510bee52ff0817939d083602bd/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerGlobalScopeProxy.h
[modify] https://crrev.com/b4811d08e6d426510bee52ff0817939d083602bd/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerThread.cpp
[modify] https://crrev.com/b4811d08e6d426510bee52ff0817939d083602bd/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerThread.h

I think we can close this issue since the leak detector works.