refactor: decouple api::WebRequest from api::BrowserContext (#48848)

* refactor: rename api::Session::CreateFrom() to api::Session::FromOrCreate()

This is both clearer and more consistent with other classes

* refactor: add Session::FromOrCreate(content::BrowserContext*)

* refactor: reimplement api::WebRequest::FromOrCreate() using api::Session::FromOrCreate()

* refactor: use base::PassKey to ensure WebRequest is only instantiated by Session

* refactor: remove WebRequest::From()

no longer needed; Session already guarantees uniqueness

* refactor: remove unused isolate arg from WebRequest ctor

* refactor: do not attach WebRequest to BrowserContext

no longer needed now that access goes through Session

shell/browser/api/electron_api_session.cc

@@ -19,6 +19,7 @@
 #include "base/files/file_util.h"
 #include "base/scoped_observation.h"
 #include "base/strings/string_util.h"
+#include "base/types/pass_key.h"
 #include "base/uuid.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/predictors/predictors_traffic_annotations.h"  // nogncheck
@@ -1361,7 +1362,7 @@ v8::Local<v8::Value> Session::ServiceWorkerContext(v8::Isolate* isolate) {
 
 v8::Local<v8::Value> Session::WebRequest(v8::Isolate* isolate) {
   if (web_request_.IsEmptyThreadSafe()) {
-    auto handle = WebRequest::Create(isolate, browser_context());
+    auto handle = WebRequest::Create(base::PassKey<Session>{}, isolate);
     web_request_.Reset(isolate, handle.ToV8());
   }
   return web_request_.Get(isolate);
@@ -1689,8 +1690,17 @@ gin::WeakCell<Session>* Session::FromBrowserContext(
 }
 
 // static
-Session* Session::CreateFrom(v8::Isolate* isolate,
-                             ElectronBrowserContext* browser_context) {
+Session* Session::FromOrCreate(v8::Isolate* isolate,
+                               content::BrowserContext* context) {
+  if (ElectronBrowserContext::IsValidContext(context))
+    return FromOrCreate(isolate, static_cast<ElectronBrowserContext*>(context));
+  DCHECK(false);
+  return {};
+}
+
+// static
+Session* Session::FromOrCreate(v8::Isolate* isolate,
+                               ElectronBrowserContext* browser_context) {
   gin::WeakCell<Session>* existing = FromBrowserContext(browser_context);
   if (existing && existing->Get()) {
     return existing->Get();
@@ -1731,7 +1741,7 @@ Session* Session::FromPartition(v8::Isolate* isolate,
     browser_context =
         ElectronBrowserContext::From(partition, true, std::move(options));
   }
-  return CreateFrom(isolate, browser_context);
+  return FromOrCreate(isolate, browser_context);
 }
 
 // static
@@ -1752,7 +1762,7 @@ Session* Session::FromPath(gin::Arguments* args,
   browser_context =
       ElectronBrowserContext::FromPath(std::move(path), std::move(options));
 
-  return CreateFrom(args->isolate(), browser_context);
+  return FromOrCreate(args->isolate(), browser_context);
 }
 
 // static

shell/browser/api/electron_api_session.h

@@ -73,8 +73,14 @@ class Session final : public gin::Wrappable<Session>,
                       private content::DownloadManager::Observer {
  public:
   // Gets or creates Session from the |browser_context|.
-  static Session* CreateFrom(v8::Isolate* isolate,
-                             ElectronBrowserContext* browser_context);
+  static Session* FromOrCreate(v8::Isolate* isolate,
+                               ElectronBrowserContext* browser_context);
+
+  // Convenience wrapper around the previous method: Checks that
+  // |browser_context| is an ElectronBrowserContext before downcasting.
+  static Session* FromOrCreate(v8::Isolate* isolate,
+                               content::BrowserContext* browser_context);
+
   static void New();  // Dummy, do not use!
 
   static gin::WeakCell<Session>* FromBrowserContext(

shell/browser/api/electron_api_web_contents.cc

@@ -754,7 +754,7 @@ WebContents::WebContents(v8::Isolate* isolate,
   script_executor_ = std::make_unique<extensions::ScriptExecutor>(web_contents);
 #endif
 
-  session_ = Session::CreateFrom(isolate, GetBrowserContext());
+  session_ = Session::FromOrCreate(isolate, GetBrowserContext());
 
   SetUserAgent(GetBrowserContext()->GetUserAgent());
 
@@ -776,7 +776,7 @@ WebContents::WebContents(v8::Isolate* isolate,
 {
   DCHECK(type != Type::kRemote)
       << "Can't take ownership of a remote WebContents";
-  session_ = Session::CreateFrom(isolate, GetBrowserContext());
+  session_ = Session::FromOrCreate(isolate, GetBrowserContext());
   InitWithSessionAndOptions(isolate, std::move(web_contents),
                             session_->browser_context(),
                             gin::Dictionary::CreateEmpty(isolate));

shell/browser/api/electron_api_web_request.cc

@@ -72,14 +72,6 @@ namespace electron::api {
 
 namespace {
 
-const char kUserDataKey[] = "WebRequest";
-
-// BrowserContext <=> WebRequest relationship.
-struct UserData : public base::SupportsUserData::Data {
-  explicit UserData(WebRequest* data) : data(data) {}
-  raw_ptr<WebRequest> data;
-};
-
 extensions::WebRequestResourceType ParseResourceType(std::string_view value) {
   if (auto iter = ResourceTypes.find(value); iter != ResourceTypes.end())
     return iter->second;
@@ -312,15 +304,8 @@ WebRequest::ResponseListenerInfo::ResponseListenerInfo(
 WebRequest::ResponseListenerInfo::ResponseListenerInfo() = default;
 WebRequest::ResponseListenerInfo::~ResponseListenerInfo() = default;
 
-WebRequest::WebRequest(v8::Isolate* isolate,
-                       content::BrowserContext* browser_context)
-    : browser_context_(browser_context) {
-  browser_context_->SetUserData(kUserDataKey, std::make_unique<UserData>(this));
-}
-
-WebRequest::~WebRequest() {
-  browser_context_->RemoveUserData(kUserDataKey);
-}
+WebRequest::WebRequest(base::PassKey<Session>) {}
+WebRequest::~WebRequest() = default;
 
 gin::ObjectTemplateBuilder WebRequest::GetObjectTemplateBuilder(
     v8::Isolate* isolate) {
@@ -736,40 +721,19 @@ void WebRequest::HandleSimpleEvent(SimpleEvent event,
 gin_helper::Handle<WebRequest> WebRequest::FromOrCreate(
     v8::Isolate* isolate,
     content::BrowserContext* browser_context) {
-  gin_helper::Handle<WebRequest> handle = From(isolate, browser_context);
-  if (handle.IsEmpty()) {
-    // Make sure the |Session| object has the |webRequest| property created.
-    v8::Local<v8::Value> web_request =
-        Session::CreateFrom(
-            isolate, static_cast<ElectronBrowserContext*>(browser_context))
-            ->WebRequest(isolate);
-    gin::ConvertFromV8(isolate, web_request, &handle);
-  }
+  v8::Local<v8::Value> web_request =
+      Session::FromOrCreate(isolate, browser_context)->WebRequest(isolate);
+  gin_helper::Handle<WebRequest> handle;
+  gin::ConvertFromV8(isolate, web_request, &handle);
   DCHECK(!handle.IsEmpty());
   return handle;
 }
 
 // static
 gin_helper::Handle<WebRequest> WebRequest::Create(
-    v8::Isolate* isolate,
-    content::BrowserContext* browser_context) {
-  DCHECK(From(isolate, browser_context).IsEmpty())
-      << "WebRequest already created";
-  return gin_helper::CreateHandle(isolate,
-                                  new WebRequest(isolate, browser_context));
-}
-
-// static
-gin_helper::Handle<WebRequest> WebRequest::From(
-    v8::Isolate* isolate,
-    content::BrowserContext* browser_context) {
-  if (!browser_context)
-    return {};
-  auto* user_data =
-      static_cast<UserData*>(browser_context->GetUserData(kUserDataKey));
-  if (!user_data)
-    return {};
-  return gin_helper::CreateHandle(isolate, user_data->data.get());
+    base::PassKey<Session> passkey,
+    v8::Isolate* isolate) {
+  return gin_helper::CreateHandle(isolate, new WebRequest{std::move(passkey)});
 }
 
 }  // namespace electron::api

shell/browser/api/electron_api_web_request.h

@@ -9,7 +9,7 @@
 #include <set>
 #include <string>
 
-#include "base/memory/raw_ptr.h"
+#include "base/types/pass_key.h"
 #include "net/base/completion_once_callback.h"
 #include "services/network/public/cpp/resource_request.h"
 #include "shell/common/gin_helper/wrappable.h"
@@ -36,6 +36,8 @@ class Handle;
 
 namespace electron::api {
 
+class Session;
+
 class WebRequest final : public gin_helper::DeprecatedWrappable<WebRequest> {
  public:
   using BeforeSendHeadersCallback =
@@ -43,23 +45,16 @@ class WebRequest final : public gin_helper::DeprecatedWrappable<WebRequest> {
                               const std::set<std::string>& set_headers,
                               int error_code)>;
 
-  // Return the WebRequest object attached to |browser_context|, create if there
-  // is no one.
-  // Note that the lifetime of WebRequest object is managed by Session, instead
-  // of the caller.
+  // Convenience wrapper around api::Session::FromOrCreate()->WebRequest().
+  // Creates the Session and WebRequest if they don't already exist.
+  // Note that the WebRequest is owned by the session, not by the caller.
   static gin_helper::Handle<WebRequest> FromOrCreate(
       v8::Isolate* isolate,
       content::BrowserContext* browser_context);
 
-  // Return a new WebRequest object, this should only be called by Session.
-  static gin_helper::Handle<WebRequest> Create(
-      v8::Isolate* isolate,
-      content::BrowserContext* browser_context);
-
-  // Find the WebRequest object attached to |browser_context|.
-  static gin_helper::Handle<WebRequest> From(
-      v8::Isolate* isolate,
-      content::BrowserContext* browser_context);
+  // Return a new WebRequest object. This can only be called by api::Session.
+  static gin_helper::Handle<WebRequest> Create(base::PassKey<Session>,
+                                               v8::Isolate* isolate);
 
   static const char* GetClassName() { return "WebRequest"; }
 
@@ -102,7 +97,7 @@ class WebRequest final : public gin_helper::DeprecatedWrappable<WebRequest> {
   void OnRequestWillBeDestroyed(extensions::WebRequestInfo* info);
 
  private:
-  WebRequest(v8::Isolate* isolate, content::BrowserContext* browser_context);
+  explicit WebRequest(base::PassKey<Session>);
   ~WebRequest() override;
 
   // Contains info about requests that are blocked waiting for a response from
@@ -213,9 +208,6 @@ class WebRequest final : public gin_helper::DeprecatedWrappable<WebRequest> {
   std::map<SimpleEvent, SimpleListenerInfo> simple_listeners_;
   std::map<ResponseEvent, ResponseListenerInfo> response_listeners_;
   std::map<uint64_t, BlockedRequest> blocked_requests_;
-
-  // Weak-ref, it manages us.
-  raw_ptr<content::BrowserContext> browser_context_;
 };
 
 }  // namespace electron::api