Reland "[DevTools] Properly notify devtools agent host when service worker is destroyed"

This reverts commit c6d15649.

Reason for revert: the problem with gmail offline service workers mentioned in the bug has been fixed by http://crrev.com/c/2213550 and http://crrev.com/c/2213783.

Bug: 1085150

Original change's description:
> Revert "[DevTools] Properly notify devtools agent host when service worker is destroyed"
> 
> This reverts commit 1537d629.
> 
> Reason for revert: potentially breaks gmail reloading when disabling offline mode with DevTools open.
> 
> Bug: 1085150
> 
> Original change's description:
> > [DevTools] Properly notify devtools agent host when service worker is destroyed
> > 
> > ServiceWorkerDevToolsAgentHost now stores thread-safe reference to
> > ServiceWorkerContextWrapper instead of ServiceWorkerContextCore that
> > could be deleted asynchronously on the service worker core thread.
> > 
> > WorkerDestroyed notification is now sent to the observers of
> > ServiceWorkerDevToolsManager iff the worker version is both stopped
> > and doomed. Each worker can be terminated (and restarted) multiple times
> > and will result in a sequence of targetCrashed/targetReloadedAfterCrash
> > events in the protocol but the instance of its
> > ServiceWorkerDevToolsAgentHost will survive restarts and all connected
> > sessions will be preserved.
> > 
> > It fixes the events generated in the protocol for service workers from
> > targetCreated, (detachedFromTarget, attachedToTarget, targetReloadedAfterCrash)* to
> > targetCreated, (targetCrashed/targetReloadedAfterCrash)*, targetDestroyed
> > and avoids inadvertent session terminations when workers are stopped and
> > also spurious targetReloadedAfterCrash events.
> > 
> > Bug: none
> > Change-Id: I1125604d08b7842f7f8d74751091193b9af2cb59
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2099590
> > Commit-Queue: Yury Semikhatsky <yurys@chromium.org>
> > Reviewed-by: Tsuyoshi Horo <horo@chromium.org>
> > Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#750975}
> 
> TBR=dgozman@chromium.org,horo@chromium.org,caseq@chromium.org,yurys@chromium.org,einbinder@chromium.org
> 
> # Not skipping CQ checks because original CL landed > 1 day ago.
> 
> Bug: none
> Change-Id: Ibba7ce840b8c8b73c32086d141005306591e21cc
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2213448
> Reviewed-by: Yury Semikhatsky <yurys@chromium.org>
> Commit-Queue: Yury Semikhatsky <yurys@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#771365}

TBR=dgozman@chromium.org,horo@chromium.org,caseq@chromium.org,yurys@chromium.org,einbinder@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1085150, none
Change-Id: I7afbb1f53a4940f931ad0722b52da3b307223663
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2213817Reviewed-by: Yury Semikhatsky <yurys@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Commit-Queue: Yury Semikhatsky <yurys@chromium.org>
Cr-Commit-Position: refs/heads/master@{#771849}

content/browser/devtools/protocol/target_auto_attacher.cc

@@ -94,9 +94,10 @@ base::flat_set<GURL> GetFrameUrls(RenderFrameHostImpl* render_frame_host) {
   //    (from WorkerCreated). See also https://crbug.com/907072
   //
   // We are not attaching in the following case:
-  // 4. Frame is trying to navigate and we _should_ pick up an existing SW but we don't.
-  //    We _could_ do this, but since we are not pausing the navigation, there
-  //    is no principal difference between picking up SW earlier or later.
+  // 4. Frame is trying to navigate and we _should_ pick up an existing SW but
+  //    we don't. We _could_ do this, but since we are not pausing the
+  //    navigation, there is no principal difference between picking up SW
+  //    earlier or later.
   //
   // We also try to detach from SW picked up for [3] if navigation has failed
   // (from DidFinishNavigation).
@@ -362,16 +363,6 @@ void TargetAutoAttacher::WorkerCreated(ServiceWorkerDevToolsAgentHost* host,
   }
 }
 
-void TargetAutoAttacher::WorkerVersionInstalled(
-    ServiceWorkerDevToolsAgentHost* host) {
-  ReattachServiceWorkers(false);
-}
-
-void TargetAutoAttacher::WorkerVersionDoomed(
-    ServiceWorkerDevToolsAgentHost* host) {
-  ReattachServiceWorkers(false);
-}
-
 void TargetAutoAttacher::WorkerDestroyed(ServiceWorkerDevToolsAgentHost* host) {
   if (is_browser_mode()) {
     auto_attached_hosts_.erase(base::WrapRefCounted(host));

content/browser/devtools/protocol/target_auto_attacher.h

@@ -60,8 +60,6 @@ class TargetAutoAttacher : public ServiceWorkerDevToolsManager::Observer {
   // ServiceWorkerDevToolsManager::Observer implementation.
   void WorkerCreated(ServiceWorkerDevToolsAgentHost* host,
                      bool* should_pause_on_start) override;
-  void WorkerVersionInstalled(ServiceWorkerDevToolsAgentHost* host) override;
-  void WorkerVersionDoomed(ServiceWorkerDevToolsAgentHost* host) override;
   void WorkerDestroyed(ServiceWorkerDevToolsAgentHost* host) override;
 
   void UpdateFrames();

content/browser/devtools/service_worker_devtools_agent_host.cc

@@ -18,7 +18,7 @@
 #include "content/browser/devtools/protocol/schema_handler.h"
 #include "content/browser/devtools/protocol/target_handler.h"
 #include "content/browser/devtools/service_worker_devtools_manager.h"
-#include "content/browser/service_worker/service_worker_context_core.h"
+#include "content/browser/service_worker/service_worker_context_wrapper.h"
 #include "content/browser/service_worker/service_worker_version.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
@@ -29,31 +29,26 @@ namespace content {
 namespace {
 
 void TerminateServiceWorkerOnCoreThread(
-    base::WeakPtr<ServiceWorkerContextCore> context_weak,
+    scoped_refptr<ServiceWorkerContextWrapper> context,
     int64_t version_id) {
-  if (ServiceWorkerContextCore* context = context_weak.get()) {
-    if (ServiceWorkerVersion* version = context->GetLiveVersion(version_id))
-      version->StopWorker(base::DoNothing());
-  }
+  if (ServiceWorkerVersion* version = context->GetLiveVersion(version_id))
+    version->StopWorker(base::DoNothing());
 }
 
 void SetDevToolsAttachedOnCoreThread(
-    base::WeakPtr<ServiceWorkerContextCore> context_weak,
+    scoped_refptr<ServiceWorkerContextWrapper> context,
     int64_t version_id,
     bool attached) {
-  if (ServiceWorkerContextCore* context = context_weak.get()) {
-    if (ServiceWorkerVersion* version = context->GetLiveVersion(version_id))
-      version->SetDevToolsAttached(attached);
-  }
+  if (ServiceWorkerVersion* version = context->GetLiveVersion(version_id))
+    version->SetDevToolsAttached(attached);
 }
 
 void UpdateLoaderFactoriesOnCoreThread(
-    base::WeakPtr<ServiceWorkerContextCore> context_weak,
+    scoped_refptr<ServiceWorkerContextWrapper> context,
     int64_t version_id,
     std::unique_ptr<blink::PendingURLLoaderFactoryBundle> script_bundle,
     std::unique_ptr<blink::PendingURLLoaderFactoryBundle> subresource_bundle) {
-  auto* version =
-      context_weak ? context_weak->GetLiveVersion(version_id) : nullptr;
+  auto* version = context->GetLiveVersion(version_id);
   if (!version)
     return;
   version->embedded_worker()->UpdateLoaderFactories(
@@ -65,8 +60,7 @@ void UpdateLoaderFactoriesOnCoreThread(
 ServiceWorkerDevToolsAgentHost::ServiceWorkerDevToolsAgentHost(
     int worker_process_id,
     int worker_route_id,
-    const ServiceWorkerContextCore* context,
-    base::WeakPtr<ServiceWorkerContextCore> context_weak,
+    scoped_refptr<ServiceWorkerContextWrapper> context_wrapper,
     int64_t version_id,
     const GURL& url,
     const GURL& scope,
@@ -81,8 +75,7 @@ ServiceWorkerDevToolsAgentHost::ServiceWorkerDevToolsAgentHost(
       devtools_worker_token_(devtools_worker_token),
       worker_process_id_(worker_process_id),
       worker_route_id_(worker_route_id),
-      context_(context),
-      context_weak_(context_weak),
+      context_wrapper_(context_wrapper),
       version_id_(version_id),
       url_(url),
       scope_(scope),
@@ -94,8 +87,7 @@ ServiceWorkerDevToolsAgentHost::ServiceWorkerDevToolsAgentHost(
 }
 
 BrowserContext* ServiceWorkerDevToolsAgentHost::GetBrowserContext() {
-  RenderProcessHost* rph = RenderProcessHost::FromID(worker_process_id_);
-  return rph ? rph->GetBrowserContext() : nullptr;
+  return context_wrapper_->browser_context();
 }
 
 std::string ServiceWorkerDevToolsAgentHost::GetType() {
@@ -120,7 +112,7 @@ void ServiceWorkerDevToolsAgentHost::Reload() {
 bool ServiceWorkerDevToolsAgentHost::Close() {
   RunOrPostTaskOnThread(FROM_HERE, ServiceWorkerContext::GetCoreThreadId(),
                         base::BindOnce(&TerminateServiceWorkerOnCoreThread,
-                                       context_weak_, version_id_));
+                                       context_wrapper_, version_id_));
   return true;
 }
 
@@ -132,12 +124,6 @@ void ServiceWorkerDevToolsAgentHost::WorkerVersionDoomed() {
   version_doomed_time_ = base::Time::Now();
 }
 
-bool ServiceWorkerDevToolsAgentHost::Matches(
-    const ServiceWorkerContextCore* context,
-    int64_t version_id) {
-  return context_ == context && version_id_ == version_id;
-}
-
 ServiceWorkerDevToolsAgentHost::~ServiceWorkerDevToolsAgentHost() {
   ServiceWorkerDevToolsManager::GetInstance()->AgentHostDestroyed(this);
 }
@@ -195,7 +181,7 @@ void ServiceWorkerDevToolsAgentHost::WorkerRestarted(int worker_process_id,
   worker_route_id_ = worker_route_id;
 }
 
-void ServiceWorkerDevToolsAgentHost::WorkerDestroyed() {
+void ServiceWorkerDevToolsAgentHost::WorkerStopped() {
   DCHECK_NE(WORKER_TERMINATED, state_);
   state_ = WORKER_TERMINATED;
   for (auto* inspector : protocol::InspectorHandler::ForAgentHost(this))
@@ -207,9 +193,10 @@ void ServiceWorkerDevToolsAgentHost::WorkerDestroyed() {
 }
 
 void ServiceWorkerDevToolsAgentHost::UpdateIsAttached(bool attached) {
-  RunOrPostTaskOnThread(FROM_HERE, ServiceWorkerContext::GetCoreThreadId(),
-                        base::BindOnce(&SetDevToolsAttachedOnCoreThread,
-                                       context_weak_, version_id_, attached));
+  RunOrPostTaskOnThread(
+      FROM_HERE, ServiceWorkerContext::GetCoreThreadId(),
+      base::BindOnce(&SetDevToolsAttachedOnCoreThread, context_wrapper_,
+                     version_id_, attached));
 }
 
 void ServiceWorkerDevToolsAgentHost::UpdateLoaderFactories(
@@ -249,14 +236,14 @@ void ServiceWorkerDevToolsAgentHost::UpdateLoaderFactories(
       ContentBrowserClient::URLLoaderFactoryType::kServiceWorkerSubResource);
 
   if (ServiceWorkerContext::IsServiceWorkerOnUIEnabled()) {
-    UpdateLoaderFactoriesOnCoreThread(context_weak_, version_id_,
+    UpdateLoaderFactoriesOnCoreThread(context_wrapper_, version_id_,
                                       std::move(script_bundle),
                                       std::move(subresource_bundle));
     std::move(callback).Run();
   } else {
     base::PostTaskAndReply(
         FROM_HERE, {BrowserThread::IO},
-        base::BindOnce(&UpdateLoaderFactoriesOnCoreThread, context_weak_,
+        base::BindOnce(&UpdateLoaderFactoriesOnCoreThread, context_wrapper_,
                        version_id_, std::move(script_bundle),
                        std::move(subresource_bundle)),
         std::move(callback));

content/browser/devtools/service_worker_devtools_agent_host.h

@@ -31,8 +31,7 @@ class ServiceWorkerDevToolsAgentHost : public DevToolsAgentHostImpl {
   ServiceWorkerDevToolsAgentHost(
       int worker_process_id,
       int worker_route_id,
-      const ServiceWorkerContextCore* context,
-      base::WeakPtr<ServiceWorkerContextCore> context_weak,
+      scoped_refptr<ServiceWorkerContextWrapper> context_wrapper,
       int64_t version_id,
       const GURL& url,
       const GURL& scope,
@@ -60,7 +59,7 @@ class ServiceWorkerDevToolsAgentHost : public DevToolsAgentHostImpl {
       network::CrossOriginEmbedderPolicy cross_origin_embedder_policy,
       mojo::PendingRemote<network::mojom::CrossOriginEmbedderPolicyReporter>
           coep_reporter);
-  void WorkerDestroyed();
+  void WorkerStopped();
   void WorkerVersionInstalled();
   void WorkerVersionDoomed();
 
@@ -78,8 +77,9 @@ class ServiceWorkerDevToolsAgentHost : public DevToolsAgentHostImpl {
   base::Time version_doomed_time() const { return version_doomed_time_; }
 
   int64_t version_id() const { return version_id_; }
-
-  bool Matches(const ServiceWorkerContextCore* context, int64_t version_id);
+  const ServiceWorkerContextWrapper* context_wrapper() const {
+    return context_wrapper_.get();
+  }
 
  private:
   ~ServiceWorkerDevToolsAgentHost() override;
@@ -100,8 +100,7 @@ class ServiceWorkerDevToolsAgentHost : public DevToolsAgentHostImpl {
   base::UnguessableToken devtools_worker_token_;
   int worker_process_id_;
   int worker_route_id_;
-  const ServiceWorkerContextCore* context_;
-  base::WeakPtr<ServiceWorkerContextCore> context_weak_;
+  scoped_refptr<ServiceWorkerContextWrapper> context_wrapper_;
   int64_t version_id_;
   GURL url_;
   GURL scope_;

content/browser/devtools/service_worker_devtools_manager.cc

@@ -7,6 +7,7 @@
 #include "content/browser/devtools/protocol/network_handler.h"
 #include "content/browser/devtools/protocol/page_handler.h"
 #include "content/browser/devtools/service_worker_devtools_agent_host.h"
+#include "content/browser/service_worker/service_worker_context_wrapper.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/render_process_host.h"
 #include "ipc/ipc_listener.h"
@@ -43,11 +44,10 @@ void ServiceWorkerDevToolsManager::AddAllAgentHostsForBrowserContext(
   }
 }
 
-void ServiceWorkerDevToolsManager::WorkerCreated(
+void ServiceWorkerDevToolsManager::WorkerStarting(
     int worker_process_id,
     int worker_route_id,
-    const ServiceWorkerContextCore* context,
-    base::WeakPtr<ServiceWorkerContextCore> context_weak,
+    scoped_refptr<ServiceWorkerContextWrapper> context_wrapper,
     int64_t version_id,
     const GURL& url,
     const GURL& scope,
@@ -62,36 +62,31 @@ void ServiceWorkerDevToolsManager::WorkerCreated(
   const WorkerId worker_id(worker_process_id, worker_route_id);
   DCHECK(live_hosts_.find(worker_id) == live_hosts_.end());
 
-  auto it = std::find_if(
-      terminated_hosts_.begin(), terminated_hosts_.end(),
-      [&context, &version_id](ServiceWorkerDevToolsAgentHost* agent_host) {
-        return agent_host->Matches(context, version_id);
-      });
-  if (it == terminated_hosts_.end()) {
-    *devtools_worker_token = base::UnguessableToken::Create();
-    scoped_refptr<ServiceWorkerDevToolsAgentHost> host =
-        new ServiceWorkerDevToolsAgentHost(
-            worker_process_id, worker_route_id, context, context_weak,
-            version_id, url, scope, is_installed_version,
-            cross_origin_embedder_policy, std::move(coep_reporter),
-            *devtools_worker_token);
-    live_hosts_[worker_id] = host;
-    *pause_on_start = debug_service_worker_on_start_;
-    for (auto& observer : observer_list_) {
-      bool should_pause_on_start = false;
-      observer.WorkerCreated(host.get(), &should_pause_on_start);
-      if (should_pause_on_start)
-        *pause_on_start = true;
-    }
+  scoped_refptr<ServiceWorkerDevToolsAgentHost> agent_host =
+      TakeStoppedHost(context_wrapper.get(), version_id);
+  if (agent_host) {
+    live_hosts_[worker_id] = agent_host;
+    agent_host->WorkerRestarted(worker_process_id, worker_route_id);
+    *pause_on_start = agent_host->IsAttached();
+    *devtools_worker_token = agent_host->devtools_worker_token();
     return;
   }
 
-  ServiceWorkerDevToolsAgentHost* agent_host = *it;
-  terminated_hosts_.erase(it);
-  live_hosts_[worker_id] = agent_host;
-  agent_host->WorkerRestarted(worker_process_id, worker_route_id);
-  *pause_on_start = agent_host->IsAttached();
-  *devtools_worker_token = agent_host->devtools_worker_token();
+  *devtools_worker_token = base::UnguessableToken::Create();
+  scoped_refptr<ServiceWorkerDevToolsAgentHost> host =
+      new ServiceWorkerDevToolsAgentHost(
+          worker_process_id, worker_route_id, std::move(context_wrapper),
+          version_id, url, scope, is_installed_version,
+          cross_origin_embedder_policy, std::move(coep_reporter),
+          *devtools_worker_token);
+  live_hosts_[worker_id] = host;
+  *pause_on_start = debug_service_worker_on_start_;
+  for (auto& observer : observer_list_) {
+    bool should_pause_on_start = false;
+    observer.WorkerCreated(host.get(), &should_pause_on_start);
+    if (should_pause_on_start)
+      *pause_on_start = true;
+  }
 }
 
 void ServiceWorkerDevToolsManager::WorkerReadyForInspection(
@@ -123,9 +118,8 @@ void ServiceWorkerDevToolsManager::UpdateCrossOriginEmbedderPolicy(
   auto it = live_hosts_.find(worker_id);
   if (it == live_hosts_.end())
     return;
-  scoped_refptr<ServiceWorkerDevToolsAgentHost> host = it->second;
-  host->UpdateCrossOriginEmbedderPolicy(std::move(cross_origin_embedder_policy),
-                                        std::move(coep_reporter));
+  it->second->UpdateCrossOriginEmbedderPolicy(
+      std::move(cross_origin_embedder_policy), std::move(coep_reporter));
 }
 
 void ServiceWorkerDevToolsManager::WorkerVersionInstalled(int worker_process_id,
@@ -135,27 +129,34 @@ void ServiceWorkerDevToolsManager::WorkerVersionInstalled(int worker_process_id,
   auto it = live_hosts_.find(worker_id);
   if (it == live_hosts_.end())
     return;
-  scoped_refptr<ServiceWorkerDevToolsAgentHost> host = it->second;
-  host->WorkerVersionInstalled();
-  for (auto& observer : observer_list_)
-    observer.WorkerVersionInstalled(host.get());
+  it->second->WorkerVersionInstalled();
 }
 
-void ServiceWorkerDevToolsManager::WorkerVersionDoomed(int worker_process_id,
-                                                       int worker_route_id) {
+void ServiceWorkerDevToolsManager::WorkerVersionDoomed(
+    int worker_process_id,
+    int worker_route_id,
+    scoped_refptr<ServiceWorkerContextWrapper> context_wrapper,
+    int64_t version_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   const WorkerId worker_id(worker_process_id, worker_route_id);
   auto it = live_hosts_.find(worker_id);
-  if (it == live_hosts_.end())
+  if (it != live_hosts_.end()) {
+    it->second->WorkerVersionDoomed();
+    return;
+  }
+  scoped_refptr<ServiceWorkerDevToolsAgentHost> host =
+      TakeStoppedHost(context_wrapper.get(), version_id);
+  if (!host)
     return;
-  scoped_refptr<ServiceWorkerDevToolsAgentHost> host = it->second;
   host->WorkerVersionDoomed();
+  // The worker has already been stopped and since it's doomed it will never
+  // restart.
   for (auto& observer : observer_list_)
-    observer.WorkerVersionDoomed(host.get());
+    observer.WorkerDestroyed(host.get());
 }
 
-void ServiceWorkerDevToolsManager::WorkerDestroyed(int worker_process_id,
-                                                   int worker_route_id) {
+void ServiceWorkerDevToolsManager::WorkerStopped(int worker_process_id,
+                                                 int worker_route_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   const WorkerId worker_id(worker_process_id, worker_route_id);
   auto it = live_hosts_.find(worker_id);
@@ -163,21 +164,23 @@ void ServiceWorkerDevToolsManager::WorkerDestroyed(int worker_process_id,
     return;
   scoped_refptr<ServiceWorkerDevToolsAgentHost> agent_host(it->second);
   live_hosts_.erase(it);
-  terminated_hosts_.insert(agent_host.get());
-  agent_host->WorkerDestroyed();
-  for (auto& observer : observer_list_)
-    observer.WorkerDestroyed(agent_host.get());
+  agent_host->WorkerStopped();
+  if (agent_host->version_doomed_time().is_null()) {
+    stopped_hosts_.insert(agent_host.get());
+  } else {
+    // The worker version has been doomed, it will never restart.
+    for (auto& observer : observer_list_)
+      observer.WorkerDestroyed(agent_host.get());
+  }
 }
 
 void ServiceWorkerDevToolsManager::AgentHostDestroyed(
     ServiceWorkerDevToolsAgentHost* agent_host) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  auto it = terminated_hosts_.find(agent_host);
   // Might be missing during shutdown due to different
   // destruction order of this manager, service workers
   // and their agent hosts.
-  if (it != terminated_hosts_.end())
-    terminated_hosts_.erase(it);
+  stopped_hosts_.erase(agent_host);
 }
 
 void ServiceWorkerDevToolsManager::AddObserver(Observer* observer) {
@@ -248,4 +251,22 @@ void ServiceWorkerDevToolsManager::NavigationPreloadCompleted(
         request_id, protocol::Network::ResourceTypeEnum::Other, status);
 }
 
+scoped_refptr<ServiceWorkerDevToolsAgentHost>
+ServiceWorkerDevToolsManager::TakeStoppedHost(
+    const ServiceWorkerContextWrapper* context_wrapper,
+    int64_t version_id) {
+  auto it =
+      std::find_if(stopped_hosts_.begin(), stopped_hosts_.end(),
+                   [&context_wrapper,
+                    &version_id](ServiceWorkerDevToolsAgentHost* agent_host) {
+                     return agent_host->context_wrapper() == context_wrapper &&
+                            agent_host->version_id() == version_id;
+                   });
+  if (it == stopped_hosts_.end())
+    return nullptr;
+  scoped_refptr<ServiceWorkerDevToolsAgentHost> agent_host(*it);
+  stopped_hosts_.erase(it);
+  return agent_host;
+}
+
 }  // namespace content

content/browser/devtools/service_worker_devtools_manager.h

@@ -31,7 +31,7 @@ namespace content {
 
 class BrowserContext;
 class ServiceWorkerDevToolsAgentHost;
-class ServiceWorkerContextCore;
+class ServiceWorkerContextWrapper;
 
 // Manages ServiceWorkerDevToolsAgentHost's. This class lives on UI thread.
 class CONTENT_EXPORT ServiceWorkerDevToolsManager {
@@ -40,8 +40,6 @@ class CONTENT_EXPORT ServiceWorkerDevToolsManager {
    public:
     virtual void WorkerCreated(ServiceWorkerDevToolsAgentHost* host,
                                bool* should_pause_on_start) {}
-    virtual void WorkerVersionInstalled(ServiceWorkerDevToolsAgentHost* host) {}
-    virtual void WorkerVersionDoomed(ServiceWorkerDevToolsAgentHost* host) {}
     virtual void WorkerDestroyed(ServiceWorkerDevToolsAgentHost* host) {}
 
    protected:
@@ -60,11 +58,10 @@ class CONTENT_EXPORT ServiceWorkerDevToolsManager {
       BrowserContext* browser_context,
       std::vector<scoped_refptr<ServiceWorkerDevToolsAgentHost>>* result);
 
-  void WorkerCreated(
+  void WorkerStarting(
       int worker_process_id,
       int worker_route_id,
-      const ServiceWorkerContextCore* context,
-      base::WeakPtr<ServiceWorkerContextCore> context_weak,
+      scoped_refptr<ServiceWorkerContextWrapper> context_wrapper,
       int64_t version_id,
       const GURL& url,
       const GURL& scope,
@@ -87,8 +84,15 @@ class CONTENT_EXPORT ServiceWorkerDevToolsManager {
       mojo::PendingRemote<network::mojom::CrossOriginEmbedderPolicyReporter>
           coep_reporter);
   void WorkerVersionInstalled(int worker_process_id, int worker_route_id);
-  void WorkerVersionDoomed(int worker_process_id, int worker_route_id);
-  void WorkerDestroyed(int worker_process_id, int worker_route_id);
+  // If the worker instance is stopped its worker_process_id and
+  // worker_route_id will be invalid. For that case we pass context
+  // and version_id as well.
+  void WorkerVersionDoomed(
+      int worker_process_id,
+      int worker_route_id,
+      scoped_refptr<ServiceWorkerContextWrapper> context_wrapper,
+      int64_t version_id);
+  void WorkerStopped(int worker_process_id, int worker_route_id);
   void NavigationPreloadRequestSent(int worker_process_id,
                                     int worker_route_id,
                                     const std::string& request_id,
@@ -123,6 +127,10 @@ class CONTENT_EXPORT ServiceWorkerDevToolsManager {
   ServiceWorkerDevToolsManager();
   ~ServiceWorkerDevToolsManager();
 
+  scoped_refptr<ServiceWorkerDevToolsAgentHost> TakeStoppedHost(
+      const ServiceWorkerContextWrapper* context_wrapper,
+      int64_t version_id);
+
   base::ObserverList<Observer>::Unchecked observer_list_;
   bool debug_service_worker_on_start_;
 
@@ -131,7 +139,7 @@ class CONTENT_EXPORT ServiceWorkerDevToolsManager {
 
   // Clients may retain agent host for the terminated shared worker,
   // and we reconnect them when shared worker is restarted.
-  base::flat_set<ServiceWorkerDevToolsAgentHost*> terminated_hosts_;
+  base::flat_set<ServiceWorkerDevToolsAgentHost*> stopped_hosts_;
 
   DISALLOW_COPY_AND_ASSIGN(ServiceWorkerDevToolsManager);
 };

content/browser/service_worker/embedded_worker_instance.cc

@@ -91,10 +91,10 @@ void NotifyUpdateCrossOriginEmbedderPolicyOnUI(
       std::move(cross_origin_embedder_policy), std::move(coep_reporter));
 }
 
-void NotifyWorkerDestroyedOnUI(int worker_process_id, int worker_route_id) {
+void NotifyWorkerStoppedOnUI(int worker_process_id, int worker_route_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  ServiceWorkerDevToolsManager::GetInstance()->WorkerDestroyed(
-      worker_process_id, worker_route_id);
+  ServiceWorkerDevToolsManager::GetInstance()->WorkerStopped(worker_process_id,
+                                                             worker_route_id);
 }
 
 void NotifyWorkerVersionInstalledOnUI(int worker_process_id,
@@ -104,10 +104,14 @@ void NotifyWorkerVersionInstalledOnUI(int worker_process_id,
       worker_process_id, worker_route_id);
 }
 
-void NotifyWorkerVersionDoomedOnUI(int worker_process_id, int worker_route_id) {
+void NotifyWorkerVersionDoomedOnUI(
+    int worker_process_id,
+    int worker_route_id,
+    scoped_refptr<ServiceWorkerContextWrapper> context_wrapper,
+    int64_t version_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   ServiceWorkerDevToolsManager::GetInstance()->WorkerVersionDoomed(
-      worker_process_id, worker_route_id);
+      worker_process_id, worker_route_id, context_wrapper, version_id);
 }
 
 using CreateFactoryBundlesOnUICallback = base::OnceCallback<void(
@@ -223,8 +227,7 @@ void SetupOnUIThread(
         cross_origin_embedder_policy,
     blink::mojom::EmbeddedWorkerStartParamsPtr params,
     mojo::PendingReceiver<blink::mojom::EmbeddedWorkerInstanceClient> receiver,
-    ServiceWorkerContextCore* context,
-    base::WeakPtr<ServiceWorkerContextCore> weak_context,
+    scoped_refptr<ServiceWorkerContextWrapper> context_wrapper,
     const base::Optional<base::Time>& io_post_time,
     SetupProcessCallback callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
@@ -324,15 +327,15 @@ void SetupOnUIThread(
   }
   // Register to DevTools and update params accordingly.
   const int routing_id = rph->GetNextRoutingID();
-  ServiceWorkerDevToolsManager::GetInstance()->WorkerCreated(
-      process_id, routing_id, context, weak_context,
+  ServiceWorkerDevToolsManager::GetInstance()->WorkerStarting(
+      process_id, routing_id, std::move(context_wrapper),
       params->service_worker_version_id, params->script_url, params->scope,
       params->is_installed, cross_origin_embedder_policy,
       std::move(coep_reporter_for_devtools), &params->devtools_worker_token,
       &params->wait_for_debugger);
   params->service_worker_route_id = routing_id;
   // Create DevToolsProxy here to ensure that the WorkerCreated() call is
-  // balanced by DevToolsProxy's destructor calling WorkerDestroyed().
+  // balanced by DevToolsProxy's destructor calling WorkerStopped().
   devtools_proxy = std::make_unique<EmbeddedWorkerInstance::DevToolsProxy>(
       process_id, routing_id);
 
@@ -441,8 +444,10 @@ void BindCacheStorageOnUIThread(
 
 }  // namespace
 
-// Created on UI thread and moved to core thread. Proxies notifications to
-// DevToolsManager that lives on UI thread. Owned by EmbeddedWorkerInstance.
+// Created on the UI thread when the worker version is allcated a render process
+// and then moved to the core thread. It is destroyed when the worker stops.
+// Proxies notifications to DevToolsManager that lives on UI thread.
+// Owned by EmbeddedWorkerInstance.
 class EmbeddedWorkerInstance::DevToolsProxy {
  public:
   DevToolsProxy(int process_id, int agent_route_id)
@@ -453,10 +458,10 @@ class EmbeddedWorkerInstance::DevToolsProxy {
   ~DevToolsProxy() {
     DCHECK_CURRENTLY_ON(ServiceWorkerContext::GetCoreThreadId());
     if (ServiceWorkerContext::IsServiceWorkerOnUIEnabled()) {
-      NotifyWorkerDestroyedOnUI(process_id_, agent_route_id_);
+      NotifyWorkerStoppedOnUI(process_id_, agent_route_id_);
     } else {
       ui_task_runner_->PostTask(
-          FROM_HERE, base::BindOnce(NotifyWorkerDestroyedOnUI, process_id_,
+          FROM_HERE, base::BindOnce(NotifyWorkerStoppedOnUI, process_id_,
                                     agent_route_id_));
     }
   }
@@ -489,17 +494,6 @@ class EmbeddedWorkerInstance::DevToolsProxy {
     }
   }
 
-  void NotifyWorkerVersionDoomed() {
-    DCHECK_CURRENTLY_ON(ServiceWorkerContext::GetCoreThreadId());
-    if (ServiceWorkerContext::IsServiceWorkerOnUIEnabled()) {
-      NotifyWorkerVersionDoomedOnUI(process_id_, agent_route_id_);
-    } else {
-      ui_task_runner_->PostTask(
-          FROM_HERE, base::BindOnce(NotifyWorkerVersionDoomedOnUI, process_id_,
-                                    agent_route_id_));
-    }
-  }
-
   bool ShouldNotifyWorkerStopIgnored() const {
     return !worker_stop_ignored_notified_;
   }
@@ -698,8 +692,8 @@ class EmbeddedWorkerInstance::StartTask {
       SetupOnUIThread(
           instance_->embedded_worker_id(), process_manager,
           can_use_existing_process, cross_origin_embedder_policy,
-          std::move(params), std::move(receiver_), context.get(), context,
-          base::nullopt,
+          std::move(params), std::move(receiver_),
+          base::WrapRefCounted(context->wrapper()), base::nullopt,
           base::BindOnce(&StartTask::OnSetupCompleted,
                          weak_factory_.GetWeakPtr(), process_manager));
     } else {
@@ -709,7 +703,7 @@ class EmbeddedWorkerInstance::StartTask {
               &SetupOnUIThread, instance_->embedded_worker_id(),
               process_manager, can_use_existing_process,
               cross_origin_embedder_policy, std::move(params),
-              std::move(receiver_), context.get(), context,
+              std::move(receiver_), base::WrapRefCounted(context->wrapper()),
               base::make_optional<base::Time>(base::Time::Now()),
               base::BindOnce(&StartTask::OnSetupCompleted,
                              weak_factory_.GetWeakPtr(), process_manager)));
@@ -858,7 +852,6 @@ class EmbeddedWorkerInstance::StartTask {
 
 EmbeddedWorkerInstance::~EmbeddedWorkerInstance() {
   DCHECK_CURRENTLY_ON(ServiceWorkerContext::GetCoreThreadId());
-  devtools_proxy_.reset();
   ReleaseProcess();
 }
 
@@ -1072,8 +1065,18 @@ void EmbeddedWorkerInstance::OnWorkerVersionInstalled() {
 }
 
 void EmbeddedWorkerInstance::OnWorkerVersionDoomed() {
-  if (devtools_proxy_)
-    devtools_proxy_->NotifyWorkerVersionDoomed();
+  if (ServiceWorkerContext::IsServiceWorkerOnUIEnabled()) {
+    NotifyWorkerVersionDoomedOnUI(process_id(),
+                                  worker_devtools_agent_route_id(),
+                                  base::WrapRefCounted(context_->wrapper()),
+                                  owner_version_->version_id());
+  } else {
+    ui_task_runner_->PostTask(
+        FROM_HERE, base::BindOnce(NotifyWorkerVersionDoomedOnUI, process_id(),
+                                  worker_devtools_agent_route_id(),
+                                  base::WrapRefCounted(context_->wrapper()),
+                                  owner_version_->version_id()));
+  }
 }
 
 void EmbeddedWorkerInstance::OnScriptEvaluationStart() {

third_party/blink/web_tests/http/tests/devtools/service-workers/service-worker-manager.js

@@ -21,11 +21,8 @@
         var serviceWorkerManager = SDK.targetManager.mainTarget().model(SDK.ServiceWorkerManager);
         // Allow agents to do rountrips.
         TestRunner.deprecatedRunAfterPendingDispatches(function() {
-          for (var registration of serviceWorkerManager.registrations().values()) {
-            for (var version of registration.versions.values()) {
-              serviceWorkerManager.stopWorker(version.id);
-            }
-          }
+          for (var registration of serviceWorkerManager.registrations().values())
+            serviceWorkerManager.deleteRegistration(registration.id)
         });
       }
     },

third_party/blink/web_tests/http/tests/inspector-protocol/service-worker/target-events-expected.txt

+Test that target evens are fired for service worker
+Started and attached to service_worker target
+Received workerRegistrationUpdated with scopeURL = http://127.0.0.1:8000/inspector-protocol/service-worker/resources/
+Stopped service worker and received Inspector.targetCrashed event
+
+Restarted service worker and received Inspector.targetReloadedAfterCrash event
+Unregistered service worker and received Target.targetDestroyed event for the worker: true
+

third_party/blink/web_tests/http/tests/inspector-protocol/service-worker/target-events.js

+(async function(testRunner) {
+  const {page, session, dp} = await testRunner.startURL(
+      '/inspector-protocol/resources/empty.html',
+      'Test that target evens are fired for service worker');
+  const swHelper = (await testRunner.loadScript('resources/service-worker-helper.js'))(dp, session);
+
+  await dp.Target.setAutoAttach(
+    {autoAttach: true, waitForDebuggerOnStart: false, flatten: true});
+  await dp.Target.setDiscoverTargets({discover: true});
+
+  const swTargetPromises = Promise.all([
+    dp.Target.onceTargetCreated(),
+    dp.Target.onceAttachedToTarget(),
+  ]);
+  const serviceWorkerURL = '/inspector-protocol/service-worker/resources/blank-service-worker.js';
+  await swHelper.installSWAndWaitForActivated(serviceWorkerURL);
+  const [swTarget, swAttachedEvent] = await swTargetPromises;
+  const swTargetInfo = swTarget.params.targetInfo;
+  testRunner.log(`Started and attached to ${swTargetInfo.type} target`);
+
+  const swdp = session.createChild(swAttachedEvent.params.sessionId).protocol;
+  const [registration] = await Promise.all([
+    dp.ServiceWorker.onceWorkerRegistrationUpdated(),
+    dp.ServiceWorker.enable(),
+  ]);
+  const scopeURL = registration.params.registrations[0].scopeURL;
+
+  testRunner.log('Received workerRegistrationUpdated with scopeURL = '  + scopeURL);
+  await Promise.all([
+    swdp.Inspector.onceTargetCrashed(),
+    dp.ServiceWorker.stopAllWorkers(),
+  ]);
+  testRunner.log('Stopped service worker and received Inspector.targetCrashed event\n');
+
+  await Promise.all([
+    swdp.Inspector.onceTargetReloadedAfterCrash(),
+    dp.ServiceWorker.startWorker({scopeURL}),
+  ]);
+  testRunner.log('Restarted service worker and received Inspector.targetReloadedAfterCrash event');
+  const [swDestroyedEvent] = await Promise.all([
+    dp.Target.onceTargetDestroyed(),
+    dp.ServiceWorker.unregister({scopeURL}),
+    dp.ServiceWorker.stopAllWorkers(),
+  ]);
+  testRunner.log('Unregistered service worker and received Target.targetDestroyed event for the worker: ' + (swTargetInfo.targetId === swDestroyedEvent.params.targetId));
+
+  testRunner.completeTest();
+});