Skip to content

Commit ca94730

Browse files
zcbenzzcbenz
authored
chore: make WebContentsView take webPreferences as parameter (electron#22997)
* chore: add WebContentsView.webContents helper * chore: no need to handle webContents option * chore: Create WebContentsView in C++ * chore: make WebContentsView accept web_preferences * fix: nativeWindowOpen still passes WebContents to BrowserWindow * chore: no more need of WebContentsViewRelay * test: WebContentsView now takes options * fix: avoid creating 2 constructors
1 parent de89336 commit ca94730

File tree

7 files changed

+108
-92
lines changed

7 files changed

+108
-92
lines changed

‎lib/browser/api/browser-window.js

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
'use strict';
22

33
const electron = require('electron');
4-
const { WebContentsView, TopLevelWindow, deprecate } = electron;
4+
const { TopLevelWindow, deprecate } = electron;
55
const { BrowserWindow } = process.electronBinding('window');
66

77
Object.setPrototypeOf(BrowserWindow.prototype, TopLevelWindow.prototype);
@@ -13,9 +13,6 @@ BrowserWindow.prototype._init = function () {
1313
// Avoid recursive require.
1414
const { app } = electron;
1515

16-
// Create WebContentsView.
17-
this.setContentView(new WebContentsView(this.webContents));
18-
1916
const nativeSetBounds = this.setBounds;
2017
this.setBounds = (bounds, ...opts) => {
2118
bounds = {

‎shell/browser/api/electron_api_browser_window.cc

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,12 @@
1212
#include "content/browser/web_contents/web_contents_impl.h" // nogncheck
1313
#include "content/public/browser/render_process_host.h"
1414
#include "content/public/browser/render_view_host.h"
15+
#include "shell/browser/api/electron_api_web_contents_view.h"
1516
#include "shell/browser/browser.h"
1617
#include "shell/browser/unresponsive_suppressor.h"
1718
#include "shell/browser/web_contents_preferences.h"
1819
#include "shell/browser/window_list.h"
1920
#include "shell/common/color_util.h"
20-
#include "shell/common/gin_converters/value_converter.h"
2121
#include "shell/common/gin_helper/constructor.h"
2222
#include "shell/common/gin_helper/dictionary.h"
2323
#include "shell/common/gin_helper/object_template_builder.h"
@@ -32,8 +32,6 @@ namespace api {
3232
BrowserWindow::BrowserWindow(gin::Arguments* args,
3333
const gin_helper::Dictionary& options)
3434
: TopLevelWindow(args->isolate(), options), weak_factory_(this) {
35-
gin::Handle<class WebContents> web_contents;
36-
3735
// Use options.webPreferences in WebContents.
3836
v8::Isolate* isolate = args->isolate();
3937
gin_helper::Dictionary web_preferences =
@@ -60,23 +58,20 @@ BrowserWindow::BrowserWindow(gin::Arguments* args,
6058
web_preferences.Set(options::kShow, show);
6159
}
6260

63-
if (options.Get("webContents", &web_contents) && !web_contents.IsEmpty()) {
64-
// Set webPreferences from options if using an existing webContents.
65-
// These preferences will be used when the webContent launches new
66-
// render processes.
67-
auto* existing_preferences =
68-
WebContentsPreferences::From(web_contents->web_contents());
69-
base::DictionaryValue web_preferences_dict;
70-
if (gin::ConvertFromV8(isolate, web_preferences.GetHandle(),
71-
&web_preferences_dict)) {
72-
existing_preferences->Clear();
73-
existing_preferences->Merge(web_preferences_dict);
74-
}
75-
} else {
76-
// Creates the WebContents used by BrowserWindow.
77-
web_contents = WebContents::Create(isolate, web_preferences);
61+
// Copy the webContents option to webPreferences. This is only used internally
62+
// to implement nativeWindowOpen option.
63+
if (options.Get("webContents", &value)) {
64+
web_preferences.SetHidden("webContents", value);
7865
}
7966

67+
// Creates the WebContentsView.
68+
gin::Handle<WebContentsView> web_contents_view =
69+
WebContentsView::Create(isolate, web_preferences);
70+
DCHECK(web_contents_view.get());
71+
72+
// Save a reference of the WebContents.
73+
gin::Handle<WebContents> web_contents =
74+
web_contents_view->GetWebContents(isolate);
8075
web_contents_.Reset(isolate, web_contents.ToV8());
8176
api_web_contents_ = web_contents->GetWeakPtr();
8277
api_web_contents_->AddObserver(this);
@@ -95,6 +90,9 @@ BrowserWindow::BrowserWindow(gin::Arguments* args,
9590

9691
InitWithArgs(args);
9792

93+
// Install the content view after TopLevelWindow's JS code is initialized.
94+
SetContentView(gin::CreateHandle<View>(isolate, web_contents_view.get()));
95+
9896
#if defined(OS_MACOSX)
9997
OverrideNSWindowContentView(web_contents->managed_web_contents());
10098
#endif

‎shell/browser/api/electron_api_web_contents_view.cc

Lines changed: 64 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -4,52 +4,32 @@
44

55
#include "shell/browser/api/electron_api_web_contents_view.h"
66

7-
#include "content/public/browser/web_contents_user_data.h"
7+
#include "base/no_destructor.h"
88
#include "shell/browser/api/electron_api_web_contents.h"
99
#include "shell/browser/browser.h"
1010
#include "shell/browser/ui/inspectable_web_contents_view.h"
11+
#include "shell/browser/web_contents_preferences.h"
12+
#include "shell/common/gin_converters/value_converter.h"
1113
#include "shell/common/gin_helper/constructor.h"
1214
#include "shell/common/gin_helper/dictionary.h"
15+
#include "shell/common/gin_helper/object_template_builder.h"
1316
#include "shell/common/node_includes.h"
1417

1518
#if defined(OS_MACOSX)
1619
#include "shell/browser/ui/cocoa/delayed_native_view_host.h"
1720
#endif
1821

19-
namespace {
20-
21-
// Used to indicate whether a WebContents already has a view.
22-
class WebContentsViewRelay
23-
: public content::WebContentsUserData<WebContentsViewRelay> {
24-
public:
25-
~WebContentsViewRelay() override = default;
26-
27-
private:
28-
explicit WebContentsViewRelay(content::WebContents* contents) {}
29-
friend class content::WebContentsUserData<WebContentsViewRelay>;
30-
31-
electron::api::WebContentsView* view_ = nullptr;
32-
33-
WEB_CONTENTS_USER_DATA_KEY_DECL();
34-
35-
DISALLOW_COPY_AND_ASSIGN(WebContentsViewRelay);
36-
};
37-
38-
WEB_CONTENTS_USER_DATA_KEY_IMPL(WebContentsViewRelay)
39-
40-
} // namespace
41-
4222
namespace electron {
4323

4424
namespace api {
4525

4626
WebContentsView::WebContentsView(v8::Isolate* isolate,
47-
gin::Handle<WebContents> web_contents,
48-
InspectableWebContents* iwc)
27+
gin::Handle<WebContents> web_contents)
4928
#if defined(OS_MACOSX)
50-
: View(new DelayedNativeViewHost(iwc->GetView()->GetNativeView())),
29+
: View(new DelayedNativeViewHost(
30+
web_contents->managed_web_contents()->GetView()->GetNativeView())),
5131
#else
52-
: View(iwc->GetView()->GetView()),
32+
: View(web_contents->managed_web_contents()->GetView()->GetView()),
5333
#endif
5434
web_contents_(isolate, web_contents->GetWrapper()),
5535
api_web_contents_(web_contents.get()) {
@@ -59,7 +39,6 @@ WebContentsView::WebContentsView(v8::Isolate* isolate,
5939
// managed by InspectableWebContents.
6040
set_delete_view(false);
6141
#endif
62-
WebContentsViewRelay::CreateForWebContents(web_contents->web_contents());
6342
Observe(web_contents->web_contents());
6443
}
6544

@@ -78,38 +57,78 @@ WebContentsView::~WebContentsView() {
7857
}
7958
}
8059

60+
gin::Handle<WebContents> WebContentsView::GetWebContents(v8::Isolate* isolate) {
61+
return gin::CreateHandle(isolate, api_web_contents_);
62+
}
63+
8164
void WebContentsView::WebContentsDestroyed() {
8265
api_web_contents_ = nullptr;
8366
web_contents_.Reset();
8467
}
8568

69+
// static
70+
gin::Handle<WebContentsView> WebContentsView::Create(
71+
v8::Isolate* isolate,
72+
const gin_helper::Dictionary& web_preferences) {
73+
v8::Local<v8::Context> context = isolate->GetCurrentContext();
74+
v8::Local<v8::Value> arg = gin::ConvertToV8(isolate, web_preferences);
75+
v8::Local<v8::Object> obj;
76+
if (GetConstructor(isolate)->NewInstance(context, 1, &arg).ToLocal(&obj)) {
77+
gin::Handle<WebContentsView> web_contents_view;
78+
if (gin::ConvertFromV8(isolate, obj, &web_contents_view))
79+
return web_contents_view;
80+
}
81+
return gin::Handle<WebContentsView>();
82+
}
83+
84+
// static
85+
v8::Local<v8::Function> WebContentsView::GetConstructor(v8::Isolate* isolate) {
86+
static base::NoDestructor<v8::Global<v8::Function>> constructor;
87+
if (constructor.get()->IsEmpty()) {
88+
constructor->Reset(
89+
isolate, gin_helper::CreateConstructor<WebContentsView>(
90+
isolate, base::BindRepeating(&WebContentsView::New)));
91+
}
92+
return v8::Local<v8::Function>::New(isolate, *constructor.get());
93+
}
94+
8695
// static
8796
gin_helper::WrappableBase* WebContentsView::New(
8897
gin_helper::Arguments* args,
89-
gin::Handle<WebContents> web_contents) {
90-
// Currently we only support InspectableWebContents, e.g. the WebContents
91-
// created by users directly. To support devToolsWebContents we need to create
92-
// a wrapper view.
93-
if (!web_contents->managed_web_contents()) {
94-
args->ThrowError("The WebContents must be created by user");
95-
return nullptr;
96-
}
97-
// Check if the WebContents has already been added to a view.
98-
if (WebContentsViewRelay::FromWebContents(web_contents->web_contents())) {
99-
args->ThrowError("The WebContents has already been added to a View");
100-
return nullptr;
98+
const gin_helper::Dictionary& web_preferences) {
99+
// Check if BrowserWindow has passend |webContents| option to us.
100+
gin::Handle<WebContents> web_contents;
101+
if (web_preferences.GetHidden("webContents", &web_contents) &&
102+
!web_contents.IsEmpty()) {
103+
// Set webPreferences from options if using an existing webContents.
104+
// These preferences will be used when the webContent launches new
105+
// render processes.
106+
auto* existing_preferences =
107+
WebContentsPreferences::From(web_contents->web_contents());
108+
base::DictionaryValue web_preferences_dict;
109+
if (gin::ConvertFromV8(args->isolate(), web_preferences.GetHandle(),
110+
&web_preferences_dict)) {
111+
existing_preferences->Clear();
112+
existing_preferences->Merge(web_preferences_dict);
113+
}
114+
} else {
115+
// Create one if not.
116+
web_contents = WebContents::Create(args->isolate(), web_preferences);
101117
}
102118
// Constructor call.
103-
auto* view = new WebContentsView(args->isolate(), web_contents,
104-
web_contents->managed_web_contents());
119+
auto* view = new WebContentsView(args->isolate(), web_contents);
105120
view->InitWithArgs(args);
106121
return view;
107122
}
108123

109124
// static
110125
void WebContentsView::BuildPrototype(
111126
v8::Isolate* isolate,
112-
v8::Local<v8::FunctionTemplate> prototype) {}
127+
v8::Local<v8::FunctionTemplate> prototype) {
128+
prototype->SetClassName(gin::StringToV8(isolate, "WebContentsView"));
129+
gin_helper::ObjectTemplateBuilder(isolate, prototype->PrototypeTemplate())
130+
.SetProperty("webContents", &WebContentsView::GetWebContents);
131+
}
113132

114133
} // namespace api
115134

@@ -125,9 +144,7 @@ void Initialize(v8::Local<v8::Object> exports,
125144
void* priv) {
126145
v8::Isolate* isolate = context->GetIsolate();
127146
gin_helper::Dictionary dict(isolate, exports);
128-
dict.Set("WebContentsView",
129-
gin_helper::CreateConstructor<WebContentsView>(
130-
isolate, base::BindRepeating(&WebContentsView::New)));
147+
dict.Set("WebContentsView", WebContentsView::GetConstructor(isolate));
131148
}
132149

133150
} // namespace

‎shell/browser/api/electron_api_web_contents_view.h

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@
88
#include "content/public/browser/web_contents_observer.h"
99
#include "shell/browser/api/electron_api_view.h"
1010

11+
namespace gin_helper {
12+
class Dictionary;
13+
}
14+
1115
namespace electron {
1216

1317
class InspectableWebContents;
@@ -18,22 +22,34 @@ class WebContents;
1822

1923
class WebContentsView : public View, public content::WebContentsObserver {
2024
public:
21-
static gin_helper::WrappableBase* New(gin_helper::Arguments* args,
22-
gin::Handle<WebContents> web_contents);
25+
// Create a new instance of WebContentsView.
26+
static gin::Handle<WebContentsView> Create(
27+
v8::Isolate* isolate,
28+
const gin_helper::Dictionary& web_preferences);
29+
30+
// Return the cached constructor function.
31+
static v8::Local<v8::Function> GetConstructor(v8::Isolate* isolate);
2332

33+
// gin_helper::Wrappable
2434
static void BuildPrototype(v8::Isolate* isolate,
2535
v8::Local<v8::FunctionTemplate> prototype);
2636

37+
// Public APIs.
38+
gin::Handle<WebContents> GetWebContents(v8::Isolate* isolate);
39+
2740
protected:
28-
WebContentsView(v8::Isolate* isolate,
29-
gin::Handle<WebContents> web_contents,
30-
InspectableWebContents* iwc);
41+
// Takes an existing WebContents.
42+
WebContentsView(v8::Isolate* isolate, gin::Handle<WebContents> web_contents);
3143
~WebContentsView() override;
3244

3345
// content::WebContentsObserver:
3446
void WebContentsDestroyed() override;
3547

3648
private:
49+
static gin_helper::WrappableBase* New(
50+
gin_helper::Arguments* args,
51+
const gin_helper::Dictionary& web_preferences);
52+
3753
// Keep a reference to v8 wrapper.
3854
v8::Global<v8::Value> web_contents_;
3955
api::WebContents* api_web_contents_;

‎spec-main/ambient.d.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ declare namespace Electron {
2727
}
2828
class View {}
2929
class WebContentsView {
30-
constructor(webContents: WebContents)
30+
constructor(options: BrowserWindowConstructorOptions)
3131
}
3232

3333
namespace Main {

‎spec-main/api-web-contents-view-spec.ts

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,25 +4,15 @@ import * as path from 'path';
44
import { emittedOnce } from './events-helpers';
55
import { closeWindow } from './window-helpers';
66

7-
import { webContents, TopLevelWindow, WebContentsView } from 'electron/main';
7+
import { TopLevelWindow, WebContentsView } from 'electron/main';
88

99
describe('WebContentsView', () => {
1010
let w: TopLevelWindow;
1111
afterEach(() => closeWindow(w as any).then(() => { w = null as unknown as TopLevelWindow; }));
1212

1313
it('can be used as content view', () => {
14-
const web = (webContents as any).create({});
1514
w = new TopLevelWindow({ show: false });
16-
w.setContentView(new WebContentsView(web));
17-
});
18-
19-
it('prevents adding same WebContents', () => {
20-
const web = (webContents as any).create({});
21-
w = new TopLevelWindow({ show: false });
22-
w.setContentView(new WebContentsView(web));
23-
expect(() => {
24-
w.setContentView(new WebContentsView(web));
25-
}).to.throw('The WebContents has already been added to a View');
15+
w.setContentView(new WebContentsView({}));
2616
});
2717

2818
describe('new WebContentsView()', () => {
@@ -50,9 +40,8 @@ describe('WebContentsView', () => {
5040
}
5141

5242
it('doesn\'t crash when GCed during allocation', (done) => {
53-
const web = (webContents as any).create({});
5443
// eslint-disable-next-line no-new
55-
new WebContentsView(web);
44+
new WebContentsView({});
5645
setTimeout(() => {
5746
// NB. the crash we're testing for is the lack of a current `v8::Context`
5847
// when emitting an event in WebContents's destructor. V8 is inconsistent

‎spec-main/fixtures/api/leak-exit-webcontentsview.js

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
1-
const { WebContentsView, app, webContents } = require('electron');
1+
const { WebContentsView, app } = require('electron');
22
app.whenReady().then(function () {
3-
const web = webContents.create({});
4-
new WebContentsView(web) // eslint-disable-line
3+
new WebContentsView({}) // eslint-disable-line
54

65
app.quit();
76
});

0 commit comments

Comments
 (0)