fix: webview crash when removing in close event

electron · Jul 5, 2023 · d446610 · d446610
1 parent 52fe76c
commit d446610
Show file tree

Hide file tree

Showing 5 changed files with 83 additions and 2 deletions.
diff --git a/shell/browser/api/electron_api_browser_window.cc b/shell/browser/api/electron_api_browser_window.cc
@@ -112,7 +112,6 @@ BrowserWindow::~BrowserWindow() {
     api_web_contents_->RemoveObserver(this);
     // Destroy the WebContents.
     OnCloseContents();
-    api_web_contents_->Destroy();
   }
 }
 

diff --git a/shell/browser/api/electron_api_web_contents.cc b/shell/browser/api/electron_api_web_contents.cc
@@ -1297,7 +1297,9 @@ void WebContents::CloseContents(content::WebContents* source) {
   for (ExtendedWebContentsObserver& observer : observers_)
     observer.OnCloseContents();
 
-  Destroy();
+  // This is handled by the embedder frame.
+  if (!IsGuest())
+    Destroy();
 }
 
 void WebContents::ActivateContents(content::WebContents* source) {

diff --git a/spec/fixtures/crash-cases/webview-remove-on-wc-close/index.html b/spec/fixtures/crash-cases/webview-remove-on-wc-close/index.html
@@ -0,0 +1,32 @@
+<!DOCTYPE html>
+<html lang="en">
+
+<head>
+  <meta charset="UTF-8">
+  <meta name="viewport" content="width=device-width, initial-scale=1.0">
+  <title>Document</title>
+  <style>
+    .webview {
+      border: 1px solid black;
+    }
+  </style>
+</head>
+
+<body>
+  <button class="close-btn">close webview</button>
+  <webview class="webview" src="./webview.html"></webview>
+  <script>
+    const close = document.querySelector('.close-btn')
+    const webview = document.querySelector('.webview')
+
+    webview.addEventListener('close', () => {
+      webview.parentNode.removeChild(webview)
+    })
+
+    close.addEventListener('click', () => {
+      webview.executeJavaScript('window.close()', true)
+    })
+  </script>
+</body>
+
+</html>
diff --git a/spec/fixtures/crash-cases/webview-remove-on-wc-close/index.js b/spec/fixtures/crash-cases/webview-remove-on-wc-close/index.js
@@ -0,0 +1,29 @@
+const { app, BrowserWindow } = require('electron');
+
+app.whenReady().then(() => {
+  const win = new BrowserWindow({
+    webPreferences: {
+      webviewTag: true
+    }
+  });
+
+  win.loadFile('index.html');
+
+  win.webContents.on('did-attach-webview', (event, contents) => {
+    contents.on('render-process-gone', () => {
+      process.exit(1);
+    });
+
+    contents.on('destroyed', () => {
+      process.exit(0);
+    });
+
+    contents.on('did-finish-load', () => {
+      win.webContents.executeJavaScript('document.querySelector(\'.close-btn\').click()');
+    });
+
+    contents.on('will-prevent-unload', event => {
+      event.preventDefault();
+    });
+  });
+});
diff --git a/spec/fixtures/crash-cases/webview-remove-on-wc-close/webview.html b/spec/fixtures/crash-cases/webview-remove-on-wc-close/webview.html
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<html lang="en">
+
+<head>
+  <meta charset="UTF-8">
+  <meta name="viewport" content="width=device-width, initial-scale=1.0">
+  <title>Document</title>
+</head>
+
+<body>
+  <h1>webview page</h1>
+  <script>
+    window.addEventListener('beforeunload', event => {
+      event.returnValue = 'test'
+    })
+  </script>
+</body>
+
+</html>