fix: handle backporting prevent destroyed view

(cont.) references from causing crashes Referenced PR: #25411
electron · Sep 23, 2020 · a21990e · a21990e
1 parent 1d5eb77
commit a21990e
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 13 deletions.
diff --git a/shell/browser/native_window_mac.mm b/shell/browser/native_window_mac.mm
@@ -1247,8 +1247,9 @@ 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 +1267,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];

diff --git a/shell/browser/native_window_views.cc b/shell/browser/native_window_views.cc
@@ -1065,8 +1065,11 @@ 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 +1080,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);
 }
 

diff --git a/spec-main/fixtures/crash-cases/api-browser-destroy/index.js b/spec-main/fixtures/crash-cases/api-browser-destroy/index.js
@@ -1,5 +1,6 @@
 const { app, BrowserWindow, BrowserView } = require('electron');
 const { expect } = require('chai');
+const assert = require('assert');
 
 function createWindow () {
   // Create the browser window.
@@ -10,16 +11,21 @@ function createWindow () {
       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');
+
+  setTimeout(() => {
+    assert.strictEqual(view.webContents.isDestroyed, true, 'webContents has not been destroyed yet');
+    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');
+  }, 15000);
 }
 
 app.on('ready', () => {
   createWindow();
-  setTimeout(() => app.quit());
+  app.quit();
 });