fix: prevent destroyed view references from causing crashes

shell/browser/api/electron_api_top_level_window.cc

@@ -741,8 +741,10 @@ void TopLevelWindow::AddBrowserView(v8::Local<v8::Value> value) {
       gin::ConvertFromV8(isolate(), value, &browser_view)) {
     auto get_that_view = browser_views_.find(browser_view->weak_map_id());
     if (get_that_view == browser_views_.end()) {
-      window_->AddBrowserView(browser_view->view());
-      browser_view->web_contents()->SetOwnerWindow(window_.get());
+      if (browser_view->web_contents()) {
+        window_->AddBrowserView(browser_view->view());
+        browser_view->web_contents()->SetOwnerWindow(window_.get());
+      }
       browser_views_[browser_view->weak_map_id()].Reset(isolate(), value);
     }
   }
@@ -754,9 +756,10 @@ void TopLevelWindow::RemoveBrowserView(v8::Local<v8::Value> value) {
       gin::ConvertFromV8(isolate(), value, &browser_view)) {
     auto get_that_view = browser_views_.find(browser_view->weak_map_id());
     if (get_that_view != browser_views_.end()) {
-      window_->RemoveBrowserView(browser_view->view());
-      browser_view->web_contents()->SetOwnerWindow(nullptr);
-
+      if (browser_view->web_contents()) {
+        window_->RemoveBrowserView(browser_view->view());
+        browser_view->web_contents()->SetOwnerWindow(nullptr);
+      }
       (*get_that_view).second.Reset(isolate(), value);
       browser_views_.erase(get_that_view);
     }
@@ -1054,8 +1057,10 @@ void TopLevelWindow::ResetBrowserViews() {
                            v8::Local<v8::Value>::New(isolate(), item.second),
                            &browser_view) &&
         !browser_view.IsEmpty()) {
-      window_->RemoveBrowserView(browser_view->view());
-      browser_view->web_contents()->SetOwnerWindow(nullptr);
+      if (browser_view->web_contents()) {
+        window_->RemoveBrowserView(browser_view->view());
+        browser_view->web_contents()->SetOwnerWindow(nullptr);
+      }
     }
 
     item.second.Reset();

shell/browser/api/electron_api_web_contents.cc

