From 4667f4589a58fa712bbb59839b690a079549a8ee Mon Sep 17 00:00:00 2001 From: Jeremy Rose Date: Tue, 30 Aug 2022 14:22:58 -0700 Subject: [PATCH 1/3] feat: add webContents.close() --- docs/api/web-contents.md | 8 +++ .../api/electron_api_browser_window.cc | 2 +- .../browser/api/electron_api_web_contents.cc | 16 +++++ shell/browser/api/electron_api_web_contents.h | 1 + spec/api-web-contents-spec.ts | 66 +++++++++++++++++++ 5 files changed, 92 insertions(+), 1 deletion(-) diff --git a/docs/api/web-contents.md b/docs/api/web-contents.md index 2cf6e17263088..c475c652ad190 100644 --- a/docs/api/web-contents.md +++ b/docs/api/web-contents.md @@ -926,6 +926,14 @@ Returns `string` - The title of the current web page. Returns `boolean` - Whether the web page is destroyed. +#### `contents.close([opts])` + +* `opts` Object (optional) + * `waitForBeforeUnload` boolean - if true, fire the `beforeunload` event + before closing the page. + +Closes the page, as if the web content had called `window.close()`. + #### `contents.focus()` Focuses the web page. diff --git a/shell/browser/api/electron_api_browser_window.cc b/shell/browser/api/electron_api_browser_window.cc index 3d1ec969123c8..ac4b1ad364d39 100644 --- a/shell/browser/api/electron_api_browser_window.cc +++ b/shell/browser/api/electron_api_browser_window.cc @@ -135,6 +135,7 @@ BrowserWindow::~BrowserWindow() { api_web_contents_->RemoveObserver(this); // Destroy the WebContents. OnCloseContents(); + api_web_contents_->Destroy(); } } @@ -205,7 +206,6 @@ void BrowserWindow::WebContentsDestroyed() { void BrowserWindow::OnCloseContents() { BaseWindow::ResetBrowserViews(); - api_web_contents_->Destroy(); } void BrowserWindow::OnRendererResponsive(content::RenderProcessHost*) { diff --git a/shell/browser/api/electron_api_web_contents.cc b/shell/browser/api/electron_api_web_contents.cc index 9411bc0926926..8c4882587634f 100644 --- a/shell/browser/api/electron_api_web_contents.cc +++ b/shell/browser/api/electron_api_web_contents.cc @@ -1009,6 +1009,19 @@ void WebContents::Destroy() { } } +void WebContents::Close(absl::optional options) { + bool dispatch_beforeunload = false; + if (options) + options->Get("waitForBeforeUnload", &dispatch_beforeunload); + if (dispatch_beforeunload && + web_contents()->NeedToFireBeforeUnloadOrUnloadEvents()) { + NotifyUserActivation(); + web_contents()->DispatchBeforeUnload(false /* auto_cancel */); + } else { + web_contents()->Close(); + } +} + bool WebContents::DidAddMessageToConsole( content::WebContents* source, blink::mojom::ConsoleMessageLevel level, @@ -1197,6 +1210,8 @@ void WebContents::CloseContents(content::WebContents* source) { for (ExtendedWebContentsObserver& observer : observers_) observer.OnCloseContents(); + + Destroy(); } void WebContents::ActivateContents(content::WebContents* source) { @@ -3923,6 +3938,7 @@ v8::Local WebContents::FillObjectTemplate( // destroyable. return gin_helper::ObjectTemplateBuilder(isolate, templ) .SetMethod("destroy", &WebContents::Destroy) + .SetMethod("close", &WebContents::Close) .SetMethod("getBackgroundThrottling", &WebContents::GetBackgroundThrottling) .SetMethod("setBackgroundThrottling", diff --git a/shell/browser/api/electron_api_web_contents.h b/shell/browser/api/electron_api_web_contents.h index 7ccc525f56096..faf65fdeb5f3d 100644 --- a/shell/browser/api/electron_api_web_contents.h +++ b/shell/browser/api/electron_api_web_contents.h @@ -152,6 +152,7 @@ class WebContents : public ExclusiveAccessContext, const char* GetTypeName() override; void Destroy(); + void Close(absl::optional options); base::WeakPtr GetWeakPtr() { return weak_factory_.GetWeakPtr(); } bool GetBackgroundThrottling() const; diff --git a/spec/api-web-contents-spec.ts b/spec/api-web-contents-spec.ts index 3ceb081c76c9b..29f89ad68577c 100644 --- a/spec/api-web-contents-spec.ts +++ b/spec/api-web-contents-spec.ts @@ -2132,4 +2132,70 @@ describe('webContents module', () => { expect(params.y).to.be.a('number'); }); }); + + describe('close() method', () => { + afterEach(closeAllWindows); + + it('closes when close() is called', async () => { + const w = (webContents as any).create() as WebContents; + const destroyed = emittedOnce(w, 'destroyed'); + w.close(); + await destroyed; + expect(w.isDestroyed()).to.be.true(); + }); + + it('closes when close() is called after loading a page', async () => { + const w = (webContents as any).create() as WebContents; + await w.loadURL('about:blank'); + const destroyed = emittedOnce(w, 'destroyed'); + w.close(); + await destroyed; + expect(w.isDestroyed()).to.be.true(); + }); + + it('can be GCed before loading a page', async () => { + const v8Util = process._linkedBinding('electron_common_v8_util'); + let registry: FinalizationRegistry | null = null; + const cleanedUp = new Promise(resolve => { + registry = new FinalizationRegistry(resolve as any); + }); + (() => { + const w = (webContents as any).create() as WebContents; + registry!.register(w, 42); + })(); + const i = setInterval(() => v8Util.requestGarbageCollectionForTesting(), 100); + defer(() => clearInterval(i)); + expect(await cleanedUp).to.equal(42); + }); + + it('causes its parent browserwindow to be closed', async () => { + const w = new BrowserWindow({ show: false }); + await w.loadURL('about:blank'); + const closed = emittedOnce(w, 'closed'); + w.webContents.close(); + await closed; + expect(w.isDestroyed()).to.be.true(); + }); + + it('ignores beforeunload if waitForBeforeUnload not specified', async () => { + const w = (webContents as any).create() as WebContents; + await w.loadURL('about:blank'); + await w.executeJavaScript('window.onbeforeunload = () => "hello"; null'); + w.on('will-prevent-unload', () => { throw new Error('unexpected will-prevent-unload'); }); + const destroyed = emittedOnce(w, 'destroyed'); + w.close(); + await destroyed; + expect(w.isDestroyed()).to.be.true(); + }); + + it('runs beforeunload if waitForBeforeUnload is specified', async () => { + const w = (webContents as any).create() as WebContents; + await w.loadURL('about:blank'); + await w.executeJavaScript('window.onbeforeunload = () => "hello"; null'); + const willPreventUnload = emittedOnce(w, 'will-prevent-unload'); + w.close({ waitForBeforeUnload: true }); + await willPreventUnload; + expect(w.isDestroyed()).to.be.false(); + }); + }); }); From be09450559df3c0a408fdb9270422f2173236c9e Mon Sep 17 00:00:00 2001 From: Jeremy Rose Date: Wed, 7 Sep 2022 10:59:55 -0700 Subject: [PATCH 2/3] update docs, add test for beforeunload override --- docs/api/web-contents.md | 4 +++- spec/api-web-contents-spec.ts | 11 +++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/docs/api/web-contents.md b/docs/api/web-contents.md index c475c652ad190..e76fb30a8d935 100644 --- a/docs/api/web-contents.md +++ b/docs/api/web-contents.md @@ -930,7 +930,9 @@ Returns `boolean` - Whether the web page is destroyed. * `opts` Object (optional) * `waitForBeforeUnload` boolean - if true, fire the `beforeunload` event - before closing the page. + before closing the page. If the page prevents the unload, the WebContents + will not be closed. The [`will-prevent-unload`](#event-will-prevent-unload) + will be fired if the page requests prevention of unload. Closes the page, as if the web content had called `window.close()`. diff --git a/spec/api-web-contents-spec.ts b/spec/api-web-contents-spec.ts index 29f89ad68577c..6c3c928a31ed1 100644 --- a/spec/api-web-contents-spec.ts +++ b/spec/api-web-contents-spec.ts @@ -2197,5 +2197,16 @@ describe('webContents module', () => { await willPreventUnload; expect(w.isDestroyed()).to.be.false(); }); + + it('overriding beforeunload prevention results in webcontents close', async () => { + const w = (webContents as any).create() as WebContents; + await w.loadURL('about:blank'); + await w.executeJavaScript('window.onbeforeunload = () => "hello"; null'); + w.once('will-prevent-unload', e => e.preventDefault()); + const destroyed = emittedOnce(w, 'destroyed'); + w.close({ waitForBeforeUnload: true }); + await destroyed; + expect(w.isDestroyed()).to.be.true(); + }); }); }); From 267c332231eedbb538036773dfd6618d3401d190 Mon Sep 17 00:00:00 2001 From: Jeremy Rose Date: Wed, 7 Sep 2022 11:23:20 -0700 Subject: [PATCH 3/3] Update web-contents.md --- docs/api/web-contents.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/api/web-contents.md b/docs/api/web-contents.md index e76fb30a8d935..b7b9081e1a77c 100644 --- a/docs/api/web-contents.md +++ b/docs/api/web-contents.md @@ -936,6 +936,11 @@ Returns `boolean` - Whether the web page is destroyed. Closes the page, as if the web content had called `window.close()`. +If the page is successfully closed (i.e. the unload is not prevented by the +page, or `waitForBeforeUnload` is false or unspecified), the WebContents will +be destroyed and no longer usable. The [`destroyed`](#event-destroyed) event +will be emitted. + #### `contents.focus()` Focuses the web page.