fix: render process crash handling (#34428)

* fix: crash when renderer process is reused

Could occur when a renderer crashes and the same-origin URL is loaded again
which leads to reusing the renderer process.

* test: renderer process crash recovery

* fix: handle case which leads to render frame DCHECK

* fix: lint
This commit is contained in:
Samuel Maddock 2022-06-02 20:23:01 -04:00 committed by GitHub
parent eb26f99f6e
commit b00c026a54
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 38 additions and 1 deletions

View file

@ -188,13 +188,20 @@ const mojo::Remote<mojom::ElectronRenderer>& WebFrameMain::GetRendererApi() {
} }
void WebFrameMain::MaybeSetupMojoConnection() { void WebFrameMain::MaybeSetupMojoConnection() {
if (render_frame_disposed_) {
// RFH may not be set yet if called between when a new RFH is created and
// before it's been swapped with an old RFH.
LOG(INFO) << "Attempt to setup WebFrameMain connection while render frame "
"is disposed";
return;
}
if (!renderer_api_) { if (!renderer_api_) {
pending_receiver_ = renderer_api_.BindNewPipeAndPassReceiver(); pending_receiver_ = renderer_api_.BindNewPipeAndPassReceiver();
renderer_api_.set_disconnect_handler(base::BindOnce( renderer_api_.set_disconnect_handler(base::BindOnce(
&WebFrameMain::OnRendererConnectionError, weak_factory_.GetWeakPtr())); &WebFrameMain::OnRendererConnectionError, weak_factory_.GetWeakPtr()));
} }
// Render frame should exist when this method is called.
DCHECK(render_frame_); DCHECK(render_frame_);
// Wait for RenderFrame to be created in renderer before accessing remote. // Wait for RenderFrame to be created in renderer before accessing remote.

View file

@ -451,6 +451,9 @@ void ElectronBrowserClient::RenderProcessWillLaunch(
new extensions::MessagingAPIMessageFilter(process_id, browser_context)); new extensions::MessagingAPIMessageFilter(process_id, browser_context));
#endif #endif
// Remove in case the host is reused after a crash, otherwise noop.
host->RemoveObserver(this);
// ensure the ProcessPreferences is removed later // ensure the ProcessPreferences is removed later
host->AddObserver(this); host->AddObserver(this);
} }

View file

@ -224,6 +224,33 @@ describe('webFrameMain module', () => {
expect(w.webContents.mainFrame).to.equal(mainFrame); expect(w.webContents.mainFrame).to.equal(mainFrame);
expect(mainFrame.url).to.equal(crossOriginUrl); expect(mainFrame.url).to.equal(crossOriginUrl);
}); });
it('recovers from renderer crash on same-origin', async () => {
const server = await createServer();
// Keep reference to mainFrame alive throughout crash and recovery.
const { mainFrame } = w.webContents;
await w.webContents.loadURL(server.url);
w.webContents.forcefullyCrashRenderer();
await w.webContents.loadURL(server.url);
// Log just to keep mainFrame in scope.
console.log('mainFrame.url', mainFrame.url);
});
// Fixed by #34411
it('recovers from renderer crash on cross-origin', async () => {
const server = await createServer();
// 'localhost' is treated as a separate origin.
const crossOriginUrl = server.url.replace('127.0.0.1', 'localhost');
// Keep reference to mainFrame alive throughout crash and recovery.
const { mainFrame } = w.webContents;
await w.webContents.loadURL(server.url);
w.webContents.forcefullyCrashRenderer();
// A short wait seems to be required to reproduce the crash.
await new Promise(resolve => setTimeout(resolve, 100));
await w.webContents.loadURL(crossOriginUrl);
// Log just to keep mainFrame in scope.
console.log('mainFrame.url', mainFrame.url);
});
}); });
describe('webFrameMain.fromId', () => { describe('webFrameMain.fromId', () => {