Browse Source

fix: check web_contents() for destroyed WebContents (#27965)

Co-authored-by: Cheng Zhao <[email protected]>
trop[bot] 4 years ago
parent
commit
e5eafc8ee9

+ 5 - 4
shell/browser/api/electron_api_web_contents.cc

@@ -877,7 +877,7 @@ content::WebContents* WebContents::OpenURLFromTab(
     return nullptr;
   }
 
-  if (!weak_this)
+  if (!weak_this || !web_contents())
     return nullptr;
 
   return CommonWebContentsDelegate::OpenURLFromTab(source, params);
@@ -1139,7 +1139,7 @@ void WebContents::RenderProcessGone(base::TerminationStatus status) {
   Emit("crashed", status == base::TERMINATION_STATUS_PROCESS_WAS_KILLED);
 
   // User might destroy WebContents in the crashed event.
-  if (!weak_this)
+  if (!weak_this || !web_contents())
     return;
 
   v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
@@ -1209,7 +1209,7 @@ void WebContents::DidFinishLoad(content::RenderFrameHost* render_frame_host,
   // ⚠️WARNING!⚠️
   // Emit() triggers JS which can call destroy() on |this|. It's not safe to
   // assume that |this| points to valid memory at this point.
-  if (is_main_frame && weak_this)
+  if (is_main_frame && weak_this && web_contents())
     Emit("did-finish-load");
 }
 
@@ -1580,6 +1580,7 @@ void WebContents::WebContentsDestroyed() {
   // also do not want any method to be used, so just mark as destroyed here.
   MarkDestroyed();
 
+  Observe(nullptr);  // this->web_contents() will return nullptr
   Emit("destroyed");
 
   // For guest view based on OOPIF, the WebContents is released by the embedder
@@ -1726,7 +1727,7 @@ void WebContents::LoadURL(const GURL& url,
   // ⚠️WARNING!⚠️
   // LoadURLWithParams() triggers JS events which can call destroy() on |this|.
   // It's not safe to assume that |this| points to valid memory at this point.
-  if (!weak_this)
+  if (!weak_this || !web_contents())
     return;
 }
 

+ 7 - 0
spec-main/api-web-contents-spec.ts

@@ -2007,6 +2007,13 @@ describe('webContents module', () => {
       });
       contents.loadURL('about:blank').then(() => contents.forcefullyCrashRenderer());
     });
+
+    it('does not crash main process when quiting in it', async () => {
+      const appPath = path.join(mainFixturesPath, 'apps', 'quit', 'main.js');
+      const appProcess = ChildProcess.spawn(process.execPath, [appPath]);
+      const [code] = await emittedOnce(appProcess, 'close');
+      expect(code).to.equal(0);
+    });
   });
 
   it('emits a cancelable event before creating a child webcontents', async () => {

+ 10 - 0
spec-main/fixtures/apps/quit/main.js

@@ -0,0 +1,10 @@
+const { app, BrowserWindow } = require('electron');
+
+app.once('ready', () => {
+  const w = new BrowserWindow({ show: false, webPreferences: { nodeIntegration: true } });
+  w.webContents.once('crashed', () => {
+    app.quit();
+  });
+  w.webContents.loadURL('about:blank');
+  w.webContents.executeJavaScript('process.crash()');
+});