refactor: move gin_helper::Constructible methods to prototype (#37087)

shell/browser/api/electron_api_browser_view.cc

@@ -192,10 +192,9 @@ v8::Local<v8::Value> BrowserView::GetWebContents(v8::Isolate* isolate) {
 }
 
 // static
-v8::Local<v8::ObjectTemplate> BrowserView::FillObjectTemplate(
-    v8::Isolate* isolate,
-    v8::Local<v8::ObjectTemplate> templ) {
-  return gin::ObjectTemplateBuilder(isolate, "BrowserView", templ)
+void BrowserView::FillObjectTemplate(v8::Isolate* isolate,
+                                     v8::Local<v8::ObjectTemplate> templ) {
+  gin::ObjectTemplateBuilder(isolate, "BrowserView", templ)
       .SetMethod("setAutoResize", &BrowserView::SetAutoResize)
       .SetMethod("setBounds", &BrowserView::SetBounds)
       .SetMethod("getBounds", &BrowserView::GetBounds)

shell/browser/api/electron_api_browser_view.h

@@ -44,9 +44,7 @@ class BrowserView : public gin::Wrappable<BrowserView>,
   // gin_helper::Constructible
   static gin::Handle<BrowserView> New(gin_helper::ErrorThrower thrower,
                                       gin::Arguments* args);
-  static v8::Local<v8::ObjectTemplate> FillObjectTemplate(
-      v8::Isolate*,
-      v8::Local<v8::ObjectTemplate>);
+  static void FillObjectTemplate(v8::Isolate*, v8::Local<v8::ObjectTemplate>);
 
   // gin::Wrappable
   static gin::WrapperInfo kWrapperInfo;

shell/browser/api/electron_api_menu.cc

@@ -266,10 +266,9 @@ void Menu::OnMenuWillShow() {
 }
 
 // static
-v8::Local<v8::ObjectTemplate> Menu::FillObjectTemplate(
-    v8::Isolate* isolate,
-    v8::Local<v8::ObjectTemplate> templ) {
-  return gin::ObjectTemplateBuilder(isolate, "Menu", templ)
+void Menu::FillObjectTemplate(v8::Isolate* isolate,
+                              v8::Local<v8::ObjectTemplate> templ) {
+  gin::ObjectTemplateBuilder(isolate, "Menu", templ)
       .SetMethod("insertItem", &Menu::InsertItemAt)
       .SetMethod("insertCheckItem", &Menu::InsertCheckItemAt)
       .SetMethod("insertRadioItem", &Menu::InsertRadioItemAt)

shell/browser/api/electron_api_menu.h

@@ -27,9 +27,7 @@ class Menu : public gin::Wrappable<Menu>,
  public:
   // gin_helper::Constructible
   static gin::Handle<Menu> New(gin::Arguments* args);
-  static v8::Local<v8::ObjectTemplate> FillObjectTemplate(
-      v8::Isolate*,
-      v8::Local<v8::ObjectTemplate>);
+  static void FillObjectTemplate(v8::Isolate*, v8::Local<v8::ObjectTemplate>);
 
   // gin::Wrappable
   static gin::WrapperInfo kWrapperInfo;

shell/browser/api/electron_api_notification.cc

@@ -253,10 +253,9 @@ bool Notification::IsSupported() {
                ->GetNotificationPresenter();
 }
 
-v8::Local<v8::ObjectTemplate> Notification::FillObjectTemplate(
-    v8::Isolate* isolate,
-    v8::Local<v8::ObjectTemplate> templ) {
-  return gin::ObjectTemplateBuilder(isolate, "Notification", templ)
+void Notification::FillObjectTemplate(v8::Isolate* isolate,
+                                      v8::Local<v8::ObjectTemplate> templ) {
+  gin::ObjectTemplateBuilder(isolate, "Notification", templ)
       .SetMethod("show", &Notification::Show)
       .SetMethod("close", &Notification::Close)
       .SetProperty("title", &Notification::GetTitle, &Notification::SetTitle)

shell/browser/api/electron_api_notification.h

@@ -38,9 +38,7 @@ class Notification : public gin::Wrappable<Notification>,
   // gin_helper::Constructible
   static gin::Handle<Notification> New(gin_helper::ErrorThrower thrower,
                                        gin::Arguments* args);
-  static v8::Local<v8::ObjectTemplate> FillObjectTemplate(
-      v8::Isolate*,
-      v8::Local<v8::ObjectTemplate>);
+  static void FillObjectTemplate(v8::Isolate*, v8::Local<v8::ObjectTemplate>);
 
   // NotificationDelegate:
   void NotificationAction(int index) override;

shell/browser/api/electron_api_tray.cc

@@ -394,10 +394,9 @@ bool Tray::CheckAlive() {
 }
 
 // static
-v8::Local<v8::ObjectTemplate> Tray::FillObjectTemplate(
-    v8::Isolate* isolate,
-    v8::Local<v8::ObjectTemplate> templ) {
-  return gin::ObjectTemplateBuilder(isolate, "Tray", templ)
+void Tray::FillObjectTemplate(v8::Isolate* isolate,
+                              v8::Local<v8::ObjectTemplate> templ) {
+  gin::ObjectTemplateBuilder(isolate, "Tray", templ)
       .SetMethod("destroy", &Tray::Destroy)
       .SetMethod("isDestroyed", &Tray::IsDestroyed)
       .SetMethod("setImage", &Tray::SetImage)

shell/browser/api/electron_api_tray.h

@@ -45,9 +45,7 @@ class Tray : public gin::Wrappable<Tray>,
                                v8::Local<v8::Value> image,
                                absl::optional<UUID> guid,
                                gin::Arguments* args);
-  static v8::Local<v8::ObjectTemplate> FillObjectTemplate(
-      v8::Isolate*,
-      v8::Local<v8::ObjectTemplate>);
+  static void FillObjectTemplate(v8::Isolate*, v8::Local<v8::ObjectTemplate>);
 
   // gin::Wrappable
   static gin::WrapperInfo kWrapperInfo;

shell/browser/api/electron_api_web_contents.cc

@@ -3941,9 +3941,8 @@ void WebContents::UpdateHtmlApiFullscreen(bool fullscreen) {
 }
 
 // static
-v8::Local<v8::ObjectTemplate> WebContents::FillObjectTemplate(
-    v8::Isolate* isolate,
-    v8::Local<v8::ObjectTemplate> templ) {
+void WebContents::FillObjectTemplate(v8::Isolate* isolate,
+                                     v8::Local<v8::ObjectTemplate> templ) {
   gin::InvokerOptions options;
   options.holder_is_first_argument = true;
   options.holder_type = "WebContents";
@@ -3955,7 +3954,7 @@ v8::Local<v8::ObjectTemplate> WebContents::FillObjectTemplate(
   // We use gin_helper::ObjectTemplateBuilder instead of
   // gin::ObjectTemplateBuilder here to handle the fact that WebContents is
   // destroyable.
-  return gin_helper::ObjectTemplateBuilder(isolate, templ)
+  gin_helper::ObjectTemplateBuilder(isolate, templ)
       .SetMethod("destroy", &WebContents::Destroy)
       .SetMethod("close", &WebContents::Close)
       .SetMethod("getBackgroundThrottling",

shell/browser/api/electron_api_web_contents.h

@@ -150,9 +150,7 @@ class WebContents : public ExclusiveAccessContext,
 
   // gin::Wrappable
   static gin::WrapperInfo kWrapperInfo;
-  static v8::Local<v8::ObjectTemplate> FillObjectTemplate(
-      v8::Isolate*,
-      v8::Local<v8::ObjectTemplate>);
+  static void FillObjectTemplate(v8::Isolate*, v8::Local<v8::ObjectTemplate>);
   const char* GetTypeName() override;
 
   void Destroy();

shell/browser/api/electron_api_web_frame_main.cc

@@ -386,10 +386,9 @@ gin::Handle<WebFrameMain> WebFrameMain::FromOrNull(
 }
 
 // static
-v8::Local<v8::ObjectTemplate> WebFrameMain::FillObjectTemplate(
-    v8::Isolate* isolate,
-    v8::Local<v8::ObjectTemplate> templ) {
-  return gin_helper::ObjectTemplateBuilder(isolate, templ)
+void WebFrameMain::FillObjectTemplate(v8::Isolate* isolate,
+                                      v8::Local<v8::ObjectTemplate> templ) {
+  gin_helper::ObjectTemplateBuilder(isolate, templ)
       .SetMethod("executeJavaScript", &WebFrameMain::ExecuteJavaScript)
       .SetMethod("reload", &WebFrameMain::Reload)
       .SetMethod("_send", &WebFrameMain::Send)

shell/browser/api/electron_api_web_frame_main.h

@@ -53,9 +53,7 @@ class WebFrameMain : public gin::Wrappable<WebFrameMain>,
 
   // gin::Wrappable
   static gin::WrapperInfo kWrapperInfo;
-  static v8::Local<v8::ObjectTemplate> FillObjectTemplate(
-      v8::Isolate*,
-      v8::Local<v8::ObjectTemplate>);
+  static void FillObjectTemplate(v8::Isolate*, v8::Local<v8::ObjectTemplate>);
   const char* GetTypeName() override;
 
   content::RenderFrameHost* render_frame_host() const { return render_frame_; }

shell/common/gin_helper/constructible.h

@@ -24,7 +24,7 @@ class EventEmitterMixin;
 //                   public gin_helper::Constructible<Example> {
 //    public:
 //     static gin::Handle<Tray> New(...usual gin method arguments...);
-//     static v8::Local<v8::ObjectTemplate> FillObjectTemplate(
+//     static void FillObjectTemplate(
 //         v8::Isolate*,
 //         v8::Local<v8::ObjectTemplate>);
 //   }
@@ -55,9 +55,8 @@ class Constructible {
       }
       constructor->InstanceTemplate()->SetInternalFieldCount(
           gin::kNumberOfInternalFields);
-      v8::Local<v8::ObjectTemplate> obj_templ =
-          T::FillObjectTemplate(isolate, constructor->InstanceTemplate());
-      data->SetObjectTemplate(wrapper_info, obj_templ);
+      T::FillObjectTemplate(isolate, constructor->PrototypeTemplate());
+      data->SetObjectTemplate(wrapper_info, constructor->InstanceTemplate());
       data->SetFunctionTemplate(wrapper_info, constructor);
     }
     return constructor->GetFunction(context).ToLocalChecked();

spec/api-web-frame-main-spec.ts

@@ -177,12 +177,12 @@ describe('webFrameMain module', () => {
       const w = new BrowserWindow({ show: false });
       await w.loadFile(path.join(subframesPath, 'frame.html'));
       const webFrame = w.webContents.mainFrame;
-      expect(webFrame).to.have.ownProperty('url').that.is.a('string');
-      expect(webFrame).to.have.ownProperty('frameTreeNodeId').that.is.a('number');
-      expect(webFrame).to.have.ownProperty('name').that.is.a('string');
-      expect(webFrame).to.have.ownProperty('osProcessId').that.is.a('number');
-      expect(webFrame).to.have.ownProperty('processId').that.is.a('number');
-      expect(webFrame).to.have.ownProperty('routingId').that.is.a('number');
+      expect(webFrame).to.have.property('url').that.is.a('string');
+      expect(webFrame).to.have.property('frameTreeNodeId').that.is.a('number');
+      expect(webFrame).to.have.property('name').that.is.a('string');
+      expect(webFrame).to.have.property('osProcessId').that.is.a('number');
+      expect(webFrame).to.have.property('processId').that.is.a('number');
+      expect(webFrame).to.have.property('routingId').that.is.a('number');
     });
   });
 

spec/chromium-spec.ts

@@ -2787,7 +2787,7 @@ describe('navigator.hid', () => {
     let haveDevices = false;
     let selectFired = false;
     w.webContents.session.on('select-hid-device', (event, details, callback) => {
-      expect(details.frame).to.have.ownProperty('frameTreeNodeId').that.is.a('number');
+      expect(details.frame).to.have.property('frameTreeNodeId').that.is.a('number');
       selectFired = true;
       if (details.deviceList.length > 0) {
         haveDevices = true;
@@ -2810,7 +2810,7 @@ describe('navigator.hid', () => {
       w.loadURL(serverUrl);
       const [,,,,, frameProcessId, frameRoutingId] = await emittedOnce(w.webContents, 'did-frame-navigate');
       const frame = webFrameMain.fromId(frameProcessId, frameRoutingId);
-      expect(frame).to.not.be.empty();
+      expect(!!frame).to.be.true();
       if (frame) {
         const grantedDevicesOnNewPage = await frame.executeJavaScript('navigator.hid.getDevices()');
         expect(grantedDevicesOnNewPage).to.be.empty();
@@ -2988,7 +2988,7 @@ describe('navigator.usb', () => {
     let haveDevices = false;
     let selectFired = false;
     w.webContents.session.on('select-usb-device', (event, details, callback) => {
-      expect(details.frame).to.have.ownProperty('frameTreeNodeId').that.is.a('number');
+      expect(details.frame).to.have.property('frameTreeNodeId').that.is.a('number');
       selectFired = true;
       if (details.deviceList.length > 0) {
         haveDevices = true;
@@ -3011,7 +3011,7 @@ describe('navigator.usb', () => {
       w.loadURL(serverUrl);
       const [,,,,, frameProcessId, frameRoutingId] = await emittedOnce(w.webContents, 'did-frame-navigate');
       const frame = webFrameMain.fromId(frameProcessId, frameRoutingId);
-      expect(frame).to.not.be.empty();
+      expect(!!frame).to.be.true();
       if (frame) {
         const grantedDevicesOnNewPage = await frame.executeJavaScript('navigator.usb.getDevices()');
         expect(grantedDevicesOnNewPage).to.be.empty();