From 5e25d23794a4b3b1bda35c79fcc1b2fdd787ad69 Mon Sep 17 00:00:00 2001
From: Shelley Vohr <shelley.vohr@gmail.com>
Date: Wed, 1 Mar 2023 11:35:06 +0100
Subject: [PATCH] fix: handle closing `webContents` in `BrowserView`s (#37420)

* fix: handle closing webContents in BrowserViews

* test: add window.close() test
---
 shell/browser/api/electron_api_browser_view.cc |  3 +++
 .../browser/api/electron_api_browser_window.cc |  8 ++++++--
 shell/browser/api/electron_api_web_contents.cc |  4 +---
 spec/api-browser-view-spec.ts                  | 18 ++++++++++++++++++
 4 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/shell/browser/api/electron_api_browser_view.cc b/shell/browser/api/electron_api_browser_view.cc
index 20ee7c756ef3..acfbcd4c62d1 100644
--- a/shell/browser/api/electron_api_browser_view.cc
+++ b/shell/browser/api/electron_api_browser_view.cc
@@ -126,6 +126,9 @@ BrowserView::~BrowserView() {
 }
 
 void BrowserView::WebContentsDestroyed() {
+  if (owner_window())
+    owner_window()->window()->RemoveDraggableRegionProvider(this);
+
   api_web_contents_ = nullptr;
   web_contents_.Reset();
   Unpin();
diff --git a/shell/browser/api/electron_api_browser_window.cc b/shell/browser/api/electron_api_browser_window.cc
index f1fd37e0c740..02a3aa0cdd2d 100644
--- a/shell/browser/api/electron_api_browser_window.cc
+++ b/shell/browser/api/electron_api_browser_window.cc
@@ -112,6 +112,7 @@ BrowserWindow::~BrowserWindow() {
     api_web_contents_->RemoveObserver(this);
     // Destroy the WebContents.
     OnCloseContents();
+    api_web_contents_->Destroy();
   }
 }
 
@@ -139,7 +140,6 @@ void BrowserWindow::WebContentsDestroyed() {
 
 void BrowserWindow::OnCloseContents() {
   BaseWindow::ResetBrowserViews();
-  api_web_contents_->Destroy();
 }
 
 void BrowserWindow::OnRendererResponsive(content::RenderProcessHost*) {
@@ -198,7 +198,11 @@ void BrowserWindow::OnCloseButtonClicked(bool* prevent_default) {
 
   // Trigger beforeunload events for associated BrowserViews.
   for (NativeBrowserView* view : window_->browser_views()) {
-    auto* vwc = view->GetInspectableWebContents()->GetWebContents();
+    auto* iwc = view->GetInspectableWebContents();
+    if (!iwc)
+      continue;
+
+    auto* vwc = iwc->GetWebContents();
     auto* api_web_contents = api::WebContents::From(vwc);
 
     // Required to make beforeunload handler work.
diff --git a/shell/browser/api/electron_api_web_contents.cc b/shell/browser/api/electron_api_web_contents.cc
index 4df972cd3f82..9acdae57be96 100644
--- a/shell/browser/api/electron_api_web_contents.cc
+++ b/shell/browser/api/electron_api_web_contents.cc
@@ -1223,9 +1223,7 @@ void WebContents::CloseContents(content::WebContents* source) {
   for (ExtendedWebContentsObserver& observer : observers_)
     observer.OnCloseContents();
 
-  // If there are observers, OnCloseContents will call Destroy()
-  if (observers_.empty())
-    Destroy();
+  Destroy();
 }
 
 void WebContents::ActivateContents(content::WebContents* source) {
diff --git a/spec/api-browser-view-spec.ts b/spec/api-browser-view-spec.ts
index 65b70e911b53..d485be567edf 100644
--- a/spec/api-browser-view-spec.ts
+++ b/spec/api-browser-view-spec.ts
@@ -345,6 +345,24 @@ describe('BrowserView module', () => {
       const [code] = await once(rc.process, 'exit');
       expect(code).to.equal(0);
     });
+
+    it('emits the destroyed event when webContents.close() is called', async () => {
+      view = new BrowserView();
+      w.setBrowserView(view);
+      await view.webContents.loadFile(path.join(fixtures, 'pages', 'a.html'));
+
+      view.webContents.close();
+      await once(view.webContents, 'destroyed');
+    });
+
+    it('emits the destroyed event when window.close() is called', async () => {
+      view = new BrowserView();
+      w.setBrowserView(view);
+      await view.webContents.loadFile(path.join(fixtures, 'pages', 'a.html'));
+
+      view.webContents.executeJavaScript('window.close()');
+      await once(view.webContents, 'destroyed');
+    });
   });
 
   describe('window.open()', () => {