refactor: remove raw_ptr<content::StoragePartition> from ServiceWorkerContext and ServiceWorkerKey (#50663)

This removes two `raw_ptr<context::StoragePartition>` instances.

These pointers were used to build a ServiceWorkerMain* lookup key.
The key was built from [version_id, raw_ptr<StoragePartition>].
Unfortunately these keys could be dangling on shutdown.

This PR now uses stable, immutable fields for building the key:
[version_id, BrowserContext::UniqueId(), context::StoragePartitionConfig].
context::StoragePartitionConfig is a unique lookup key for StoragePartition
within a BrowserContext.

shell/browser/api/electron_api_service_worker_context.cc

@@ -80,9 +80,12 @@ gin::DeprecatedWrapperInfo ServiceWorkerContext::kWrapperInfo = {
 
 ServiceWorkerContext::ServiceWorkerContext(
     v8::Isolate* isolate,
-    ElectronBrowserContext* browser_context) {
-  storage_partition_ = browser_context->GetDefaultStoragePartition();
-  service_worker_context_ = storage_partition_->GetServiceWorkerContext();
+    ElectronBrowserContext* browser_context)
+    : service_worker_context_{browser_context->GetDefaultStoragePartition()
+                                  ->GetServiceWorkerContext()},
+      browser_context_id_{browser_context->UniqueId()},
+      storage_partition_config_{
+          browser_context->GetDefaultStoragePartition()->GetConfig()} {
   service_worker_context_->AddObserver(this);
 }
 
@@ -93,9 +96,8 @@ ServiceWorkerContext::~ServiceWorkerContext() {
 void ServiceWorkerContext::OnRunningStatusChanged(
     int64_t version_id,
     blink::EmbeddedWorkerStatus running_status) {
-  ServiceWorkerMain* worker =
-      ServiceWorkerMain::FromVersionID(version_id, storage_partition_);
-  if (worker)
+  if (auto* worker = ServiceWorkerMain::FromVersionID(
+          browser_context_id_, storage_partition_config_, version_id))
     worker->OnRunningStatusChanged(running_status);
 
   v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
@@ -133,9 +135,8 @@ void ServiceWorkerContext::OnRegistrationCompleted(const GURL& scope) {
 
 void ServiceWorkerContext::OnVersionRedundant(int64_t version_id,
                                               const GURL& scope) {
-  ServiceWorkerMain* worker =
-      ServiceWorkerMain::FromVersionID(version_id, storage_partition_);
-  if (worker)
+  if (auto* worker = ServiceWorkerMain::FromVersionID(
+          browser_context_id_, storage_partition_config_, version_id))
     worker->OnVersionRedundant();
 }
 
@@ -206,18 +207,19 @@ v8::Local<v8::Value> ServiceWorkerContext::GetWorkerFromVersionID(
     v8::Isolate* isolate,
     int64_t version_id) {
   return ServiceWorkerMain::From(isolate, service_worker_context_,
-                                 storage_partition_, version_id)
+                                 browser_context_id_, storage_partition_config_,
+                                 version_id)
       .ToV8();
 }
 
 gin_helper::Handle<ServiceWorkerMain>
 ServiceWorkerContext::GetWorkerFromVersionIDIfExists(v8::Isolate* isolate,
                                                      int64_t version_id) {
-  ServiceWorkerMain* worker =
-      ServiceWorkerMain::FromVersionID(version_id, storage_partition_);
-  if (!worker)
-    return gin_helper::Handle<ServiceWorkerMain>();
-  return gin_helper::CreateHandle(isolate, worker);
+  if (auto* worker = ServiceWorkerMain::FromVersionID(
+          browser_context_id_, storage_partition_config_, version_id))
+    return gin_helper::CreateHandle(isolate, worker);
+
+  return {};
 }
 
 v8::Local<v8::Promise> ServiceWorkerContext::StartWorkerForScope(

shell/browser/api/electron_api_service_worker_context.h

@@ -5,18 +5,17 @@
 #ifndef ELECTRON_SHELL_BROWSER_API_ELECTRON_API_SERVICE_WORKER_CONTEXT_H_
 #define ELECTRON_SHELL_BROWSER_API_ELECTRON_API_SERVICE_WORKER_CONTEXT_H_
 
+#include <string>
+
 #include "base/memory/raw_ptr.h"
 #include "content/public/browser/service_worker_context.h"
 #include "content/public/browser/service_worker_context_observer.h"
+#include "content/public/browser/storage_partition_config.h"
 #include "shell/browser/event_emitter_mixin.h"
 #include "shell/common/gin_helper/wrappable.h"
 #include "third_party/blink/public/common/service_worker/embedded_worker_status.h"
 #include "third_party/blink/public/common/tokens/tokens.h"
 
-namespace content {
-class StoragePartition;
-}
-
 namespace gin_helper {
 template <typename T>
 class Handle;
@@ -99,9 +98,13 @@ class ServiceWorkerContext final
 
   raw_ptr<content::ServiceWorkerContext> service_worker_context_;
 
-  // Service worker registration and versions are unique to a storage partition.
-  // Keep a reference to the storage partition to be used for lookups.
-  raw_ptr<content::StoragePartition> storage_partition_;
+  // A key identifying the owning BrowserContext.
+  // Used in ServiceWorkerMain lookups.
+  const std::string browser_context_id_;
+
+  // A key identifying a StoragePartition within a BrowserContext.
+  // Used in ServiceWorkerMain lookups.
+  const content::StoragePartitionConfig storage_partition_config_;
 
   base::WeakPtrFactory<ServiceWorkerContext> weak_ptr_factory_{this};
 };

shell/browser/api/electron_api_service_worker_main.cc

@@ -7,6 +7,8 @@
 #include <string>
 #include <utility>
 
+#include "base/containers/flat_map.h"
+#include "base/containers/map_util.h"
 #include "base/logging.h"
 #include "base/no_destructor.h"
 #include "content/browser/service_worker/service_worker_context_wrapper.h"  // nogncheck
@@ -27,7 +29,6 @@
 #include "shell/common/gin_helper/promise.h"
 #include "shell/common/node_includes.h"
 #include "shell/common/v8_util.h"
-#include "third_party/abseil-cpp/absl/container/flat_hash_map.h"
 
 namespace {
 
@@ -58,27 +59,23 @@ std::optional<content::ServiceWorkerVersionBaseInfo> GetLiveVersionInfo(
 namespace electron::api {
 
 // ServiceWorkerKey -> ServiceWorkerMain*
-using VersionIdMap = absl::flat_hash_map<ServiceWorkerKey,
-                                         ServiceWorkerMain*,
-                                         ServiceWorkerKey::Hasher>;
-
-VersionIdMap& GetVersionIdMap() {
-  static base::NoDestructor<VersionIdMap> instance;
+auto& GetVersionIdMap() {
+  using Map = base::flat_map<ServiceWorkerKey, ServiceWorkerMain*>;
+  static base::NoDestructor<Map> instance;
   return *instance;
 }
 
 ServiceWorkerMain* FromServiceWorkerKey(const ServiceWorkerKey& key) {
-  VersionIdMap& version_map = GetVersionIdMap();
-  auto iter = version_map.find(key);
-  auto* service_worker = iter == version_map.end() ? nullptr : iter->second;
-  return service_worker;
+  return base::FindPtrOrNull(GetVersionIdMap(), key);
 }
 
 // static
 ServiceWorkerMain* ServiceWorkerMain::FromVersionID(
-    int64_t version_id,
-    const content::StoragePartition* storage_partition) {
-  ServiceWorkerKey key(version_id, storage_partition);
+    std::string browser_context_id,
+    content::StoragePartitionConfig storage_partition_config,
+    int64_t version_id) {
+  const ServiceWorkerKey key{std::move(browser_context_id),
+                             std::move(storage_partition_config), version_id};
   return FromServiceWorkerKey(key);
 }
 
@@ -87,8 +84,10 @@ gin::DeprecatedWrapperInfo ServiceWorkerMain::kWrapperInfo = {
 
 ServiceWorkerMain::ServiceWorkerMain(content::ServiceWorkerContext* sw_context,
                                      int64_t version_id,
-                                     const ServiceWorkerKey& key)
-    : version_id_(version_id), key_(key), service_worker_context_(sw_context) {
+                                     ServiceWorkerKey key)
+    : version_id_{version_id},
+      key_{std::move(key)},
+      service_worker_context_{sw_context} {
   GetVersionIdMap().emplace(key_, this);
   InvalidateVersionInfo();
 }
@@ -298,12 +297,14 @@ gin_helper::Handle<ServiceWorkerMain> ServiceWorkerMain::New(
 gin_helper::Handle<ServiceWorkerMain> ServiceWorkerMain::From(
     v8::Isolate* isolate,
     content::ServiceWorkerContext* sw_context,
-    const content::StoragePartition* storage_partition,
+    std::string browser_context_id,
+    content::StoragePartitionConfig storage_partition_config,
     int64_t version_id) {
-  ServiceWorkerKey service_worker_key(version_id, storage_partition);
+  ServiceWorkerKey service_worker_key{std::move(browser_context_id),
+                                      std::move(storage_partition_config),
+                                      version_id};
 
-  auto* service_worker = FromServiceWorkerKey(service_worker_key);
-  if (service_worker)
+  if (auto* service_worker = FromServiceWorkerKey(service_worker_key))
     return gin_helper::CreateHandle(isolate, service_worker);
 
   // Ensure ServiceWorkerVersion exists and is not redundant (pending deletion)
@@ -313,8 +314,8 @@ gin_helper::Handle<ServiceWorkerMain> ServiceWorkerMain::From(
   }
 
   auto handle = gin_helper::CreateHandle(
-      isolate,
-      new ServiceWorkerMain(sw_context, version_id, service_worker_key));
+      isolate, new ServiceWorkerMain{sw_context, version_id,
+                                     std::move(service_worker_key)});
 
   // Prevent garbage collection of worker until it has been deleted internally.
   handle->Pin(isolate);

shell/browser/api/electron_api_service_worker_main.h

@@ -5,13 +5,13 @@
 #ifndef ELECTRON_SHELL_BROWSER_API_ELECTRON_API_SERVICE_WORKER_MAIN_H_
 #define ELECTRON_SHELL_BROWSER_API_ELECTRON_API_SERVICE_WORKER_MAIN_H_
 
+#include <compare>
 #include <string>
 
 #include "base/memory/raw_ptr.h"
-#include "base/process/process.h"
-#include "content/public/browser/global_routing_id.h"
 #include "content/public/browser/service_worker_context.h"
 #include "content/public/browser/service_worker_version_base_info.h"
+#include "content/public/browser/storage_partition_config.h"
 #include "mojo/public/cpp/bindings/associated_receiver.h"
 #include "mojo/public/cpp/bindings/associated_remote.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
@@ -24,10 +24,6 @@
 
 class GURL;
 
-namespace content {
-class StoragePartition;
-}
-
 namespace gin {
 class Arguments;
 }  // namespace gin
@@ -42,41 +38,21 @@ class Promise;
 
 namespace electron::api {
 
-// Key to uniquely identify a ServiceWorkerMain by its Version ID within the
-// associated StoragePartition.
+// Key to uniquely identify a ServiceWorkerMain by its
+// BrowserContext ID, the StoragePartition key, and version id.
 struct ServiceWorkerKey {
+  std::string browser_context_id;
+  content::StoragePartitionConfig storage_partition_config;
   int64_t version_id;
-  raw_ptr<const content::StoragePartition> storage_partition;
-
-  ServiceWorkerKey(int64_t id, const content::StoragePartition* partition)
-      : version_id(id), storage_partition(partition) {}
-
-  bool operator<(const ServiceWorkerKey& other) const {
-    return std::tie(version_id, storage_partition) <
-           std::tie(other.version_id, other.storage_partition);
-  }
-
-  bool operator==(const ServiceWorkerKey& other) const {
-    return version_id == other.version_id &&
-           storage_partition == other.storage_partition;
-  }
-
-  struct Hasher {
-    std::size_t operator()(const ServiceWorkerKey& key) const {
-      return std::hash<const content::StoragePartition*>()(
-                 key.storage_partition) ^
-             std::hash<int64_t>()(key.version_id);
-    }
-  };
+  auto operator<=>(const ServiceWorkerKey&) const = default;
 };
 
 // Creates a wrapper to align with the lifecycle of the non-public
 // content::ServiceWorkerVersion. Object instances are pinned for the lifetime
 // of the underlying SW such that registered IPC handlers continue to dispatch.
 //
-// Instances are uniquely identified by pairing their version ID and the
-// StoragePartition in which they're registered. In Electron, this is always
-// the default StoragePartition for the associated BrowserContext.
+// Instances are uniquely identified by pairing their version ID with the
+// BrowserContext and StoragePartition in which they're registered.
 class ServiceWorkerMain final
     : public gin_helper::DeprecatedWrappable<ServiceWorkerMain>,
       public gin_helper::Pinnable<ServiceWorkerMain>,
@@ -88,11 +64,13 @@ class ServiceWorkerMain final
   static gin_helper::Handle<ServiceWorkerMain> From(
       v8::Isolate* isolate,
       content::ServiceWorkerContext* sw_context,
-      const content::StoragePartition* storage_partition,
+      std::string browser_context_id,
+      content::StoragePartitionConfig storage_partition_config,
       int64_t version_id);
   static ServiceWorkerMain* FromVersionID(
-      int64_t version_id,
-      const content::StoragePartition* storage_partition);
+      std::string browser_context_id,
+      content::StoragePartitionConfig storage_partition_config,
+      int64_t version_id);
 
   // gin_helper::Constructible
   static void FillObjectTemplate(v8::Isolate*, v8::Local<v8::ObjectTemplate>);
@@ -112,7 +90,7 @@ class ServiceWorkerMain final
  protected:
   explicit ServiceWorkerMain(content::ServiceWorkerContext* sw_context,
                              int64_t version_id,
-                             const ServiceWorkerKey& key);
+                             ServiceWorkerKey key);
   ~ServiceWorkerMain() override;
 
  private:
@@ -146,11 +124,12 @@ class ServiceWorkerMain final
   GURL ScopeURL() const;
   GURL ScriptURL() const;
 
-  // Version ID unique only to the StoragePartition.
-  int64_t version_id_;
+  // Version ID assigned by the service worker storage.
+  const int64_t version_id_;
 
-  // Unique identifier pairing the Version ID and StoragePartition.
-  ServiceWorkerKey key_;
+  // Unique identifier pairing the Version ID, BrowserContext, and
+  // StoragePartition.
+  const ServiceWorkerKey key_;
 
   // Whether the Service Worker version has been destroyed.
   bool version_destroyed_ = false;