Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 4 commits into from
Feb 6, 2023
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
850ec53
refactor: move gin_helper::Constructible methods to prototype
nornagon Feb 1, 2023
876ce17
make some tests more lax about prototype vs object
nornagon Feb 1, 2023
9d479f4
more
nornagon Feb 2, 2023
7d8b58b
update header
nornagon Feb 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 3 additions & 4 deletions shell/browser/api/electron_api_browser_view.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 1 addition & 3 deletions shell/browser/api/electron_api_browser_view.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 3 additions & 4 deletions shell/browser/api/electron_api_menu.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 1 addition & 3 deletions shell/browser/api/electron_api_menu.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 3 additions & 4 deletions shell/browser/api/electron_api_notification.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 1 addition & 3 deletions shell/browser/api/electron_api_notification.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 3 additions & 4 deletions shell/browser/api/electron_api_tray.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,10 +395,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)
Expand Down
4 changes: 1 addition & 3 deletions shell/browser/api/electron_api_tray.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 3 additions & 4 deletions shell/browser/api/electron_api_web_contents.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3985,9 +3985,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";
Expand All @@ -3999,7 +3998,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",
Expand Down
4 changes: 1 addition & 3 deletions shell/browser/api/electron_api_web_contents.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
7 changes: 3 additions & 4 deletions shell/browser/api/electron_api_web_frame_main.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 1 addition & 3 deletions shell/browser/api/electron_api_web_frame_main.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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_; }
Expand Down
7 changes: 3 additions & 4 deletions shell/common/gin_helper/constructible.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>);
// }
Expand Down Expand Up @@ -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());
nornagon marked this conversation as resolved.
Show resolved Hide resolved
data->SetObjectTemplate(wrapper_info, constructor->InstanceTemplate());
data->SetFunctionTemplate(wrapper_info, constructor);
}
return constructor->GetFunction(context).ToLocalChecked();
Expand Down
12 changes: 6 additions & 6 deletions spec/api-web-frame-main-spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});

Expand Down
8 changes: 4 additions & 4 deletions spec/chromium-spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2786,7 +2786,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;
Expand All @@ -2809,7 +2809,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();
Expand Down Expand Up @@ -2987,7 +2987,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;
Expand All @@ -3010,7 +3010,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();
Expand Down