fix: make BrowserView aware of owning window (#31842)

Co-authored-by: Shelley Vohr <[email protected]>

shell/browser/api/electron_api_base_window.cc

@@ -768,18 +768,16 @@ void BaseWindow::AddBrowserView(v8::Local<v8::Value> value) {
       gin::ConvertFromV8(isolate(), value, &browser_view)) {
     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());
+      // If we're reparenting a BrowserView, ensure that it's detached from
+      // its previous owner window.
+      auto* owner_window = browser_view->owner_window();
+      if (owner_window && owner_window != window_.get()) {
+        owner_window->RemoveBrowserView(browser_view->view());
+        browser_view->SetOwnerWindow(nullptr);
       }
+
+      window_->AddBrowserView(browser_view->view());
+      browser_view->SetOwnerWindow(window_.get());
       browser_views_[browser_view->ID()].Reset(isolate(), value);
     }
   }
@@ -791,10 +789,8 @@ void BaseWindow::RemoveBrowserView(v8::Local<v8::Value> value) {
       gin::ConvertFromV8(isolate(), value, &browser_view)) {
     auto get_that_view = browser_views_.find(browser_view->ID());
     if (get_that_view != browser_views_.end()) {
-      if (browser_view->web_contents()) {
-        window_->RemoveBrowserView(browser_view->view());
-        browser_view->web_contents()->SetOwnerWindow(nullptr);
-      }
+      window_->RemoveBrowserView(browser_view->view());
+      browser_view->SetOwnerWindow(nullptr);
       (*get_that_view).second.Reset(isolate(), value);
       browser_views_.erase(get_that_view);
     }
@@ -806,9 +802,7 @@ void BaseWindow::SetTopBrowserView(v8::Local<v8::Value> value,
   gin::Handle<BrowserView> browser_view;
   if (value->IsObject() &&
       gin::ConvertFromV8(isolate(), value, &browser_view)) {
-    if (!browser_view->web_contents())
-      return;
-    auto* owner_window = browser_view->web_contents()->owner_window();
+    auto* owner_window = browser_view->owner_window();
     auto get_that_view = browser_views_.find(browser_view->ID());
     if (get_that_view == browser_views_.end() ||
         (owner_window && owner_window != window_.get())) {
@@ -1137,12 +1131,10 @@ void BaseWindow::ResetBrowserViews() {
         !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()) {
-        auto* owner_window = browser_view->web_contents()->owner_window();
-        if (owner_window && owner_window == window_.get()) {
-          browser_view->web_contents()->SetOwnerWindow(nullptr);
-          owner_window->RemoveBrowserView(browser_view->view());
-        }
+      auto* owner_window = browser_view->owner_window();
+      if (owner_window && owner_window == window_.get()) {
+        browser_view->SetOwnerWindow(nullptr);
+        owner_window->RemoveBrowserView(browser_view->view());
       }
     }
 

shell/browser/api/electron_api_browser_view.cc

@@ -99,10 +99,18 @@ BrowserView::BrowserView(gin::Arguments* args,
       NativeBrowserView::Create(api_web_contents_->inspectable_web_contents()));
 }
 
+void BrowserView::SetOwnerWindow(NativeWindow* window) {
+  // Ensure WebContents and BrowserView owner windows are in sync.
+  if (web_contents())
+    web_contents()->SetOwnerWindow(window);
+
+  owner_window_ = window ? window->GetWeakPtr() : nullptr;
+}
+
 BrowserView::~BrowserView() {
-  if (api_web_contents_) {  // destroy() called without closing WebContents
-    api_web_contents_->RemoveObserver(this);
-    api_web_contents_->Destroy();
+  if (web_contents()) {  // destroy() called without closing WebContents
+    web_contents()->RemoveObserver(this);
+    web_contents()->Destroy();
   }
 }
 

shell/browser/api/electron_api_browser_view.h

@@ -14,6 +14,7 @@
 #include "gin/wrappable.h"
 #include "shell/browser/extended_web_contents_observer.h"
 #include "shell/browser/native_browser_view.h"
+#include "shell/browser/native_window.h"
 #include "shell/common/api/api.mojom.h"
 #include "shell/common/gin_helper/constructible.h"
 #include "shell/common/gin_helper/error_thrower.h"
@@ -52,6 +53,10 @@ class BrowserView : public gin::Wrappable<BrowserView>,
   WebContents* web_contents() const { return api_web_contents_; }
   NativeBrowserView* view() const { return view_.get(); }
 
+  NativeWindow* owner_window() const { return owner_window_.get(); }
+
+  void SetOwnerWindow(NativeWindow* window);
+
   int32_t ID() const { return id_; }
 
  protected:
@@ -76,6 +81,7 @@ class BrowserView : public gin::Wrappable<BrowserView>,
   class WebContents* api_web_contents_ = nullptr;
 
   std::unique_ptr<NativeBrowserView> view_;
+  base::WeakPtr<NativeWindow> owner_window_;
 
   int32_t id_;