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/+/2213448Reviewed-by: Yury Semikhatsky <yurys@chromium.org>
Commit-Queue: Yury Semikhatsky <yurys@chromium.org>
Cr-Commit-Position: refs/heads/master@{#771365}

content/browser/devtools/protocol/target_auto_attacher.cc

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

content/browser/devtools/service_worker_devtools_agent_host.h

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

content/browser/devtools/service_worker_devtools_manager.cc

@@ -7,7 +7,6 @@
 #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"
@@ -44,10 +43,11 @@ void ServiceWorkerDevToolsManager::AddAllAgentHostsForBrowserContext(
   }
 }
 
-void ServiceWorkerDevToolsManager::WorkerStarting(
+void ServiceWorkerDevToolsManager::WorkerCreated(
     int worker_process_id,
     int worker_route_id,
-    scoped_refptr<ServiceWorkerContextWrapper> context_wrapper,
+    const ServiceWorkerContextCore* context,
+    base::WeakPtr<ServiceWorkerContextCore> context_weak,
     int64_t version_id,
     const GURL& url,
     const GURL& scope,
@@ -62,31 +62,36 @@ void ServiceWorkerDevToolsManager::WorkerStarting(
   const WorkerId worker_id(worker_process_id, worker_route_id);
   DCHECK(live_hosts_.find(worker_id) == live_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();
+  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;
+    }
     return;
   }
 
-  *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;
-  }
+  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(
@@ -118,8 +123,9 @@ void ServiceWorkerDevToolsManager::UpdateCrossOriginEmbedderPolicy(
   auto it = live_hosts_.find(worker_id);
   if (it == live_hosts_.end())
     return;
-  it->second->UpdateCrossOriginEmbedderPolicy(
-      std::move(cross_origin_embedder_policy), std::move(coep_reporter));
+  scoped_refptr<ServiceWorkerDevToolsAgentHost> host = it->second;
+  host->UpdateCrossOriginEmbedderPolicy(std::move(cross_origin_embedder_policy),
+                                        std::move(coep_reporter));
 }
 
 void ServiceWorkerDevToolsManager::WorkerVersionInstalled(int worker_process_id,
@@ -129,34 +135,27 @@ void ServiceWorkerDevToolsManager::WorkerVersionInstalled(int worker_process_id,
   auto it = live_hosts_.find(worker_id);
   if (it == live_hosts_.end())
     return;
-  it->second->WorkerVersionInstalled();
+  scoped_refptr<ServiceWorkerDevToolsAgentHost> host = it->second;
+  host->WorkerVersionInstalled();
+  for (auto& observer : observer_list_)
+    observer.WorkerVersionInstalled(host.get());
 }
 
-void ServiceWorkerDevToolsManager::WorkerVersionDoomed(
-    int worker_process_id,
-    int worker_route_id,
-    scoped_refptr<ServiceWorkerContextWrapper> context_wrapper,
-    int64_t version_id) {
+void ServiceWorkerDevToolsManager::WorkerVersionDoomed(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);
-  if (it != live_hosts_.end()) {
-    it->second->WorkerVersionDoomed();
-    return;
-  }
-  scoped_refptr<ServiceWorkerDevToolsAgentHost> host =
-      TakeStoppedHost(context_wrapper.get(), version_id);
-  if (!host)
+  if (it == live_hosts_.end())
     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.WorkerDestroyed(host.get());
+    observer.WorkerVersionDoomed(host.get());
 }
 
-void ServiceWorkerDevToolsManager::WorkerStopped(int worker_process_id,
-                                                 int worker_route_id) {
+void ServiceWorkerDevToolsManager::WorkerDestroyed(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);
@@ -164,23 +163,21 @@ void ServiceWorkerDevToolsManager::WorkerStopped(int worker_process_id,
     return;
   scoped_refptr<ServiceWorkerDevToolsAgentHost> agent_host(it->second);
   live_hosts_.erase(it);
-  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());
-  }
+  terminated_hosts_.insert(agent_host.get());
+  agent_host->WorkerDestroyed();
+  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.
-  stopped_hosts_.erase(agent_host);
+  if (it != terminated_hosts_.end())
+    terminated_hosts_.erase(it);
 }
 
 void ServiceWorkerDevToolsManager::AddObserver(Observer* observer) {
@@ -251,22 +248,4 @@ 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 ServiceWorkerContextWrapper;
+class ServiceWorkerContextCore;
 
 // Manages ServiceWorkerDevToolsAgentHost's. This class lives on UI thread.
 class CONTENT_EXPORT ServiceWorkerDevToolsManager {
@@ -40,6 +40,8 @@ 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:
@@ -58,10 +60,11 @@ class CONTENT_EXPORT ServiceWorkerDevToolsManager {
       BrowserContext* browser_context,
       std::vector<scoped_refptr<ServiceWorkerDevToolsAgentHost>>* result);
 
-  void WorkerStarting(
+  void WorkerCreated(
       int worker_process_id,
       int worker_route_id,
-      scoped_refptr<ServiceWorkerContextWrapper> context_wrapper,
+      const ServiceWorkerContextCore* context,
+      base::WeakPtr<ServiceWorkerContextCore> context_weak,
       int64_t version_id,
       const GURL& url,
       const GURL& scope,
@@ -84,15 +87,8 @@ class CONTENT_EXPORT ServiceWorkerDevToolsManager {
       mojo::PendingRemote<network::mojom::CrossOriginEmbedderPolicyReporter>
           coep_reporter);
   void WorkerVersionInstalled(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 WorkerVersionDoomed(int worker_process_id, int worker_route_id);
+  void WorkerDestroyed(int worker_process_id, int worker_route_id);
   void NavigationPreloadRequestSent(int worker_process_id,
                                     int worker_route_id,
                                     const std::string& request_id,
@@ -127,10 +123,6 @@ 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_;
 
@@ -139,7 +131,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*> stopped_hosts_;
+  base::flat_set<ServiceWorkerDevToolsAgentHost*> terminated_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 NotifyWorkerStoppedOnUI(int worker_process_id, int worker_route_id) {
+void NotifyWorkerDestroyedOnUI(int worker_process_id, int worker_route_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  ServiceWorkerDevToolsManager::GetInstance()->WorkerStopped(worker_process_id,
-                                                             worker_route_id);
+  ServiceWorkerDevToolsManager::GetInstance()->WorkerDestroyed(
+      worker_process_id, worker_route_id);
 }
 
 void NotifyWorkerVersionInstalledOnUI(int worker_process_id,
@@ -104,14 +104,10 @@ void NotifyWorkerVersionInstalledOnUI(int worker_process_id,
       worker_process_id, worker_route_id);
 }
 
-void NotifyWorkerVersionDoomedOnUI(
-    int worker_process_id,
-    int worker_route_id,
-    scoped_refptr<ServiceWorkerContextWrapper> context_wrapper,
-    int64_t version_id) {
+void NotifyWorkerVersionDoomedOnUI(int worker_process_id, int worker_route_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   ServiceWorkerDevToolsManager::GetInstance()->WorkerVersionDoomed(
-      worker_process_id, worker_route_id, context_wrapper, version_id);
+      worker_process_id, worker_route_id);
 }
 
 using CreateFactoryBundlesOnUICallback = base::OnceCallback<void(
@@ -227,7 +223,8 @@ void SetupOnUIThread(
         cross_origin_embedder_policy,
     blink::mojom::EmbeddedWorkerStartParamsPtr params,
     mojo::PendingReceiver<blink::mojom::EmbeddedWorkerInstanceClient> receiver,
-    scoped_refptr<ServiceWorkerContextWrapper> context_wrapper,
+    ServiceWorkerContextCore* context,
+    base::WeakPtr<ServiceWorkerContextCore> weak_context,
     const base::Optional<base::Time>& io_post_time,
     SetupProcessCallback callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
@@ -327,15 +324,15 @@ void SetupOnUIThread(
   }
   // Register to DevTools and update params accordingly.
   const int routing_id = rph->GetNextRoutingID();
-  ServiceWorkerDevToolsManager::GetInstance()->WorkerStarting(
-      process_id, routing_id, std::move(context_wrapper),
+  ServiceWorkerDevToolsManager::GetInstance()->WorkerCreated(
+      process_id, routing_id, context, weak_context,
       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 WorkerStopped().
+  // balanced by DevToolsProxy's destructor calling WorkerDestroyed().
   devtools_proxy = std::make_unique<EmbeddedWorkerInstance::DevToolsProxy>(
       process_id, routing_id);
 
@@ -444,10 +441,8 @@ void BindCacheStorageOnUIThread(
 
 }  // namespace
 
-// 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.
+// Created on UI thread and moved to core thread. Proxies notifications to
+// DevToolsManager that lives on UI thread. Owned by EmbeddedWorkerInstance.
 class EmbeddedWorkerInstance::DevToolsProxy {
  public:
   DevToolsProxy(int process_id, int agent_route_id)
@@ -458,10 +453,10 @@ class EmbeddedWorkerInstance::DevToolsProxy {
   ~DevToolsProxy() {
     DCHECK_CURRENTLY_ON(ServiceWorkerContext::GetCoreThreadId());
     if (ServiceWorkerContext::IsServiceWorkerOnUIEnabled()) {
-      NotifyWorkerStoppedOnUI(process_id_, agent_route_id_);
+      NotifyWorkerDestroyedOnUI(process_id_, agent_route_id_);
     } else {
       ui_task_runner_->PostTask(
-          FROM_HERE, base::BindOnce(NotifyWorkerStoppedOnUI, process_id_,
+          FROM_HERE, base::BindOnce(NotifyWorkerDestroyedOnUI, process_id_,
                                     agent_route_id_));
     }
   }
@@ -494,6 +489,17 @@ 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_;
   }
@@ -692,8 +698,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_),
-          base::WrapRefCounted(context->wrapper()), base::nullopt,
+          std::move(params), std::move(receiver_), context.get(), context,
+          base::nullopt,
           base::BindOnce(&StartTask::OnSetupCompleted,
                          weak_factory_.GetWeakPtr(), process_manager));
     } else {
@@ -703,7 +709,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_), base::WrapRefCounted(context->wrapper()),
+              std::move(receiver_), context.get(), context,
               base::make_optional<base::Time>(base::Time::Now()),
               base::BindOnce(&StartTask::OnSetupCompleted,
                              weak_factory_.GetWeakPtr(), process_manager)));
@@ -852,6 +858,7 @@ class EmbeddedWorkerInstance::StartTask {
 
 EmbeddedWorkerInstance::~EmbeddedWorkerInstance() {
   DCHECK_CURRENTLY_ON(ServiceWorkerContext::GetCoreThreadId());
+  devtools_proxy_.reset();
   ReleaseProcess();
 }
 
@@ -1065,18 +1072,8 @@ void EmbeddedWorkerInstance::OnWorkerVersionInstalled() {
 }
 
 void EmbeddedWorkerInstance::OnWorkerVersionDoomed() {
-  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()));
-  }
+  if (devtools_proxy_)
+    devtools_proxy_->NotifyWorkerVersionDoomed();
 }
 
 void EmbeddedWorkerInstance::OnScriptEvaluationStart() {

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

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