Skip to content

fix: emit render-process-gone outside the process-death notification#51900

Merged
jkleinsc merged 3 commits into
mainfrom
fix/renderer-startup-helper-reregister
Jun 8, 2026
Merged

fix: emit render-process-gone outside the process-death notification#51900
jkleinsc merged 3 commits into
mainfrom
fix/renderer-startup-helper-reregister

Conversation

@VerteDinde

@VerteDinde VerteDinde commented Jun 5, 2026

Copy link
Copy Markdown
Member

Description of Change

Fixes a browser-process crash (CHECK failure in extensions::RendererStartupHelper::OnRenderProcessLaunched, surfacing as EXC_BREAKPOINT on macOS / EXCEPTION_BREAKPOINT on Windows on the CrBrowserMain thread) that occurs when an app reacts synchronously to the render-process-gone event, most commonly by calling webContents.reload() from the handler.

render-process-gone was emitted synchronously from WebContents::PrimaryMainFrameRenderProcessGone, which is invoked while RenderProcessHostImpl::ProcessDied() is still iterating the host's observer list. A reload (or any navigation) from the handler re-enters RenderProcessHostImpl::Init() from inside that loop; the resulting OnRenderProcessHostCreated() dispatch is ignored by RendererStartupHelper because the stale registration from the previous renderer is still present, the helper then untracks the process when its own RenderProcessExited() runs, and when the relaunched process finishes launching, OnRenderProcessLaunched() finds no registration and the CHECK brings down the browser process.

Per review feedback, this no longer touches the upstream CHECK (originally a patches/chromium change). Instead, the render-process-gone emit is deferred by one task, so the process-death notification fully unwinds before app code can react — covering reload(), loadURL(), window recreation, and any other synchronous reaction. Chromium relaxed the same CHECK for this race on Android (https://crrev.com/c/7330559) and ChromeOS (https://crrev.com/c/7862000, https://crbug.com/512916518); extending that to desktop platforms will be pursued upstream separately, since a content-internal path to the same re-entrancy exists that embedder-side deferral cannot reach.

Two intentional behavior notes for review:

  • The event now fires one task later than before. The exitCode is captured at observer time, so the reported details are unchanged.
  • If the WebContents is destroyed within that one-task window, the event is not emitted (the JS wrapper is gone; destroyed fires as usual).

Verified locally on macOS (arm64) with the CHECK intact: a standalone repro (synchronous reload() inside render-process-gone + forcefullyCrashRenderer()) reliably crashes an unpatched build of main with FATAL:extensions/browser/renderer_startup_helper.cc:285] Check failed: GetRenderer(host) != nullptr || !client->IsSameContext(...), and survives with the deferred emit (reload completes normally). The added regression spec exercises the same sequence.

Checklist

Release Notes

Notes: Fixed a browser process crash when calling webContents.reload() or navigating synchronously from the render-process-gone event; the event is now emitted after the renderer's teardown notification has completed.

…forms

Extends Chromium's Android/ChromeOS re-register carve-out in
RendererStartupHelper::OnRenderProcessLaunched() to all platforms. The
CHECK it replaces fails when a RenderProcessHost is re-initialized from
inside the process-death notification - for example when an app reloads
a WebContents synchronously from the 'render-process-gone' event - and
brings down the browser process. Adds a regression spec that crashes a
renderer and reloads it synchronously from the handler.

Refs: crrev.com/c/7330559, crrev.com/c/7862000, crbug.com/512916518
@VerteDinde VerteDinde added semver/patch backwards-compatible bug fixes target/41-x-y PR should also be added to the "41-x-y" branch. target/42-x-y PR should also be added to the "42-x-y" branch. target/43-x-y PR should also be added to the "43-x-y" branch. labels Jun 5, 2026
@electron-cation electron-cation Bot added the new-pr 🌱 PR opened recently label Jun 5, 2026
@VerteDinde VerteDinde marked this pull request as ready for review June 6, 2026 05:15
@VerteDinde VerteDinde requested a review from a team as a code owner June 6, 2026 05:15
Replaces the renderer_startup_helper.cc chromium patch from the previous
commit: instead of relaxing the upstream CHECK, defer the
'render-process-gone' emit by one task so app code can no longer
re-enter RenderProcessHostImpl::Init() from inside the process-death
observer loop. Addresses review feedback that the CHECK should stay
intact.
@VerteDinde VerteDinde changed the title fix: handle render process reuse in RendererStartupHelper on all platforms fix: emit render-process-gone outside the process-death notification Jun 6, 2026
@electron-cation electron-cation Bot removed the new-pr 🌱 PR opened recently label Jun 7, 2026
@jkleinsc jkleinsc merged commit 571d6e9 into main Jun 8, 2026
125 of 127 checks passed
@release-clerk

release-clerk Bot commented Jun 8, 2026

Copy link
Copy Markdown

Release Notes Persisted

Fixed a browser process crash when calling webContents.reload() or navigating synchronously from the render-process-gone event; the event is now emitted after the renderer's teardown notification has completed.

@trop

trop Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

I have automatically backported this PR to "42-x-y", please check out #51916

@trop trop Bot added the in-flight/42-x-y label Jun 8, 2026
@trop trop Bot removed the target/42-x-y PR should also be added to the "42-x-y" branch. label Jun 8, 2026
@trop

trop Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

I have automatically backported this PR to "41-x-y", please check out #51917

@trop

trop Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

I have automatically backported this PR to "43-x-y", please check out #51918

@trop trop Bot added in-flight/41-x-y in-flight/43-x-y and removed target/41-x-y PR should also be added to the "41-x-y" branch. target/43-x-y PR should also be added to the "43-x-y" branch. in-flight/43-x-y labels Jun 8, 2026
@trop trop Bot added merged/43-x-y PR was merged to the "43-x-y" branch. merged/42-x-y PR was merged to the "42-x-y" branch. merged/41-x-y PR was merged to the "41-x-y" branch. and removed in-flight/42-x-y in-flight/41-x-y labels Jun 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged/41-x-y PR was merged to the "41-x-y" branch. merged/42-x-y PR was merged to the "42-x-y" branch. merged/43-x-y PR was merged to the "43-x-y" branch. semver/patch backwards-compatible bug fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants