[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}

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).
@@ -345,16 +346,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) {
   ReattachServiceWorkers(false);
 }

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());
-  }
 }
 
 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);
-  }
 }
 
 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,16 +62,20 @@ 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()) {
+  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;
+  }
+
   *devtools_worker_token = base::UnguessableToken::Create();
   scoped_refptr<ServiceWorkerDevToolsAgentHost> host =
       new ServiceWorkerDevToolsAgentHost(
-            worker_process_id, worker_route_id, context, context_weak,
+          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);
@@ -83,15 +87,6 @@ void ServiceWorkerDevToolsManager::WorkerCreated(
     if (should_pause_on_start)
       *pause_on_start = true;
   }
-    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();
 }
 
 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,26 +129,33 @@ 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,
+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);
@@ -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();
+  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

@@ -89,10 +89,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,
@@ -102,10 +102,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(
@@ -209,8 +213,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);
@@ -301,15 +304,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);
 
@@ -417,8 +420,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)
@@ -429,10 +434,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_));
     }
   }
@@ -465,17 +470,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_;
   }
@@ -674,8 +668,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 {
@@ -685,7 +679,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)));
@@ -827,7 +821,6 @@ class EmbeddedWorkerInstance::StartTask {
 
 EmbeddedWorkerInstance::~EmbeddedWorkerInstance() {
   DCHECK_CURRENTLY_ON(ServiceWorkerContext::GetCoreThreadId());
-  devtools_proxy_.reset();
   ReleaseProcess();
 }
 
@@ -1041,8 +1034,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();
+});