refactor: ginify WebContents (#24651)

docs/api/structures/keyboard-event.md

@@ -1,4 +1,4 @@
-# KeyboardEvent Object extends `Event`
+# KeyboardEvent Object
 
 * `ctrlKey` Boolean (optional) - whether the Control key was used in an accelerator to trigger the Event
 * `metaKey` Boolean (optional) - whether a meta key was used in an accelerator to trigger the Event

lib/browser/api/web-contents.ts

@@ -9,7 +9,6 @@ import { ipcMainInternal } from '@electron/internal/browser/ipc-main-internal';
 import * as ipcMainUtils from '@electron/internal/browser/ipc-main-internal-utils';
 import { parseFeatures } from '@electron/internal/common/parse-features-string';
 import { MessagePortMain } from '@electron/internal/browser/message-port-main';
-import { EventEmitter } from 'events';
 
 // session is not used here, the purpose is to make sure session is initalized
 // before the webContents module.
@@ -124,8 +123,6 @@ const defaultPrintingSetting = {
 const binding = process._linkedBinding('electron_browser_web_contents');
 const { WebContents } = binding as { WebContents: { prototype: Electron.WebContentsInternal } };
 
-Object.setPrototypeOf(WebContents.prototype, EventEmitter.prototype);
-
 WebContents.prototype.send = function (channel, ...args) {
   if (typeof channel !== 'string') {
     throw new Error('Missing required channel argument');

shell/browser/api/electron_api_browser_window.cc

@@ -78,7 +78,7 @@ BrowserWindow::BrowserWindow(gin::Arguments* args,
   Observe(api_web_contents_->web_contents());
 
   // Keep a copy of the options for later use.
-  gin_helper::Dictionary(isolate, web_contents->GetWrapper())
+  gin_helper::Dictionary(isolate, web_contents.ToV8().As<v8::Object>())
       .Set("browserWindowOptions", options);
 
   // Associate with BrowserWindow.

shell/browser/api/electron_api_web_contents.cc

@@ -12,6 +12,7 @@
 #include <utility>
 #include <vector>
 
+#include "base/containers/id_map.h"
 #include "base/no_destructor.h"
 #include "base/optional.h"
 #include "base/strings/utf_string_conversions.h"
@@ -332,6 +333,11 @@ namespace api {
 
 namespace {
 
+base::IDMap<WebContents*>& GetAllWebContents() {
+  static base::NoDestructor<base::IDMap<WebContents*>> s_all_web_contents;
+  return *s_all_web_contents;
+}
+
 // Called when CapturePage is done.
 void OnCapturePageDone(gin_helper::Promise<gfx::Image> promise,
                        const SkBitmap& bitmap) {
@@ -389,12 +395,21 @@ base::string16 GetDefaultPrinterAsync() {
 }
 #endif
 
+struct UserDataLink : public base::SupportsUserData::Data {
+  explicit UserDataLink(base::WeakPtr<WebContents> contents)
+      : web_contents(contents) {}
+
+  base::WeakPtr<WebContents> web_contents;
+};
+const void* kElectronApiWebContentsKey = &kElectronApiWebContentsKey;
+
 }  // namespace
 
 WebContents::WebContents(v8::Isolate* isolate,
                          content::WebContents* web_contents)
     : content::WebContentsObserver(web_contents),
       type_(Type::REMOTE),
+      id_(GetAllWebContents().Add(this)),
       weak_factory_(this) {
   auto session = Session::CreateFrom(isolate, GetBrowserContext());
   session_.Reset(isolate, session.ToV8());
@@ -402,8 +417,8 @@ WebContents::WebContents(v8::Isolate* isolate,
   web_contents->SetUserAgentOverride(blink::UserAgentOverride::UserAgentOnly(
                                          GetBrowserContext()->GetUserAgent()),
                                      false);
-  Init(isolate);
-  AttachAsUserData(web_contents);
+  web_contents->SetUserData(kElectronApiWebContentsKey,
+                            std::make_unique<UserDataLink>(GetWeakPtr()));
   InitZoomController(web_contents, gin::Dictionary::CreateEmpty(isolate));
 #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)
   extensions::ElectronExtensionWebContentsObserver::CreateForWebContents(
@@ -421,6 +436,7 @@ WebContents::WebContents(v8::Isolate* isolate,
                          Type type)
     : content::WebContentsObserver(web_contents.get()),
       type_(type),
+      id_(GetAllWebContents().Add(this)),
       weak_factory_(this) {
   DCHECK(type != Type::REMOTE)
       << "Can't take ownership of a remote WebContents";
@@ -432,7 +448,7 @@ WebContents::WebContents(v8::Isolate* isolate,
 
 WebContents::WebContents(v8::Isolate* isolate,
                          const gin_helper::Dictionary& options)
-    : weak_factory_(this) {
+    : id_(GetAllWebContents().Add(this)), weak_factory_(this) {
   // Read options.
   options.Get("backgroundThrottling", &background_throttling_);
 
@@ -610,11 +626,12 @@ void WebContents::InitWithSessionAndOptions(
       SetOwnerWindow(owner_window);
   }
 
-  Init(isolate);
-  AttachAsUserData(web_contents());
+  web_contents()->SetUserData(kElectronApiWebContentsKey,
+                              std::make_unique<UserDataLink>(GetWeakPtr()));
 }
 
 WebContents::~WebContents() {
+  MarkDestroyed();
   // The destroy() is called.
   if (managed_web_contents()) {
     managed_web_contents()->GetView()->SetDelegate(nullptr);
@@ -740,19 +757,19 @@ void WebContents::AddNewContents(
 content::WebContents* WebContents::OpenURLFromTab(
     content::WebContents* source,
     const content::OpenURLParams& params) {
+  auto weak_this = GetWeakPtr();
   if (params.disposition != WindowOpenDisposition::CURRENT_TAB) {
     Emit("-new-window", params.url, "", params.disposition, "", params.referrer,
          params.post_data);
     return nullptr;
   }
+  if (!weak_this)
+    return nullptr;
 
   // Give user a chance to cancel navigation.
   if (Emit("will-navigate", params.url))
     return nullptr;
-
-  // Don't load the URL if the web contents was marked as destroyed from a
-  // will-navigate event listener
-  if (IsDestroyed())
+  if (!weak_this)
     return nullptr;
 
   return CommonWebContentsDelegate::OpenURLFromTab(source, params);
@@ -1217,9 +1234,7 @@ void WebContents::MessageTo(bool internal,
                             const std::string& channel,
                             blink::CloneableMessage arguments) {
   TRACE_EVENT1("electron", "WebContents::MessageTo", "channel", channel);
-  v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
-  auto* web_contents = gin_helper::TrackableObject<WebContents>::FromWeakMapID(
-      isolate, web_contents_id);
+  auto* web_contents = FromID(web_contents_id);
 
   if (web_contents) {
     web_contents->SendIPCMessageWithSender(internal, send_to_all, channel,
@@ -1398,6 +1413,17 @@ bool WebContents::OnMessageReceived(const IPC::Message& message) {
   return handled;
 }
 
+void WebContents::MarkDestroyed() {
+  if (GetAllWebContents().Lookup(id_))
+    GetAllWebContents().Remove(id_);
+  v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
+  v8::HandleScope scope(isolate);
+  v8::Local<v8::Object> wrapper;
+  if (!GetWrapper(isolate).ToLocal(&wrapper))
+    return;
+  wrapper->SetAlignedPointerInInternalField(0, nullptr);
+}
+
 // There are three ways of destroying a webContents:
 // 1. call webContents.destroy();
 // 2. garbage collection;
@@ -1419,7 +1445,6 @@ void WebContents::WebContentsDestroyed() {
     guest_delegate_->WillDestroy();
 
   // Cleanup relationships with other parts.
-  RemoveFromWeakMap();
 
   // We can not call Destroy here because we need to call Emit first, but we
   // also do not want any method to be used, so just mark as destroyed here.
@@ -1435,7 +1460,13 @@ void WebContents::WebContentsDestroyed() {
   }
 
   // Destroy the native class in next tick.
-  base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, GetDestroyClosure());
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::BindOnce(
+                     [](base::WeakPtr<WebContents> wc) {
+                       if (wc)
+                         delete wc.get();
+                     },
+                     GetWeakPtr()));
 }
 
 void WebContents::NavigationEntryCommitted(
@@ -2632,10 +2663,6 @@ v8::Local<v8::Value> WebContents::GetOwnerBrowserWindow(
     return v8::Null(isolate);
 }
 
-int32_t WebContents::ID() const {
-  return weak_map_id();
-}
-
 v8::Local<v8::Value> WebContents::Session(v8::Isolate* isolate) {
   return v8::Local<v8::Value>::New(isolate, session_);
 }
@@ -2754,11 +2781,28 @@ v8::Local<v8::Promise> WebContents::TakeHeapSnapshot(
 }
 
 // static
-void WebContents::BuildPrototype(v8::Isolate* isolate,
-                                 v8::Local<v8::FunctionTemplate> prototype) {
-  prototype->SetClassName(gin::StringToV8(isolate, "WebContents"));
-  gin_helper::Destroyable::MakeDestroyable(isolate, prototype);
-  gin_helper::ObjectTemplateBuilder(isolate, prototype->PrototypeTemplate())
+v8::Local<v8::ObjectTemplate> WebContents::FillObjectTemplate(
+    v8::Isolate* isolate,
+    v8::Local<v8::ObjectTemplate> templ) {
+  gin::InvokerOptions options;
+  options.holder_is_first_argument = true;
+  options.holder_type = "WebContents";
+  templ->Set(
+      gin::StringToSymbol(isolate, "isDestroyed"),
+      gin::CreateFunctionTemplate(
+          isolate, base::BindRepeating(&gin_helper::Destroyable::IsDestroyed),
+          options));
+  templ->Set(
+      gin::StringToSymbol(isolate, "destroy"),
+      gin::CreateFunctionTemplate(
+          isolate, base::BindRepeating([](gin::Handle<WebContents> handle) {
+            delete handle.get();
+          }),
+          options));
+  // We use gin_helper::ObjectTemplateBuilder instead of
+  // gin::ObjectTemplateBuilder here to handle the fact that WebContents is
+  // destroyable.
+  return gin_helper::ObjectTemplateBuilder(isolate, templ)
       .SetMethod("getBackgroundThrottling",
                  &WebContents::GetBackgroundThrottling)
       .SetMethod("setBackgroundThrottling",
@@ -2870,7 +2914,12 @@ void WebContents::BuildPrototype(v8::Isolate* isolate,
       .SetProperty("session", &WebContents::Session)
       .SetProperty("hostWebContents", &WebContents::HostWebContents)
       .SetProperty("devToolsWebContents", &WebContents::DevToolsWebContents)
-      .SetProperty("debugger", &WebContents::Debugger);
+      .SetProperty("debugger", &WebContents::Debugger)
+      .Build();
+}
+
+const char* WebContents::GetTypeName() {
+  return "WebContents";
 }
 
 ElectronBrowserContext* WebContents::GetBrowserContext() const {
@@ -2882,7 +2931,17 @@ ElectronBrowserContext* WebContents::GetBrowserContext() const {
 gin::Handle<WebContents> WebContents::Create(
     v8::Isolate* isolate,
     const gin_helper::Dictionary& options) {
-  return gin::CreateHandle(isolate, new WebContents(isolate, options));
+  gin::Handle<WebContents> handle =
+      gin::CreateHandle(isolate, new WebContents(isolate, options));
+  gin_helper::CallMethod(isolate, handle.get(), "_init");
+  return handle;
+}
+
+// static
+gin::Handle<WebContents> WebContents::New(
+    v8::Isolate* isolate,
+    const gin_helper::Dictionary& options) {
+  return Create(isolate, options);
 }
 
 // static
@@ -2890,15 +2949,19 @@ gin::Handle<WebContents> WebContents::CreateAndTake(
     v8::Isolate* isolate,
     std::unique_ptr<content::WebContents> web_contents,
     Type type) {
-  return gin::CreateHandle(
+  gin::Handle<WebContents> handle = gin::CreateHandle(
       isolate, new WebContents(isolate, std::move(web_contents), type));
+  gin_helper::CallMethod(isolate, handle.get(), "_init");
+  return handle;
 }
 
 // static
 WebContents* WebContents::From(content::WebContents* web_contents) {
-  v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
-  auto* existing = TrackableObject::FromWrappedClass(isolate, web_contents);
-  return static_cast<WebContents*>(existing);
+  if (!web_contents)
+    return nullptr;
+  UserDataLink* data = static_cast<UserDataLink*>(
+      web_contents->GetUserData(kElectronApiWebContentsKey));
+  return data ? data->web_contents.get() : nullptr;
 }
 
 // static
@@ -2906,36 +2969,50 @@ gin::Handle<WebContents> WebContents::FromOrCreate(
     v8::Isolate* isolate,
     content::WebContents* web_contents) {
   WebContents* api_web_contents = From(web_contents);
-  if (!api_web_contents)
+  if (!api_web_contents) {
     api_web_contents = new WebContents(isolate, web_contents);
+    gin_helper::CallMethod(isolate, api_web_contents, "_init");
+  }
   return gin::CreateHandle(isolate, api_web_contents);
 }
 
 // static
 WebContents* WebContents::FromID(int32_t id) {
-  return FromWeakMapID(JavascriptEnvironment::GetIsolate(), id);
+  return GetAllWebContents().Lookup(id);
 }
 
+// static
+gin::WrapperInfo WebContents::kWrapperInfo = {gin::kEmbedderNativeGin};
+
 }  // namespace api
 
 }  // namespace electron
 
 namespace {
 
+using electron::api::GetAllWebContents;
 using electron::api::WebContents;
 
+std::vector<gin::Handle<WebContents>> GetAllWebContentsAsV8(
+    v8::Isolate* isolate) {
+  std::vector<gin::Handle<WebContents>> list;
+  for (auto iter = base::IDMap<WebContents*>::iterator(&GetAllWebContents());
+       !iter.IsAtEnd(); iter.Advance()) {
+    list.push_back(gin::CreateHandle(isolate, iter.GetCurrentValue()));
+  }
+  return list;
+}
+
 void Initialize(v8::Local<v8::Object> exports,
                 v8::Local<v8::Value> unused,
                 v8::Local<v8::Context> context,
                 void* priv) {
   v8::Isolate* isolate = context->GetIsolate();
   gin_helper::Dictionary dict(isolate, exports);
-  dict.Set("WebContents", WebContents::GetConstructor(isolate)
-                              ->GetFunction(context)
-                              .ToLocalChecked());
+  dict.Set("WebContents", WebContents::GetConstructor(context));
   dict.SetMethod("create", &WebContents::Create);
-  dict.SetMethod("fromId", &WebContents::FromWeakMapID);
-  dict.SetMethod("getAllWebContents", &WebContents::GetAll);
+  dict.SetMethod("fromId", &WebContents::FromID);
+  dict.SetMethod("getAllWebContents", &GetAllWebContentsAsV8);
 }
 
 }  // namespace

shell/browser/api/electron_api_web_contents.h

@@ -8,6 +8,7 @@
 #include <map>
 #include <memory>
 #include <string>
+#include <utility>
 #include <vector>
 
 #include "base/observer_list.h"
@@ -22,14 +23,17 @@
 #include "electron/buildflags/buildflags.h"
 #include "electron/shell/common/api/api.mojom.h"
 #include "gin/handle.h"
+#include "gin/wrappable.h"
 #include "mojo/public/cpp/bindings/receiver_set.h"
 #include "printing/buildflags/buildflags.h"
 #include "services/service_manager/public/cpp/binder_registry.h"
 #include "shell/browser/api/frame_subscriber.h"
 #include "shell/browser/api/save_page_handler.h"
 #include "shell/browser/common_web_contents_delegate.h"
+#include "shell/browser/event_emitter_mixin.h"
+#include "shell/common/gin_helper/cleaned_up_at_exit.h"
+#include "shell/common/gin_helper/constructible.h"
 #include "shell/common/gin_helper/error_thrower.h"
-#include "shell/common/gin_helper/trackable_object.h"
 #include "ui/gfx/image/image.h"
 
 #if BUILDFLAG(ENABLE_PRINTING)
@@ -131,7 +135,10 @@ class ExtendedWebContentsObserver : public base::CheckedObserver {
 };
 
 // Wrapper around the content::WebContents.
-class WebContents : public gin_helper::TrackableObject<WebContents>,
+class WebContents : public gin::Wrappable<WebContents>,
+                    public gin_helper::EventEmitterMixin<WebContents>,
+                    public gin_helper::Constructible<WebContents>,
+                    public gin_helper::CleanedUpAtExit,
                     public CommonWebContentsDelegate,
                     public content::WebContentsObserver,
                     public mojom::ElectronBrowser {
@@ -148,6 +155,8 @@ class WebContents : public gin_helper::TrackableObject<WebContents>,
   // Create a new WebContents and return the V8 wrapper of it.
   static gin::Handle<WebContents> Create(v8::Isolate* isolate,
                                          const gin_helper::Dictionary& options);
+  static gin::Handle<WebContents> New(v8::Isolate* isolate,
+                                      const gin_helper::Dictionary& options);
 
   // Create a new V8 wrapper for an existing |web_content|.
   //
@@ -170,8 +179,12 @@ class WebContents : public gin_helper::TrackableObject<WebContents>,
       v8::Isolate* isolate,
       content::WebContents* web_contents);
 
-  static void BuildPrototype(v8::Isolate* isolate,
-                             v8::Local<v8::FunctionTemplate> prototype);
+  // gin::Wrappable
+  static gin::WrapperInfo kWrapperInfo;
+  static v8::Local<v8::ObjectTemplate> FillObjectTemplate(
+      v8::Isolate*,
+      v8::Local<v8::ObjectTemplate>);
+  const char* GetTypeName() override;
 
   base::WeakPtr<WebContents> GetWeakPtr() { return weak_factory_.GetWeakPtr(); }
 
@@ -375,7 +388,7 @@ class WebContents : public gin_helper::TrackableObject<WebContents>,
                                           const base::FilePath& file_path);
 
   // Properties.
-  int32_t ID() const;
+  int32_t ID() const { return id_; }
   v8::Local<v8::Value> Session(v8::Isolate* isolate);
   content::WebContents* HostWebContents() const;
   v8::Local<v8::Value> DevToolsWebContents(v8::Isolate* isolate);
@@ -395,6 +408,25 @@ class WebContents : public gin_helper::TrackableObject<WebContents>,
   bool EmitNavigationEvent(const std::string& event,
                            content::NavigationHandle* navigation_handle);
 
+  // this.emit(name, new Event(sender, message), args...);
+  template <typename... Args>
+  bool EmitWithSender(base::StringPiece name,
+                      content::RenderFrameHost* sender,
+                      electron::mojom::ElectronBrowser::InvokeCallback callback,
+                      Args&&... args) {
+    DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+    v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
+    v8::HandleScope handle_scope(isolate);
+    v8::Local<v8::Object> wrapper;
+    if (!GetWrapper(isolate).ToLocal(&wrapper))
+      return false;
+    v8::Local<v8::Object> event = gin_helper::internal::CreateNativeEvent(
+        isolate, wrapper, sender, std::move(callback));
+    return EmitCustomEvent(name, event, std::forward<Args>(args)...);
+  }
+
+  void MarkDestroyed();
+
   WebContents* embedder() { return embedder_; }
 
 #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)
@@ -637,6 +669,8 @@ class WebContents : public gin_helper::TrackableObject<WebContents>,
   // The type of current WebContents.
   Type type_ = Type::BROWSER_WINDOW;
 
+  int32_t id_;
+
   // Request id used for findInPage request.
   uint32_t request_id_ = 0;
 

shell/browser/event_emitter_mixin.h

@@ -44,9 +44,7 @@ class EventEmitterMixin {
     v8::Local<v8::Object> wrapper;
     if (!static_cast<T*>(this)->GetWrapper(isolate).ToLocal(&wrapper))
       return false;
-    v8::Local<v8::Object> event =
-        internal::CreateEvent(isolate, wrapper, custom_event);
-    return EmitWithEvent(isolate, wrapper, name, event,
+    return EmitWithEvent(isolate, wrapper, name, custom_event,
                          std::forward<Args>(args)...);
   }
 

shell/common/gin_helper/constructor.h

@@ -157,7 +157,7 @@ v8::Local<v8::Function> CreateConstructor(
   CHECK(!called) << "CreateConstructor can only be called for one type once";
   called = true;
 #endif
-  v8::Local<v8::FunctionTemplate> templ = CreateFunctionTemplate(
+  v8::Local<v8::FunctionTemplate> templ = gin_helper::CreateFunctionTemplate(
       isolate, base::BindRepeating(&internal::InvokeNew<Sig>, func));
   templ->InstanceTemplate()->SetInternalFieldCount(1);
   T::BuildPrototype(isolate, templ);

shell/common/gin_helper/event_emitter.h

@@ -70,23 +70,6 @@ class EventEmitter : public gin_helper::Wrappable<T> {
     return EmitWithEvent(name, event, std::forward<Args>(args)...);
   }
 
-  // this.emit(name, new Event(sender, message), args...);
-  template <typename... Args>
-  bool EmitWithSender(base::StringPiece name,
-                      content::RenderFrameHost* sender,
-                      electron::mojom::ElectronBrowser::InvokeCallback callback,
-                      Args&&... args) {
-    DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-    v8::Locker locker(isolate());
-    v8::HandleScope handle_scope(isolate());
-    v8::Local<v8::Object> wrapper = GetWrapper();
-    if (wrapper.IsEmpty())
-      return false;
-    v8::Local<v8::Object> event = internal::CreateNativeEvent(
-        isolate(), wrapper, sender, std::move(callback));
-    return EmitWithEvent(name, event, std::forward<Args>(args)...);
-  }
-
  protected:
   EventEmitter() {}