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

This commit is contained in:
Cheng Zhao 2021-03-03 02:38:56 +09:00 committed by GitHub
parent ed8e57e424
commit ede8611937
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 22 additions and 4 deletions

View file

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

View file

@ -2016,6 +2016,13 @@ describe('webContents module', () => {
}); });
contents.loadURL('about:blank').then(() => contents.forcefullyCrashRenderer()); 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 () => { it('emits a cancelable event before creating a child webcontents', async () => {

View file

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