fix: prevent destroyed view references from causing crashes (#25411)

Closes #21666.

This PR is fixing crashes caused by referencing and attempting to modify previously destroyed views.
Before, when a view was destroyed and then the contents were referenced for modification, the system would crash as undefined memory was accessed. This fix explicitly makes the pointer to the destroyed view's contents null, so that this will not happen.

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();
 }

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

@@ -0,0 +1,25 @@
+const { app, BrowserWindow, BrowserView } = require('electron');
+const { expect } = require('chai');
+
+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();
+  setTimeout(() => app.quit());
+});