From c0b48658d8abf83d7bd6d49ba0bbd92257ca4984 Mon Sep 17 00:00:00 2001
From: Shelley Vohr <shelley.vohr@gmail.com>
Date: Thu, 7 Jan 2021 11:43:46 -0800
Subject: [PATCH] fix: handle BrowserView reparenting (#27219)

---
 shell/browser/api/electron_api_base_window.cc | 17 +++++++++++++--
 spec-main/api-browser-view-spec.ts            | 21 +++++++++++++++++++
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/shell/browser/api/electron_api_base_window.cc b/shell/browser/api/electron_api_base_window.cc
index 24ed27c088b4e..9e636903dab35 100644
--- a/shell/browser/api/electron_api_base_window.cc
+++ b/shell/browser/api/electron_api_base_window.cc
@@ -755,6 +755,14 @@ void BaseWindow::AddBrowserView(v8::Local<v8::Value> value) {
     auto get_that_view = browser_views_.find(browser_view->ID());
     if (get_that_view == browser_views_.end()) {
       if (browser_view->web_contents()) {
+        // If we're reparenting a BrowserView, ensure that it's detached from
+        // its previous owner window.
+        auto* owner_window = browser_view->web_contents()->owner_window();
+        if (owner_window && owner_window != window_.get()) {
+          owner_window->RemoveBrowserView(browser_view->view());
+          browser_view->web_contents()->SetOwnerWindow(nullptr);
+        }
+
         window_->AddBrowserView(browser_view->view());
         browser_view->web_contents()->SetOwnerWindow(window_.get());
       }
@@ -1067,9 +1075,14 @@ void BaseWindow::ResetBrowserViews() {
                            v8::Local<v8::Value>::New(isolate(), item.second),
                            &browser_view) &&
         !browser_view.IsEmpty()) {
+      // There's a chance that the BrowserView may have been reparented - only
+      // reset if the owner window is *this* window.
       if (browser_view->web_contents()) {
-        browser_view->web_contents()->SetOwnerWindow(nullptr);
-        window_->RemoveBrowserView(browser_view->view());
+        auto* owner_window = browser_view->web_contents()->owner_window();
+        if (owner_window == window_.get()) {
+          browser_view->web_contents()->SetOwnerWindow(nullptr);
+          owner_window->RemoveBrowserView(browser_view->view());
+        }
       }
     }
 
diff --git a/spec-main/api-browser-view-spec.ts b/spec-main/api-browser-view-spec.ts
index 597050e97cb74..f6bdaacab3a8f 100644
--- a/spec-main/api-browser-view-spec.ts
+++ b/spec-main/api-browser-view-spec.ts
@@ -158,6 +158,27 @@ describe('BrowserView module', () => {
         w.addBrowserView(view1);
       }).to.not.throw();
     });
+
+    it('can handle BrowserView reparenting', async () => {
+      view = new BrowserView();
+
+      w.addBrowserView(view);
+      view.webContents.loadURL('about:blank');
+      await emittedOnce(view.webContents, 'did-finish-load');
+
+      const w2 = new BrowserWindow({ show: false });
+      w2.addBrowserView(view);
+
+      w.close();
+
+      view.webContents.loadURL(`file://${fixtures}/pages/blank.html`);
+      await emittedOnce(view.webContents, 'did-finish-load');
+
+      // Clean up - the afterEach hook assumes the webContents on w is still alive.
+      w = new BrowserWindow({ show: false });
+      w2.close();
+      w2.destroy();
+    });
   });
 
   describe('BrowserWindow.removeBrowserView()', () => {