Skip to content

Commit 9bd0fc5

Browse files
nornagonnornagon
authored
refactor: ginify BrowserView (electron#23578)
1 parent 66d65a6 commit 9bd0fc5

File tree

12 files changed

+128
-165
lines changed

12 files changed

+128
-165
lines changed

‎docs/api/browser-view.md

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -28,25 +28,6 @@ view.webContents.loadURL('https://electronjs.org')
2828
* `options` Object (optional)
2929
* `webPreferences` Object (optional) - See [BrowserWindow](browser-window.md).
3030

31-
### Static Methods
32-
33-
#### `BrowserView.getAllViews()`
34-
35-
Returns `BrowserView[]` - An array of all opened BrowserViews.
36-
37-
#### `BrowserView.fromWebContents(webContents)`
38-
39-
* `webContents` [WebContents](web-contents.md)
40-
41-
Returns `BrowserView | null` - The BrowserView that owns the given `webContents`
42-
or `null` if the contents are not owned by a BrowserView.
43-
44-
#### `BrowserView.fromId(id)`
45-
46-
* `id` Integer
47-
48-
Returns `BrowserView` - The view with the given `id`.
49-
5031
### Instance Properties
5132

5233
Objects created with `new BrowserView` have the following properties:
@@ -55,10 +36,6 @@ Objects created with `new BrowserView` have the following properties:
5536

5637
A [`WebContents`](web-contents.md) object owned by this view.
5738

58-
#### `view.id` _Experimental_
59-
60-
A `Integer` representing the unique ID of the view.
61-
6239
### Instance Methods
6340

6441
Objects created with `new BrowserView` have the following instance methods:

‎lib/browser/api/browser-view.ts

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,3 @@
1-
import { EventEmitter } from 'events';
2-
31
const { BrowserView } = process._linkedBinding('electron_browser_browser_view');
42

5-
Object.setPrototypeOf(BrowserView.prototype, EventEmitter.prototype);
6-
7-
BrowserView.fromWebContents = (webContents: Electron.WebContents) => {
8-
for (const view of BrowserView.getAllViews()) {
9-
if (view.webContents.equal(webContents)) return view;
10-
}
11-
12-
return null;
13-
};
14-
153
export default BrowserView;

‎shell/browser/api/electron_api_base_window.cc

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -738,11 +738,11 @@ void BaseWindow::AddBrowserView(v8::Local<v8::Value> value) {
738738
gin::Handle<BrowserView> browser_view;
739739
if (value->IsObject() &&
740740
gin::ConvertFromV8(isolate(), value, &browser_view)) {
741-
auto get_that_view = browser_views_.find(browser_view->weak_map_id());
741+
auto get_that_view = browser_views_.find(browser_view->ID());
742742
if (get_that_view == browser_views_.end()) {
743743
window_->AddBrowserView(browser_view->view());
744744
browser_view->web_contents()->SetOwnerWindow(window_.get());
745-
browser_views_[browser_view->weak_map_id()].Reset(isolate(), value);
745+
browser_views_[browser_view->ID()].Reset(isolate(), value);
746746
}
747747
}
748748
}
@@ -751,7 +751,7 @@ void BaseWindow::RemoveBrowserView(v8::Local<v8::Value> value) {
751751
gin::Handle<BrowserView> browser_view;
752752
if (value->IsObject() &&
753753
gin::ConvertFromV8(isolate(), value, &browser_view)) {
754-
auto get_that_view = browser_views_.find(browser_view->weak_map_id());
754+
auto get_that_view = browser_views_.find(browser_view->ID());
755755
if (get_that_view != browser_views_.end()) {
756756
window_->RemoveBrowserView(browser_view->view());
757757
browser_view->web_contents()->SetOwnerWindow(nullptr);

‎shell/browser/api/electron_api_browser_view.cc

Lines changed: 31 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,24 @@ struct Converter<electron::AutoResizeFlags> {
5252

5353
} // namespace gin
5454

55+
namespace {
56+
57+
int32_t GetNextId() {
58+
static int32_t next_id = 1;
59+
return next_id++;
60+
}
61+
62+
} // namespace
63+
5564
namespace electron {
5665

5766
namespace api {
5867

68+
gin::WrapperInfo BrowserView::kWrapperInfo = {gin::kEmbedderNativeGin};
69+
5970
BrowserView::BrowserView(gin::Arguments* args,
60-
const gin_helper::Dictionary& options) {
71+
const gin_helper::Dictionary& options)
72+
: id_(GetNextId()) {
6173
v8::Isolate* isolate = args->isolate();
6274
gin_helper::Dictionary web_preferences =
6375
gin::Dictionary::CreateEmpty(isolate);
@@ -72,8 +84,6 @@ BrowserView::BrowserView(gin::Arguments* args,
7284

7385
view_.reset(
7486
NativeBrowserView::Create(api_web_contents_->managed_web_contents()));
75-
76-
InitWithArgs(args);
7787
}
7888

7989
BrowserView::~BrowserView() {
@@ -87,24 +97,24 @@ BrowserView::~BrowserView() {
8797
void BrowserView::WebContentsDestroyed() {
8898
api_web_contents_ = nullptr;
8999
web_contents_.Reset();
100+
Unpin();
90101
}
91102

92103
// static
93-
gin_helper::WrappableBase* BrowserView::New(gin_helper::ErrorThrower thrower,
94-
gin::Arguments* args) {
104+
gin::Handle<BrowserView> BrowserView::New(gin_helper::ErrorThrower thrower,
105+
gin::Arguments* args) {
95106
if (!Browser::Get()->is_ready()) {
96107
thrower.ThrowError("Cannot create BrowserView before app is ready");
97-
return nullptr;
108+
return gin::Handle<BrowserView>();
98109
}
99110

100111
gin::Dictionary options = gin::Dictionary::CreateEmpty(args->isolate());
101112
args->GetNext(&options);
102113

103-
return new BrowserView(args, options);
104-
}
105-
106-
int32_t BrowserView::ID() const {
107-
return weak_map_id();
114+
auto handle =
115+
gin::CreateHandle(args->isolate(), new BrowserView(args, options));
116+
handle->Pin(args->isolate());
117+
return handle;
108118
}
109119

110120
void BrowserView::SetAutoResize(AutoResizeFlags flags) {
@@ -123,26 +133,25 @@ void BrowserView::SetBackgroundColor(const std::string& color_name) {
123133
view_->SetBackgroundColor(ParseHexColor(color_name));
124134
}
125135

126-
v8::Local<v8::Value> BrowserView::GetWebContents() {
136+
v8::Local<v8::Value> BrowserView::GetWebContents(v8::Isolate* isolate) {
127137
if (web_contents_.IsEmpty()) {
128-
return v8::Null(isolate());
138+
return v8::Null(isolate);
129139
}
130140

131-
return v8::Local<v8::Value>::New(isolate(), web_contents_);
141+
return v8::Local<v8::Value>::New(isolate, web_contents_);
132142
}
133143

134144
// static
135-
void BrowserView::BuildPrototype(v8::Isolate* isolate,
136-
v8::Local<v8::FunctionTemplate> prototype) {
137-
prototype->SetClassName(gin::StringToV8(isolate, "BrowserView"));
138-
gin_helper::Destroyable::MakeDestroyable(isolate, prototype);
139-
gin_helper::ObjectTemplateBuilder(isolate, prototype->PrototypeTemplate())
145+
v8::Local<v8::ObjectTemplate> BrowserView::FillObjectTemplate(
146+
v8::Isolate* isolate,
147+
v8::Local<v8::ObjectTemplate> templ) {
148+
return gin::ObjectTemplateBuilder(isolate, "BrowserView", templ)
140149
.SetMethod("setAutoResize", &BrowserView::SetAutoResize)
141150
.SetMethod("setBounds", &BrowserView::SetBounds)
142151
.SetMethod("getBounds", &BrowserView::GetBounds)
143152
.SetMethod("setBackgroundColor", &BrowserView::SetBackgroundColor)
144153
.SetProperty("webContents", &BrowserView::GetWebContents)
145-
.SetProperty("id", &BrowserView::ID);
154+
.Build();
146155
}
147156

148157
} // namespace api
@@ -158,16 +167,9 @@ void Initialize(v8::Local<v8::Object> exports,
158167
v8::Local<v8::Context> context,
159168
void* priv) {
160169
v8::Isolate* isolate = context->GetIsolate();
161-
BrowserView::SetConstructor(isolate, base::BindRepeating(&BrowserView::New));
162-
163-
gin_helper::Dictionary browser_view(isolate,
164-
BrowserView::GetConstructor(isolate)
165-
->GetFunction(context)
166-
.ToLocalChecked());
167-
browser_view.SetMethod("fromId", &BrowserView::FromWeakMapID);
168-
browser_view.SetMethod("getAllViews", &BrowserView::GetAll);
170+
169171
gin_helper::Dictionary dict(isolate, exports);
170-
dict.Set("BrowserView", browser_view);
172+
dict.Set("BrowserView", BrowserView::GetConstructor(context));
171173
}
172174

173175
} // namespace

‎shell/browser/api/electron_api_browser_view.h

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,11 @@
1010

1111
#include "content/public/browser/web_contents_observer.h"
1212
#include "gin/handle.h"
13+
#include "gin/wrappable.h"
1314
#include "shell/browser/native_browser_view.h"
15+
#include "shell/common/gin_helper/constructible.h"
1416
#include "shell/common/gin_helper/error_thrower.h"
15-
#include "shell/common/gin_helper/trackable_object.h"
17+
#include "shell/common/gin_helper/pinnable.h"
1618

1719
namespace gfx {
1820
class Rect;
@@ -30,19 +32,25 @@ namespace api {
3032

3133
class WebContents;
3234

33-
class BrowserView : public gin_helper::TrackableObject<BrowserView>,
35+
class BrowserView : public gin::Wrappable<BrowserView>,
36+
public gin_helper::Constructible<BrowserView>,
37+
public gin_helper::Pinnable<BrowserView>,
3438
public content::WebContentsObserver {
3539
public:
36-
static gin_helper::WrappableBase* New(gin_helper::ErrorThrower thrower,
37-
gin::Arguments* args);
40+
// gin_helper::Constructible
41+
static gin::Handle<BrowserView> New(gin_helper::ErrorThrower thrower,
42+
gin::Arguments* args);
43+
static v8::Local<v8::ObjectTemplate> FillObjectTemplate(
44+
v8::Isolate*,
45+
v8::Local<v8::ObjectTemplate>);
3846

39-
static void BuildPrototype(v8::Isolate* isolate,
40-
v8::Local<v8::FunctionTemplate> prototype);
47+
// gin::Wrappable
48+
static gin::WrapperInfo kWrapperInfo;
4149

4250
WebContents* web_contents() const { return api_web_contents_; }
4351
NativeBrowserView* view() const { return view_.get(); }
4452

45-
int32_t ID() const;
53+
int32_t ID() const { return id_; }
4654

4755
protected:
4856
BrowserView(gin::Arguments* args, const gin_helper::Dictionary& options);
@@ -56,13 +64,15 @@ class BrowserView : public gin_helper::TrackableObject<BrowserView>,
5664
void SetBounds(const gfx::Rect& bounds);
5765
gfx::Rect GetBounds();
5866
void SetBackgroundColor(const std::string& color_name);
59-
v8::Local<v8::Value> GetWebContents();
67+
v8::Local<v8::Value> GetWebContents(v8::Isolate*);
6068

6169
v8::Global<v8::Value> web_contents_;
6270
class WebContents* api_web_contents_ = nullptr;
6371

6472
std::unique_ptr<NativeBrowserView> view_;
6573

74+
int32_t id_;
75+
6676
DISALLOW_COPY_AND_ASSIGN(BrowserView);
6777
};
6878

‎shell/browser/api/electron_api_browser_window.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,10 @@ void BrowserWindow::OnCloseButtonClicked(bool* prevent_default) {
276276
}
277277

278278
void BrowserWindow::OnWindowClosed() {
279+
// We need to reset the browser views before we call Cleanup, because the
280+
// browser views are child views of the main web contents view, which will be
281+
// deleted by Cleanup.
282+
BaseWindow::ResetBrowserViews();
279283
Cleanup();
280284
// See BaseWindow::OnWindowClosed on why calling InvalidateWeakPtrs.
281285
weak_factory_.InvalidateWeakPtrs();

‎shell/common/gin_helper/constructible.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@
55
#ifndef SHELL_COMMON_GIN_HELPER_CONSTRUCTIBLE_H_
66
#define SHELL_COMMON_GIN_HELPER_CONSTRUCTIBLE_H_
77

8+
#include "gin/per_isolate_data.h"
89
#include "gin/wrappable.h"
10+
#include "shell/browser/event_emitter_mixin.h"
911
#include "shell/common/gin_helper/function_template_extensions.h"
1012

1113
namespace gin_helper {

0 commit comments

Comments
 (0)