fix: render process crash handling (#34430)

* 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

Co-authored-by: samuelmaddock <samuel.maddock@gmail.com>

shell/browser/api/electron_api_web_frame_main.cc

@@ -189,13 +189,20 @@ const mojo::Remote<mojom::ElectronRenderer>& WebFrameMain::GetRendererApi() {
 }
 
 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_) {
     pending_receiver_ = renderer_api_.BindNewPipeAndPassReceiver();
     renderer_api_.set_disconnect_handler(base::BindOnce(
         &WebFrameMain::OnRendererConnectionError, weak_factory_.GetWeakPtr()));
   }
 
-  // Render frame should exist when this method is called.
   DCHECK(render_frame_);
 
   // Wait for RenderFrame to be created in renderer before accessing remote.

shell/browser/electron_browser_client.cc

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

spec-main/api-web-frame-main-spec.ts

@@ -224,6 +224,33 @@ describe('webFrameMain module', () => {
       expect(w.webContents.mainFrame).to.equal(mainFrame);
       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', () => {