diff --git a/lib/browser/api/browser-window.js b/lib/browser/api/browser-window.js index 4bda2972c128..3294cb28f584 100644 --- a/lib/browser/api/browser-window.js +++ b/lib/browser/api/browser-window.js @@ -1,7 +1,7 @@ 'use strict'; const electron = require('electron'); -const { WebContentsView, TopLevelWindow, deprecate } = electron; +const { TopLevelWindow, deprecate } = electron; const { BrowserWindow } = process.electronBinding('window'); Object.setPrototypeOf(BrowserWindow.prototype, TopLevelWindow.prototype); @@ -13,9 +13,6 @@ BrowserWindow.prototype._init = function () { // Avoid recursive require. const { app } = electron; - // Create WebContentsView. - this.setContentView(new WebContentsView(this.webContents)); - const nativeSetBounds = this.setBounds; this.setBounds = (bounds, ...opts) => { bounds = { diff --git a/shell/browser/api/electron_api_browser_window.cc b/shell/browser/api/electron_api_browser_window.cc index 966ab3f99392..bbc8ea563b72 100644 --- a/shell/browser/api/electron_api_browser_window.cc +++ b/shell/browser/api/electron_api_browser_window.cc @@ -12,12 +12,12 @@ #include "content/browser/web_contents/web_contents_impl.h" // nogncheck #include "content/public/browser/render_process_host.h" #include "content/public/browser/render_view_host.h" +#include "shell/browser/api/electron_api_web_contents_view.h" #include "shell/browser/browser.h" #include "shell/browser/unresponsive_suppressor.h" #include "shell/browser/web_contents_preferences.h" #include "shell/browser/window_list.h" #include "shell/common/color_util.h" -#include "shell/common/gin_converters/value_converter.h" #include "shell/common/gin_helper/constructor.h" #include "shell/common/gin_helper/dictionary.h" #include "shell/common/gin_helper/object_template_builder.h" @@ -32,8 +32,6 @@ namespace api { BrowserWindow::BrowserWindow(gin::Arguments* args, const gin_helper::Dictionary& options) : TopLevelWindow(args->isolate(), options), weak_factory_(this) { - gin::Handle web_contents; - // Use options.webPreferences in WebContents. v8::Isolate* isolate = args->isolate(); gin_helper::Dictionary web_preferences = @@ -60,23 +58,20 @@ BrowserWindow::BrowserWindow(gin::Arguments* args, web_preferences.Set(options::kShow, show); } - if (options.Get("webContents", &web_contents) && !web_contents.IsEmpty()) { - // Set webPreferences from options if using an existing webContents. - // These preferences will be used when the webContent launches new - // render processes. - auto* existing_preferences = - WebContentsPreferences::From(web_contents->web_contents()); - base::DictionaryValue web_preferences_dict; - if (gin::ConvertFromV8(isolate, web_preferences.GetHandle(), - &web_preferences_dict)) { - existing_preferences->Clear(); - existing_preferences->Merge(web_preferences_dict); - } - } else { - // Creates the WebContents used by BrowserWindow. - web_contents = WebContents::Create(isolate, web_preferences); + // Copy the webContents option to webPreferences. This is only used internally + // to implement nativeWindowOpen option. + if (options.Get("webContents", &value)) { + web_preferences.SetHidden("webContents", value); } + // Creates the WebContentsView. + gin::Handle web_contents_view = + WebContentsView::Create(isolate, web_preferences); + DCHECK(web_contents_view.get()); + + // Save a reference of the WebContents. + gin::Handle web_contents = + web_contents_view->GetWebContents(isolate); web_contents_.Reset(isolate, web_contents.ToV8()); api_web_contents_ = web_contents->GetWeakPtr(); api_web_contents_->AddObserver(this); @@ -95,6 +90,9 @@ BrowserWindow::BrowserWindow(gin::Arguments* args, InitWithArgs(args); + // Install the content view after TopLevelWindow's JS code is initialized. + SetContentView(gin::CreateHandle(isolate, web_contents_view.get())); + #if defined(OS_MACOSX) OverrideNSWindowContentView(web_contents->managed_web_contents()); #endif diff --git a/shell/browser/api/electron_api_web_contents_view.cc b/shell/browser/api/electron_api_web_contents_view.cc index 26ddf61e3cda..23e0821d87ee 100644 --- a/shell/browser/api/electron_api_web_contents_view.cc +++ b/shell/browser/api/electron_api_web_contents_view.cc @@ -4,52 +4,32 @@ #include "shell/browser/api/electron_api_web_contents_view.h" -#include "content/public/browser/web_contents_user_data.h" +#include "base/no_destructor.h" #include "shell/browser/api/electron_api_web_contents.h" #include "shell/browser/browser.h" #include "shell/browser/ui/inspectable_web_contents_view.h" +#include "shell/browser/web_contents_preferences.h" +#include "shell/common/gin_converters/value_converter.h" #include "shell/common/gin_helper/constructor.h" #include "shell/common/gin_helper/dictionary.h" +#include "shell/common/gin_helper/object_template_builder.h" #include "shell/common/node_includes.h" #if defined(OS_MACOSX) #include "shell/browser/ui/cocoa/delayed_native_view_host.h" #endif -namespace { - -// Used to indicate whether a WebContents already has a view. -class WebContentsViewRelay - : public content::WebContentsUserData { - public: - ~WebContentsViewRelay() override = default; - - private: - explicit WebContentsViewRelay(content::WebContents* contents) {} - friend class content::WebContentsUserData; - - electron::api::WebContentsView* view_ = nullptr; - - WEB_CONTENTS_USER_DATA_KEY_DECL(); - - DISALLOW_COPY_AND_ASSIGN(WebContentsViewRelay); -}; - -WEB_CONTENTS_USER_DATA_KEY_IMPL(WebContentsViewRelay) - -} // namespace - namespace electron { namespace api { WebContentsView::WebContentsView(v8::Isolate* isolate, - gin::Handle web_contents, - InspectableWebContents* iwc) + gin::Handle web_contents) #if defined(OS_MACOSX) - : View(new DelayedNativeViewHost(iwc->GetView()->GetNativeView())), + : View(new DelayedNativeViewHost( + web_contents->managed_web_contents()->GetView()->GetNativeView())), #else - : View(iwc->GetView()->GetView()), + : View(web_contents->managed_web_contents()->GetView()->GetView()), #endif web_contents_(isolate, web_contents->GetWrapper()), api_web_contents_(web_contents.get()) { @@ -59,7 +39,6 @@ WebContentsView::WebContentsView(v8::Isolate* isolate, // managed by InspectableWebContents. set_delete_view(false); #endif - WebContentsViewRelay::CreateForWebContents(web_contents->web_contents()); Observe(web_contents->web_contents()); } @@ -78,30 +57,66 @@ WebContentsView::~WebContentsView() { } } +gin::Handle WebContentsView::GetWebContents(v8::Isolate* isolate) { + return gin::CreateHandle(isolate, api_web_contents_); +} + void WebContentsView::WebContentsDestroyed() { api_web_contents_ = nullptr; web_contents_.Reset(); } +// static +gin::Handle WebContentsView::Create( + v8::Isolate* isolate, + const gin_helper::Dictionary& web_preferences) { + v8::Local context = isolate->GetCurrentContext(); + v8::Local arg = gin::ConvertToV8(isolate, web_preferences); + v8::Local obj; + if (GetConstructor(isolate)->NewInstance(context, 1, &arg).ToLocal(&obj)) { + gin::Handle web_contents_view; + if (gin::ConvertFromV8(isolate, obj, &web_contents_view)) + return web_contents_view; + } + return gin::Handle(); +} + +// static +v8::Local WebContentsView::GetConstructor(v8::Isolate* isolate) { + static base::NoDestructor> constructor; + if (constructor.get()->IsEmpty()) { + constructor->Reset( + isolate, gin_helper::CreateConstructor( + isolate, base::BindRepeating(&WebContentsView::New))); + } + return v8::Local::New(isolate, *constructor.get()); +} + // static gin_helper::WrappableBase* WebContentsView::New( gin_helper::Arguments* args, - gin::Handle web_contents) { - // Currently we only support InspectableWebContents, e.g. the WebContents - // created by users directly. To support devToolsWebContents we need to create - // a wrapper view. - if (!web_contents->managed_web_contents()) { - args->ThrowError("The WebContents must be created by user"); - return nullptr; - } - // Check if the WebContents has already been added to a view. - if (WebContentsViewRelay::FromWebContents(web_contents->web_contents())) { - args->ThrowError("The WebContents has already been added to a View"); - return nullptr; + const gin_helper::Dictionary& web_preferences) { + // Check if BrowserWindow has passend |webContents| option to us. + gin::Handle web_contents; + if (web_preferences.GetHidden("webContents", &web_contents) && + !web_contents.IsEmpty()) { + // Set webPreferences from options if using an existing webContents. + // These preferences will be used when the webContent launches new + // render processes. + auto* existing_preferences = + WebContentsPreferences::From(web_contents->web_contents()); + base::DictionaryValue web_preferences_dict; + if (gin::ConvertFromV8(args->isolate(), web_preferences.GetHandle(), + &web_preferences_dict)) { + existing_preferences->Clear(); + existing_preferences->Merge(web_preferences_dict); + } + } else { + // Create one if not. + web_contents = WebContents::Create(args->isolate(), web_preferences); } // Constructor call. - auto* view = new WebContentsView(args->isolate(), web_contents, - web_contents->managed_web_contents()); + auto* view = new WebContentsView(args->isolate(), web_contents); view->InitWithArgs(args); return view; } @@ -109,7 +124,11 @@ gin_helper::WrappableBase* WebContentsView::New( // static void WebContentsView::BuildPrototype( v8::Isolate* isolate, - v8::Local prototype) {} + v8::Local prototype) { + prototype->SetClassName(gin::StringToV8(isolate, "WebContentsView")); + gin_helper::ObjectTemplateBuilder(isolate, prototype->PrototypeTemplate()) + .SetProperty("webContents", &WebContentsView::GetWebContents); +} } // namespace api @@ -125,9 +144,7 @@ void Initialize(v8::Local exports, void* priv) { v8::Isolate* isolate = context->GetIsolate(); gin_helper::Dictionary dict(isolate, exports); - dict.Set("WebContentsView", - gin_helper::CreateConstructor( - isolate, base::BindRepeating(&WebContentsView::New))); + dict.Set("WebContentsView", WebContentsView::GetConstructor(isolate)); } } // namespace diff --git a/shell/browser/api/electron_api_web_contents_view.h b/shell/browser/api/electron_api_web_contents_view.h index ad43630eb1cc..26e2c6025623 100644 --- a/shell/browser/api/electron_api_web_contents_view.h +++ b/shell/browser/api/electron_api_web_contents_view.h @@ -8,6 +8,10 @@ #include "content/public/browser/web_contents_observer.h" #include "shell/browser/api/electron_api_view.h" +namespace gin_helper { +class Dictionary; +} + namespace electron { class InspectableWebContents; @@ -18,22 +22,34 @@ class WebContents; class WebContentsView : public View, public content::WebContentsObserver { public: - static gin_helper::WrappableBase* New(gin_helper::Arguments* args, - gin::Handle web_contents); + // Create a new instance of WebContentsView. + static gin::Handle Create( + v8::Isolate* isolate, + const gin_helper::Dictionary& web_preferences); + // Return the cached constructor function. + static v8::Local GetConstructor(v8::Isolate* isolate); + + // gin_helper::Wrappable static void BuildPrototype(v8::Isolate* isolate, v8::Local prototype); + // Public APIs. + gin::Handle GetWebContents(v8::Isolate* isolate); + protected: - WebContentsView(v8::Isolate* isolate, - gin::Handle web_contents, - InspectableWebContents* iwc); + // Takes an existing WebContents. + WebContentsView(v8::Isolate* isolate, gin::Handle web_contents); ~WebContentsView() override; // content::WebContentsObserver: void WebContentsDestroyed() override; private: + static gin_helper::WrappableBase* New( + gin_helper::Arguments* args, + const gin_helper::Dictionary& web_preferences); + // Keep a reference to v8 wrapper. v8::Global web_contents_; api::WebContents* api_web_contents_; diff --git a/spec-main/ambient.d.ts b/spec-main/ambient.d.ts index 625eff7e83a5..bd63f7b111a1 100644 --- a/spec-main/ambient.d.ts +++ b/spec-main/ambient.d.ts @@ -27,7 +27,7 @@ declare namespace Electron { } class View {} class WebContentsView { - constructor(webContents: WebContents) + constructor(options: BrowserWindowConstructorOptions) } namespace Main { diff --git a/spec-main/api-web-contents-view-spec.ts b/spec-main/api-web-contents-view-spec.ts index 0be0c7a671ca..6dcaca283a14 100644 --- a/spec-main/api-web-contents-view-spec.ts +++ b/spec-main/api-web-contents-view-spec.ts @@ -4,25 +4,15 @@ import * as path from 'path'; import { emittedOnce } from './events-helpers'; import { closeWindow } from './window-helpers'; -import { webContents, TopLevelWindow, WebContentsView } from 'electron/main'; +import { TopLevelWindow, WebContentsView } from 'electron/main'; describe('WebContentsView', () => { let w: TopLevelWindow; afterEach(() => closeWindow(w as any).then(() => { w = null as unknown as TopLevelWindow; })); it('can be used as content view', () => { - const web = (webContents as any).create({}); w = new TopLevelWindow({ show: false }); - w.setContentView(new WebContentsView(web)); - }); - - it('prevents adding same WebContents', () => { - const web = (webContents as any).create({}); - w = new TopLevelWindow({ show: false }); - w.setContentView(new WebContentsView(web)); - expect(() => { - w.setContentView(new WebContentsView(web)); - }).to.throw('The WebContents has already been added to a View'); + w.setContentView(new WebContentsView({})); }); describe('new WebContentsView()', () => { @@ -50,9 +40,8 @@ describe('WebContentsView', () => { } it('doesn\'t crash when GCed during allocation', (done) => { - const web = (webContents as any).create({}); // eslint-disable-next-line no-new - new WebContentsView(web); + new WebContentsView({}); setTimeout(() => { // NB. the crash we're testing for is the lack of a current `v8::Context` // when emitting an event in WebContents's destructor. V8 is inconsistent diff --git a/spec-main/fixtures/api/leak-exit-webcontentsview.js b/spec-main/fixtures/api/leak-exit-webcontentsview.js index cc98d941346b..3211bfd40b00 100644 --- a/spec-main/fixtures/api/leak-exit-webcontentsview.js +++ b/spec-main/fixtures/api/leak-exit-webcontentsview.js @@ -1,7 +1,6 @@ -const { WebContentsView, app, webContents } = require('electron'); +const { WebContentsView, app } = require('electron'); app.whenReady().then(function () { - const web = webContents.create({}); - new WebContentsView(web) // eslint-disable-line + new WebContentsView({}) // eslint-disable-line app.quit(); });