@@ -698,7 +698,8 @@ WebContents::~WebContents() {
     } else {
       // Destroy WebContents asynchronously unless app is shutting down,
       // because destroy() might be called inside WebContents's event handler.
-      DestroyWebContents(!IsGuest() /* async */);
+      bool is_browser_view = type_ == Type::BROWSER_VIEW;
+      DestroyWebContents(!(IsGuest() || is_browser_view) /* async */);
       // The WebContentsDestroyed will not be called automatically because we
       // destroy the webContents in the next tick. So we have to manually
       // call it here to make sure "destroyed" event is emitted.

shell/browser/native_browser_view.cc

@@ -13,16 +13,26 @@ namespace electron {
 
 NativeBrowserView::NativeBrowserView(
     InspectableWebContents* inspectable_web_contents)
-    : inspectable_web_contents_(inspectable_web_contents) {}
+    : inspectable_web_contents_(inspectable_web_contents) {
+  Observe(inspectable_web_contents_->GetWebContents());
+}
 
 NativeBrowserView::~NativeBrowserView() = default;
 
 InspectableWebContentsView* NativeBrowserView::GetInspectableWebContentsView() {
+  if (!inspectable_web_contents_)
+    return nullptr;
   return inspectable_web_contents_->GetView();
 }
 
 content::WebContents* NativeBrowserView::GetWebContents() {
+  if (!inspectable_web_contents_)
+    return nullptr;
   return inspectable_web_contents_->GetWebContents();
 }
 
+void NativeBrowserView::WebContentsDestroyed() {
+  inspectable_web_contents_ = nullptr;
+}
+
 }  // namespace electron

shell/browser/native_browser_view.h

@@ -9,6 +9,7 @@
 
 #include "base/macros.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/browser/web_contents_observer.h"
 #include "third_party/skia/include/core/SkColor.h"
 
 namespace gfx {
@@ -27,9 +28,9 @@ enum AutoResizeFlags {
 class InspectableWebContents;
 class InspectableWebContentsView;
 
-class NativeBrowserView {
+class NativeBrowserView : public content::WebContentsObserver {
  public:
-  virtual ~NativeBrowserView();
+  ~NativeBrowserView() override;
 
   static NativeBrowserView* Create(
       InspectableWebContents* inspectable_web_contents);
@@ -52,6 +53,8 @@ class NativeBrowserView {
 
  protected:
   explicit NativeBrowserView(InspectableWebContents* inspectable_web_contents);
+  // content::WebContentsObserver:
+  void WebContentsDestroyed() override;
 
   InspectableWebContents* inspectable_web_contents_;
 

shell/browser/native_browser_view_mac.mm

@@ -162,8 +162,10 @@ - (BOOL)mouseDownCanMoveWindow {
 NativeBrowserViewMac::NativeBrowserViewMac(
     InspectableWebContents* inspectable_web_contents)
     : NativeBrowserView(inspectable_web_contents) {
-  auto* view =
-      GetInspectableWebContentsView()->GetNativeView().GetNativeNSView();
+  auto* iwc_view = GetInspectableWebContentsView();
+  if (!iwc_view)
+    return;
+  auto* view = iwc_view->GetNativeView().GetNativeNSView();
   view.autoresizingMask = kDefaultAutoResizingMask;
 }
 
@@ -186,14 +188,18 @@ - (BOOL)mouseDownCanMoveWindow {
         NSViewMaxYMargin | NSViewMinYMargin | NSViewHeightSizable;
   }
 
-  auto* view =
-      GetInspectableWebContentsView()->GetNativeView().GetNativeNSView();
+  auto* iwc_view = GetInspectableWebContentsView();
+  if (!iwc_view)
+    return;
+  auto* view = iwc_view->GetNativeView().GetNativeNSView();
   view.autoresizingMask = autoresizing_mask;
 }
 
 void NativeBrowserViewMac::SetBounds(const gfx::Rect& bounds) {
-  auto* view =
-      GetInspectableWebContentsView()->GetNativeView().GetNativeNSView();
+  auto* iwc_view = GetInspectableWebContentsView();
+  if (!iwc_view)
+    return;
+  auto* view = iwc_view->GetNativeView().GetNativeNSView();
   auto* superview = view.superview;
   const auto superview_height = superview ? superview.frame.size.height : 0;
   view.frame =
@@ -202,8 +208,10 @@ - (BOOL)mouseDownCanMoveWindow {
 }
 
 gfx::Rect NativeBrowserViewMac::GetBounds() {
-  NSView* view =
-      GetInspectableWebContentsView()->GetNativeView().GetNativeNSView();
+  auto* iwc_view = GetInspectableWebContentsView();
+  if (!iwc_view)
+    return gfx::Rect();
+  NSView* view = iwc_view->GetNativeView().GetNativeNSView();
   const int superview_height =
       (view.superview) ? view.superview.frame.size.height : 0;
   return gfx::Rect(
@@ -213,17 +221,22 @@ - (BOOL)mouseDownCanMoveWindow {
 }
 
 void NativeBrowserViewMac::SetBackgroundColor(SkColor color) {
-  auto* view =
-      GetInspectableWebContentsView()->GetNativeView().GetNativeNSView();
+  auto* iwc_view = GetInspectableWebContentsView();
+  if (!iwc_view)
+    return;
+  auto* view = iwc_view->GetNativeView().GetNativeNSView();
   view.wantsLayer = YES;
   view.layer.backgroundColor = skia::CGColorCreateFromSkColor(color);
 }
 
 void NativeBrowserViewMac::UpdateDraggableRegions(
     const std::vector<gfx::Rect>& drag_exclude_rects) {
+  auto* iwc_view = GetInspectableWebContentsView();
+  auto* web_contents = GetWebContents();
+  if (!(iwc_view && web_contents))
+    return;
   NSView* web_view = GetWebContents()->GetNativeView().GetNativeNSView();
-  NSView* inspectable_view =
-      GetInspectableWebContentsView()->GetNativeView().GetNativeNSView();
+  NSView* inspectable_view = iwc_view->GetNativeView().GetNativeNSView();
   NSView* window_content_view = inspectable_view.superview;
   const auto window_content_view_height = NSHeight(window_content_view.bounds);
 

shell/browser/native_browser_view_views.cc

@@ -26,7 +26,10 @@ void NativeBrowserViewViews::SetAutoResizeProportions(
     const gfx::Size& window_size) {
   if ((auto_resize_flags_ & AutoResizeFlags::kAutoResizeHorizontal) &&
       !auto_horizontal_proportion_set_) {
-    auto* view = GetInspectableWebContentsView()->GetView();
+    auto* iwc_view = GetInspectableWebContentsView();
+    if (iwc_view)
+      return;
+    auto* view = iwc_view->GetView();
     auto view_bounds = view->bounds();
     auto_horizontal_proportion_width_ =
         static_cast<float>(window_size.width()) /
@@ -37,7 +40,10 @@ void NativeBrowserViewViews::SetAutoResizeProportions(
   }
   if ((auto_resize_flags_ & AutoResizeFlags::kAutoResizeVertical) &&
       !auto_vertical_proportion_set_) {
-    auto* view = GetInspectableWebContentsView()->GetView();
+    auto* iwc_view = GetInspectableWebContentsView();
+    if (iwc_view)
+      return;
+    auto* view = iwc_view->GetView();
     auto view_bounds = view->bounds();
     auto_vertical_proportion_height_ =
         static_cast<float>(window_size.height()) /
@@ -51,7 +57,10 @@ void NativeBrowserViewViews::SetAutoResizeProportions(
 void NativeBrowserViewViews::AutoResize(const gfx::Rect& new_window,
                                         int width_delta,
                                         int height_delta) {
-  auto* view = GetInspectableWebContentsView()->GetView();
+  auto* iwc_view = GetInspectableWebContentsView();
+  if (iwc_view)
+    return;
+  auto* view = iwc_view->GetView();
   const auto flags = GetAutoResizeFlags();
   if (!(flags & kAutoResizeWidth)) {
     width_delta = 0;
@@ -92,17 +101,26 @@ void NativeBrowserViewViews::ResetAutoResizeProportions() {
 }
 
 void NativeBrowserViewViews::SetBounds(const gfx::Rect& bounds) {
-  auto* view = GetInspectableWebContentsView()->GetView();
+  auto* iwc_view = GetInspectableWebContentsView();
+  if (!iwc_view)
+    return;
+  auto* view = iwc_view->GetView();
   view->SetBoundsRect(bounds);
   ResetAutoResizeProportions();
 }
 
 gfx::Rect NativeBrowserViewViews::GetBounds() {
-  return GetInspectableWebContentsView()->GetView()->bounds();
+  auto* iwc_view = GetInspectableWebContentsView();
+  if (!iwc_view)
+    return gfx::Rect();
+  return iwc_view->GetView()->bounds();
 }
 
 void NativeBrowserViewViews::SetBackgroundColor(SkColor color) {
-  auto* view = GetInspectableWebContentsView()->GetView();
+  auto* iwc_view = GetInspectableWebContentsView();
+  if (!iwc_view)
+    return;
+  auto* view = iwc_view->GetView();
   view->SetBackground(views::CreateSolidBackground(color));
   view->SchedulePaint();
 }

shell/browser/native_window_mac.mm

@@ -1247,8 +1247,11 @@ void ViewDidMoveToSuperview(NSView* self, SEL _cmd) {
   }
 
   add_browser_view(view);
-  auto* native_view =
-      view->GetInspectableWebContentsView()->GetNativeView().GetNativeNSView();
+  auto* iwc_view = view->GetInspectableWebContentsView();
+  if (!iwc_view)
+    return;
+
+  auto* native_view = iwc_view->GetNativeView().GetNativeNSView();
   [[window_ contentView] addSubview:native_view
                          positioned:NSWindowAbove
                          relativeTo:nil];
@@ -1266,8 +1269,11 @@ void ViewDidMoveToSuperview(NSView* self, SEL _cmd) {
     return;
   }
 
-  [view->GetInspectableWebContentsView()->GetNativeView().GetNativeNSView()
-      removeFromSuperview];
+  auto* iwc_view = view->GetInspectableWebContentsView();
+  if (!iwc_view)
+    return;
+
+  [iwc_view->GetNativeView().GetNativeNSView() removeFromSuperview];
   remove_browser_view(view);
 
   [CATransaction commit];

shell/browser/native_window_views.cc

@@ -1065,8 +1065,10 @@ void NativeWindowViews::AddBrowserView(NativeBrowserView* view) {
 
   add_browser_view(view);
 
-  content_view()->AddChildView(
-      view->GetInspectableWebContentsView()->GetView());
+  auto* iwc_view = view->GetInspectableWebContentsView();
+  if (!iwc_view)
+    return;
+  content_view()->AddChildView(iwc_view->GetView());
 }
 
 void NativeWindowViews::RemoveBrowserView(NativeBrowserView* view) {
@@ -1077,8 +1079,11 @@ void NativeWindowViews::RemoveBrowserView(NativeBrowserView* view) {
     return;
   }
 
-  content_view()->RemoveChildView(
-      view->GetInspectableWebContentsView()->GetView());
+  auto* iwc_view = view->GetInspectableWebContentsView();
+  if (!iwc_view)
+    return;
+
+  content_view()->RemoveChildView(iwc_view->GetView());
   remove_browser_view(view);
 }
 

spec-main/fixtures/crash-cases/api-browser-destroy/index.js

@@ -0,0 +1,27 @@
+const { app, BrowserWindow, BrowserView } = require('electron');
+const { expect } = require('chai');
+const assert = require('assert');
+
+function createWindow () {
+  // Create the browser window.
+  const mainWindow = new BrowserWindow({
+    width: 800,
+    height: 600,
+    webPreferences: {
+      nodeIntegration: true
+    }
+  });
+  const view = new BrowserView();
+  mainWindow.addBrowserView(view);
+  view.webContents.destroy();
+
+  view.setBounds({ x: 0, y: 0, width: 0, height: 0 });
+  const bounds = view.getBounds();
+  expect(bounds).to.deep.equal({ x: 0, y: 0, width: 0, height: 0 });
+  view.setBackgroundColor('#56cc5b10');
+}
+
+app.on('ready', () => {
+  createWindow();
+  app.quit();
+});