From c7bdd907d7e1fdcb7741de02f5ca23d47a436437 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Mon, 10 Jul 2023 11:49:20 +0200 Subject: [PATCH] fix: set prototype names on `gin::Constructible` classes (#39006) * fix: set prototype names on gin::Constructible classes * test: add tests --- shell/browser/api/electron_api_browser_view.cc | 6 +++++- shell/browser/api/electron_api_browser_view.h | 2 ++ shell/browser/api/electron_api_menu.cc | 4 ++++ shell/browser/api/electron_api_menu.h | 2 ++ shell/browser/api/electron_api_notification.cc | 6 +++++- shell/browser/api/electron_api_notification.h | 2 ++ shell/browser/api/electron_api_protocol.cc | 4 ++-- shell/browser/api/electron_api_protocol.h | 8 +++++--- shell/browser/api/electron_api_session.cc | 4 ++-- shell/browser/api/electron_api_session.h | 1 + shell/browser/api/electron_api_tray.cc | 6 +++++- shell/browser/api/electron_api_tray.h | 3 +++ shell/browser/api/electron_api_web_contents.cc | 4 ++-- shell/browser/api/electron_api_web_contents.h | 5 ++++- shell/browser/api/electron_api_web_frame_main.cc | 2 +- shell/browser/api/electron_api_web_frame_main.h | 5 ++++- shell/common/gin_helper/constructible.h | 1 + shell/common/gin_helper/event.h | 1 + spec/api-browser-view-spec.ts | 4 ++++ spec/api-browser-window-spec.ts | 4 ++++ spec/api-menu-spec.ts | 4 ++++ spec/api-notification-spec.ts | 4 ++++ spec/api-tray-spec.ts | 4 ++++ 23 files changed, 71 insertions(+), 15 deletions(-) diff --git a/shell/browser/api/electron_api_browser_view.cc b/shell/browser/api/electron_api_browser_view.cc index 2049664c6abf..9f703f1ec647 100644 --- a/shell/browser/api/electron_api_browser_view.cc +++ b/shell/browser/api/electron_api_browser_view.cc @@ -198,7 +198,7 @@ v8::Local BrowserView::GetWebContents(v8::Isolate* isolate) { // static void BrowserView::FillObjectTemplate(v8::Isolate* isolate, v8::Local templ) { - gin::ObjectTemplateBuilder(isolate, "BrowserView", templ) + gin::ObjectTemplateBuilder(isolate, GetClassName(), templ) .SetMethod("setAutoResize", &BrowserView::SetAutoResize) .SetMethod("setBounds", &BrowserView::SetBounds) .SetMethod("getBounds", &BrowserView::GetBounds) @@ -207,6 +207,10 @@ void BrowserView::FillObjectTemplate(v8::Isolate* isolate, .Build(); } +const char* BrowserView::GetTypeName() { + return GetClassName(); +} + } // namespace electron::api namespace { diff --git a/shell/browser/api/electron_api_browser_view.h b/shell/browser/api/electron_api_browser_view.h index caf31bae155a..6cf9ee20fdc2 100644 --- a/shell/browser/api/electron_api_browser_view.h +++ b/shell/browser/api/electron_api_browser_view.h @@ -45,9 +45,11 @@ class BrowserView : public gin::Wrappable, static gin::Handle New(gin_helper::ErrorThrower thrower, gin::Arguments* args); static void FillObjectTemplate(v8::Isolate*, v8::Local); + static const char* GetClassName() { return "BrowserView"; } // gin::Wrappable static gin::WrapperInfo kWrapperInfo; + const char* GetTypeName() override; WebContents* web_contents() const { return api_web_contents_; } NativeBrowserView* view() const { return view_.get(); } diff --git a/shell/browser/api/electron_api_menu.cc b/shell/browser/api/electron_api_menu.cc index 8e3d46fbe4a8..257013e63fbd 100644 --- a/shell/browser/api/electron_api_menu.cc +++ b/shell/browser/api/electron_api_menu.cc @@ -299,6 +299,10 @@ void Menu::FillObjectTemplate(v8::Isolate* isolate, .Build(); } +const char* Menu::GetTypeName() { + return GetClassName(); +} + } // namespace electron::api namespace { diff --git a/shell/browser/api/electron_api_menu.h b/shell/browser/api/electron_api_menu.h index f52b7c23de96..3cb0681efd7e 100644 --- a/shell/browser/api/electron_api_menu.h +++ b/shell/browser/api/electron_api_menu.h @@ -29,9 +29,11 @@ class Menu : public gin::Wrappable, // gin_helper::Constructible static gin::Handle New(gin::Arguments* args); static void FillObjectTemplate(v8::Isolate*, v8::Local); + static const char* GetClassName() { return "Menu"; } // gin::Wrappable static gin::WrapperInfo kWrapperInfo; + const char* GetTypeName() override; #if BUILDFLAG(IS_MAC) // Set the global menubar. diff --git a/shell/browser/api/electron_api_notification.cc b/shell/browser/api/electron_api_notification.cc index ecc2d8bf0ba2..76f4cc3c4e4c 100644 --- a/shell/browser/api/electron_api_notification.cc +++ b/shell/browser/api/electron_api_notification.cc @@ -256,7 +256,7 @@ bool Notification::IsSupported() { void Notification::FillObjectTemplate(v8::Isolate* isolate, v8::Local templ) { - gin::ObjectTemplateBuilder(isolate, "Notification", templ) + gin::ObjectTemplateBuilder(isolate, GetClassName(), templ) .SetMethod("show", &Notification::Show) .SetMethod("close", &Notification::Close) .SetProperty("title", &Notification::GetTitle, &Notification::SetTitle) @@ -282,6 +282,10 @@ void Notification::FillObjectTemplate(v8::Isolate* isolate, .Build(); } +const char* Notification::GetTypeName() { + return GetClassName(); +} + } // namespace electron::api namespace { diff --git a/shell/browser/api/electron_api_notification.h b/shell/browser/api/electron_api_notification.h index 3a4a46d32756..fade205150a1 100644 --- a/shell/browser/api/electron_api_notification.h +++ b/shell/browser/api/electron_api_notification.h @@ -40,6 +40,7 @@ class Notification : public gin::Wrappable, static gin::Handle New(gin_helper::ErrorThrower thrower, gin::Arguments* args); static void FillObjectTemplate(v8::Isolate*, v8::Local); + static const char* GetClassName() { return "Notification"; } // NotificationDelegate: void NotificationAction(int index) override; @@ -52,6 +53,7 @@ class Notification : public gin::Wrappable, // gin::Wrappable static gin::WrapperInfo kWrapperInfo; + const char* GetTypeName() override; // disable copy Notification(const Notification&) = delete; diff --git a/shell/browser/api/electron_api_protocol.cc b/shell/browser/api/electron_api_protocol.cc index 59b8ad6ced88..444964e79a75 100644 --- a/shell/browser/api/electron_api_protocol.cc +++ b/shell/browser/api/electron_api_protocol.cc @@ -283,7 +283,7 @@ gin::Handle Protocol::New(gin_helper::ErrorThrower thrower) { v8::Local Protocol::FillObjectTemplate( v8::Isolate* isolate, v8::Local tmpl) { - return gin::ObjectTemplateBuilder(isolate, "Protocol", tmpl) + return gin::ObjectTemplateBuilder(isolate, GetClassName(), tmpl) .SetMethod("registerStringProtocol", &Protocol::RegisterProtocolFor) .SetMethod("registerBufferProtocol", @@ -317,7 +317,7 @@ v8::Local Protocol::FillObjectTemplate( } const char* Protocol::GetTypeName() { - return "Protocol"; + return GetClassName(); } } // namespace electron::api diff --git a/shell/browser/api/electron_api_protocol.h b/shell/browser/api/electron_api_protocol.h index ef69d93726bd..8fc493473b8a 100644 --- a/shell/browser/api/electron_api_protocol.h +++ b/shell/browser/api/electron_api_protocol.h @@ -45,13 +45,15 @@ class Protocol : public gin::Wrappable, static gin::Handle Create(v8::Isolate* isolate, ElectronBrowserContext* browser_context); + // gin_helper::Constructible static gin::Handle New(gin_helper::ErrorThrower thrower); - - // gin::Wrappable - static gin::WrapperInfo kWrapperInfo; static v8::Local FillObjectTemplate( v8::Isolate* isolate, v8::Local tmpl); + static const char* GetClassName() { return "Protocol"; } + + // gin::Wrappable + static gin::WrapperInfo kWrapperInfo; const char* GetTypeName() override; private: diff --git a/shell/browser/api/electron_api_session.cc b/shell/browser/api/electron_api_session.cc index 82594de091e1..660b9b730a82 100644 --- a/shell/browser/api/electron_api_session.cc +++ b/shell/browser/api/electron_api_session.cc @@ -1301,7 +1301,7 @@ gin::Handle Session::New() { void Session::FillObjectTemplate(v8::Isolate* isolate, v8::Local templ) { - gin::ObjectTemplateBuilder(isolate, "Session", templ) + gin::ObjectTemplateBuilder(isolate, GetClassName(), templ) .SetMethod("resolveHost", &Session::ResolveHost) .SetMethod("resolveProxy", &Session::ResolveProxy) .SetMethod("getCacheSize", &Session::GetCacheSize) @@ -1379,7 +1379,7 @@ void Session::FillObjectTemplate(v8::Isolate* isolate, } const char* Session::GetTypeName() { - return "Session"; + return GetClassName(); } } // namespace electron::api diff --git a/shell/browser/api/electron_api_session.h b/shell/browser/api/electron_api_session.h index dec77b2c5eca..a71609273cba 100644 --- a/shell/browser/api/electron_api_session.h +++ b/shell/browser/api/electron_api_session.h @@ -95,6 +95,7 @@ class Session : public gin::Wrappable, // gin::Wrappable static gin::WrapperInfo kWrapperInfo; static void FillObjectTemplate(v8::Isolate*, v8::Local); + static const char* GetClassName() { return "Session"; } const char* GetTypeName() override; // Methods. diff --git a/shell/browser/api/electron_api_tray.cc b/shell/browser/api/electron_api_tray.cc index 6a6e5d7397f3..e1bcab8ffcc7 100644 --- a/shell/browser/api/electron_api_tray.cc +++ b/shell/browser/api/electron_api_tray.cc @@ -386,7 +386,7 @@ bool Tray::CheckAlive() { // static void Tray::FillObjectTemplate(v8::Isolate* isolate, v8::Local templ) { - gin::ObjectTemplateBuilder(isolate, "Tray", templ) + gin::ObjectTemplateBuilder(isolate, GetClassName(), templ) .SetMethod("destroy", &Tray::Destroy) .SetMethod("isDestroyed", &Tray::IsDestroyed) .SetMethod("setImage", &Tray::SetImage) @@ -408,6 +408,10 @@ void Tray::FillObjectTemplate(v8::Isolate* isolate, .Build(); } +const char* Tray::GetTypeName() { + return GetClassName(); +} + } // namespace electron::api namespace { diff --git a/shell/browser/api/electron_api_tray.h b/shell/browser/api/electron_api_tray.h index ba28862e87b4..c7a3f7582176 100644 --- a/shell/browser/api/electron_api_tray.h +++ b/shell/browser/api/electron_api_tray.h @@ -45,10 +45,13 @@ class Tray : public gin::Wrappable, v8::Local image, absl::optional guid, gin::Arguments* args); + static void FillObjectTemplate(v8::Isolate*, v8::Local); + static const char* GetClassName() { return "Tray"; } // gin::Wrappable static gin::WrapperInfo kWrapperInfo; + const char* GetTypeName() override; // disable copy Tray(const Tray&) = delete; diff --git a/shell/browser/api/electron_api_web_contents.cc b/shell/browser/api/electron_api_web_contents.cc index 5f4d985f8b72..6451752385e5 100644 --- a/shell/browser/api/electron_api_web_contents.cc +++ b/shell/browser/api/electron_api_web_contents.cc @@ -4161,7 +4161,7 @@ void WebContents::FillObjectTemplate(v8::Isolate* isolate, v8::Local templ) { gin::InvokerOptions options; options.holder_is_first_argument = true; - options.holder_type = "WebContents"; + options.holder_type = GetClassName(); templ->Set( gin::StringToSymbol(isolate, "isDestroyed"), gin::CreateFunctionTemplate( @@ -4301,7 +4301,7 @@ void WebContents::FillObjectTemplate(v8::Isolate* isolate, } const char* WebContents::GetTypeName() { - return "WebContents"; + return GetClassName(); } ElectronBrowserContext* WebContents::GetBrowserContext() const { diff --git a/shell/browser/api/electron_api_web_contents.h b/shell/browser/api/electron_api_web_contents.h index becf6ee920e6..e15280832775 100644 --- a/shell/browser/api/electron_api_web_contents.h +++ b/shell/browser/api/electron_api_web_contents.h @@ -147,9 +147,12 @@ class WebContents : public ExclusiveAccessContext, v8::Isolate* isolate, const gin_helper::Dictionary& web_preferences); + // gin_helper::Constructible + static void FillObjectTemplate(v8::Isolate*, v8::Local); + static const char* GetClassName() { return "WebContents"; } + // gin::Wrappable static gin::WrapperInfo kWrapperInfo; - static void FillObjectTemplate(v8::Isolate*, v8::Local); const char* GetTypeName() override; void Destroy(); diff --git a/shell/browser/api/electron_api_web_frame_main.cc b/shell/browser/api/electron_api_web_frame_main.cc index f2d86fdd8b35..a1d06620f2d7 100644 --- a/shell/browser/api/electron_api_web_frame_main.cc +++ b/shell/browser/api/electron_api_web_frame_main.cc @@ -409,7 +409,7 @@ void WebFrameMain::FillObjectTemplate(v8::Isolate* isolate, } const char* WebFrameMain::GetTypeName() { - return "WebFrameMain"; + return GetClassName(); } } // namespace electron::api diff --git a/shell/browser/api/electron_api_web_frame_main.h b/shell/browser/api/electron_api_web_frame_main.h index 56fb2eb3ac8c..d3ac3914d64c 100644 --- a/shell/browser/api/electron_api_web_frame_main.h +++ b/shell/browser/api/electron_api_web_frame_main.h @@ -52,9 +52,12 @@ class WebFrameMain : public gin::Wrappable, static WebFrameMain* FromRenderFrameHost( content::RenderFrameHost* render_frame_host); + // gin_helper::Constructible + static void FillObjectTemplate(v8::Isolate*, v8::Local); + static const char* GetClassName() { return "WebFrameMain"; } + // gin::Wrappable static gin::WrapperInfo kWrapperInfo; - static void FillObjectTemplate(v8::Isolate*, v8::Local); const char* GetTypeName() override; content::RenderFrameHost* render_frame_host() const { return render_frame_; } diff --git a/shell/common/gin_helper/constructible.h b/shell/common/gin_helper/constructible.h index 8d396bdf6419..908df416a847 100644 --- a/shell/common/gin_helper/constructible.h +++ b/shell/common/gin_helper/constructible.h @@ -55,6 +55,7 @@ class Constructible { } constructor->InstanceTemplate()->SetInternalFieldCount( gin::kNumberOfInternalFields); + constructor->SetClassName(gin::StringToV8(isolate, T::GetClassName())); T::FillObjectTemplate(isolate, constructor->PrototypeTemplate()); data->SetObjectTemplate(wrapper_info, constructor->InstanceTemplate()); data->SetFunctionTemplate(wrapper_info, constructor); diff --git a/shell/common/gin_helper/event.h b/shell/common/gin_helper/event.h index ce4880c8f5e4..b55426b9f536 100644 --- a/shell/common/gin_helper/event.h +++ b/shell/common/gin_helper/event.h @@ -27,6 +27,7 @@ class Event : public gin::Wrappable, static v8::Local FillObjectTemplate( v8::Isolate* isolate, v8::Local prototype); + static const char* GetClassName() { return "Event"; } // gin::Wrappable static gin::WrapperInfo kWrapperInfo; diff --git a/spec/api-browser-view-spec.ts b/spec/api-browser-view-spec.ts index 80e26e21d730..3932510d2118 100644 --- a/spec/api-browser-view-spec.ts +++ b/spec/api-browser-view-spec.ts @@ -40,6 +40,10 @@ describe('BrowserView module', () => { expect(webContents.getAllWebContents()).to.have.length(0); }); + it('sets the correct class name on the prototype', () => { + expect(BrowserView.prototype.constructor.name).to.equal('BrowserView'); + }); + it('can be created with an existing webContents', async () => { const wc = (webContents as typeof ElectronInternal.WebContents).create({ sandbox: true }); await wc.loadURL('about:blank'); diff --git a/spec/api-browser-window-spec.ts b/spec/api-browser-window-spec.ts index 6fd7fb9d8825..25071881ff51 100644 --- a/spec/api-browser-window-spec.ts +++ b/spec/api-browser-window-spec.ts @@ -47,6 +47,10 @@ const isBeforeUnload = (event: Event, level: number, message: string) => { }; describe('BrowserWindow module', () => { + it('sets the correct class name on the prototype', () => { + expect(BrowserWindow.prototype.constructor.name).to.equal('BrowserWindow'); + }); + describe('BrowserWindow constructor', () => { it('allows passing void 0 as the webContents', async () => { expect(() => { diff --git a/spec/api-menu-spec.ts b/spec/api-menu-spec.ts index b27f0d3a4da8..95d7cc197047 100644 --- a/spec/api-menu-spec.ts +++ b/spec/api-menu-spec.ts @@ -11,6 +11,10 @@ import { setTimeout } from 'node:timers/promises'; const fixturesPath = path.resolve(__dirname, 'fixtures'); describe('Menu module', function () { + it('sets the correct class name on the prototype', () => { + expect(Menu.prototype.constructor.name).to.equal('Menu'); + }); + describe('Menu.buildFromTemplate', () => { it('should be able to attach extra fields', () => { const menu = Menu.buildFromTemplate([ diff --git a/spec/api-notification-spec.ts b/spec/api-notification-spec.ts index b56d17c1fb80..27188358fbd9 100644 --- a/spec/api-notification-spec.ts +++ b/spec/api-notification-spec.ts @@ -4,6 +4,10 @@ import { once } from 'node:events'; import { ifit } from './lib/spec-helpers'; describe('Notification module', () => { + it('sets the correct class name on the prototype', () => { + expect(Notification.prototype.constructor.name).to.equal('Notification'); + }); + it('is supported', () => { expect(Notification.isSupported()).to.be.a('boolean'); }); diff --git a/spec/api-tray-spec.ts b/spec/api-tray-spec.ts index ac098c9bee63..a5a0b2aed1a2 100644 --- a/spec/api-tray-spec.ts +++ b/spec/api-tray-spec.ts @@ -15,6 +15,10 @@ describe('tray module', () => { }); describe('new Tray', () => { + it('sets the correct class name on the prototype', () => { + expect(Tray.prototype.constructor.name).to.equal('Tray'); + }); + it('throws a descriptive error for a missing file', () => { const badPath = path.resolve('I', 'Do', 'Not', 'Exist'); expect(() => {