fix: prevent use-after-free when destroying guest WebContents during event emission
Multiple event emission sites in WebContents destroy the underlying C++
object via a JavaScript event handler calling webContents.destroy(), then
continue to dereference the freed `this` pointer. This is exploitable
through <webview> guest WebContents because Destroy() calls `delete this`
synchronously for guests, unlike non-guests which safely defer deletion.
The fix has two layers:
1. A new `is_emitting_event_` flag is checked in Destroy() — when true,
guest deletion is deferred to a posted task instead of executing
synchronously. This is separate from `is_safe_to_delete_` (which
gates LoadURL re-entrancy) to avoid rejecting legitimate loadURL
calls from event handlers.
2. AutoReset<bool> guards on `is_emitting_event_` are added to
CloseContents, RenderViewDeleted, DidFinishNavigation, and
SetContentsBounds, preventing synchronous destruction while their
Emit() calls are on the stack.
Destroy() now requires both `is_safe_to_delete_` (navigation re-entrancy)
and `!is_emitting_event_` (event emission) to allow synchronous guest
deletion. The existing AutoReset guards on `is_safe_to_delete_` in
DidStartNavigation, DidRedirectNavigation, and ReadyToCommitNavigation
are also now effective for guests.
Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
fix: simpleFullScreen exits when web content calls requestFullscreen
SetHtmlApiFullscreen only checked IsFullscreen() to detect that the
window was already fullscreen, missing the simple-fullscreen case on
macOS. When web content triggered requestFullscreen the code fell
through to SetFullScreen(true) which toggled simple fullscreen off.
Include IsSimpleFullScreen() in the guard so the HTML-API fullscreen
state is updated without touching the window's fullscreen mode.
Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
Menu was holding a SelfKeepAlive to itself from construction, so any
Menu that was never opened (e.g. an application menu replaced before
being shown) stayed pinned in cppgc forever. Repeated calls to
Menu.setApplicationMenu leaked every prior Menu along with its model
and items.
Restore the original Pin/Unpin lifecycle: start keep_alive_ empty and
only assign `this` in OnMenuWillShow. OnMenuWillClose already clears
it.
Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
fix: propagate requesting frame through sync permission checks (#50679)
WebContentsPermissionHelper::CheckPermission was hardcoding
GetPrimaryMainFrame() and deriving the requesting origin from
web_contents_->GetLastCommittedURL(), so the setPermissionCheckHandler
callback always received the top frame's origin and
details.isMainFrame/details.requestingUrl always reflected the main
frame, even when a cross-origin subframe with allow="serial" or
allow="camera; microphone" triggered the check.
Thread the requesting RenderFrameHost through CheckPermission,
CheckSerialAccessPermission, and CheckMediaAccessPermission so the
permission manager receives the real requesting frame. Update the
serial delegate and WebContents::CheckMediaAccessPermission callers to
pass the frame they already have.
Adds a regression test that loads a cross-origin iframe with
allow="camera; microphone", calls enumerateDevices() from within the
iframe, and asserts the permission check handler receives the iframe
origin for requestingOrigin, isMainFrame, and requestingUrl.
The `OnTraceBufferUsageAvailable` callback creates V8 handles via
`Dictionary::CreateEmpty()` before `promise.Resolve()` enters its
`SettleScope` (which provides a `HandleScope`). When the callback
fires asynchronously from a Mojo response (i.e. when a trace session
is active), there is no `HandleScope` on the stack, causing a fatal
V8 error: "Cannot create a handle without a HandleScope".
Add an explicit `v8::HandleScope` at the top of the callback, matching
the pattern used by the other contentTracing APIs which resolve their
promises through `SettleScope` or the static `ResolvePromise` helper.
Made-with: Cursor
Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Alexey Kozy <alexey@anysphere.co>
* fix: correct utility process exit code on Windows
On Windows, process exit codes are 32-bit unsigned integers (DWORD).
When passed from Chromium to Electron as a signed int and then
implicitly converted to uint64_t, values with the high bit set
(e.g., NTSTATUS codes) undergo sign extension, producing incorrect
values.
Cast the exit code to uint32_t before widening to uint64_t to
prevent sign extension and preserve the original Windows exit code.
Fixes#49455
Co-authored-by: João Silva <joaomrsilva@tecnico.ulisboa.pt>
* fix: narrow HandleTermination and Shutdown to uint32_t, add tests
Co-authored-by: João Silva <joaomrsilva@tecnico.ulisboa.pt>
---------
Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: João Silva <joaomrsilva@tecnico.ulisboa.pt>
fix: correctly set IsActive() in BaseWindow on MacOS
Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Kyle Cutler <kycutler@microsoft.com>
Avoids a freeze when failing to enter fullscreen on macOS.
Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: John Beutner <beutner.john@gmail.com>
* docs: document that getCursorScreenPoint() needs a Window on Wayland
Co-authored-by: Charles Kerr <charles@charleskerr.com>
* feat: add IsWayland() helper
Co-authored-by: Charles Kerr <charles@charleskerr.com>
* fix: Wayland crash in GetCursorScreenPoint()
fix: support Screen::GetCursorScreenPoint() on X11
Co-authored-by: Charles Kerr <charles@charleskerr.com>
---------
Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Charles Kerr <charles@charleskerr.com>
PowerMonitor registered OS-level callbacks (HWND UserData and
WTS/suspend notifications on Windows, shutdown handler and lock-screen
observer on macOS) but never cleaned them up in its destructor. The JS
layer also only held the native object in a closure-local variable,
allowing GC to reclaim it while those registrations still referenced
freed memory.
Retain the native PowerMonitor at module level in power-monitor.ts so
it cannot be garbage-collected. Add DestroyPlatformSpecificMonitors()
to properly tear down OS registrations on destruction: on Windows,
unregister WTS and suspend notifications, clear GWLP_USERDATA, and
destroy the HWND; on macOS, remove the emitter from the global
MacLockMonitor and reset the Browser shutdown handler.
Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
fix: bind offscreen paint callback to child WebContents
Previously, MaybeOverrideCreateParamsForNewWindow bound the
OffScreenWebContentsView's paint callback to the parent WebContents
using base::Unretained(this). This was both unsafe (dangling pointer
risk if the parent is destroyed before the child) and semantically
incorrect — paint events belong to the child window, not the opener.
Replace the callback in MaybeOverrideCreateParamsForNewWindow with
base::DoNothing(), then rebind it to the child WebContents in
AddNewContents via a new SetCallback method on OffScreenWebContentsView.
Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
EnterFullscreenModeForTab, RequestPointerLock, and RequestKeyboardLock
bind callbacks with base::Unretained(this); fullscreen also captures a
raw RenderFrameHost*. These callbacks may be invoked by the app's JS
permission handler after the WebContents or RenderFrameHost is destroyed.
Use GetWeakPtr() in all three call sites, and capture a
GlobalRenderFrameHostToken instead of the raw RenderFrameHost* for
fullscreen so the pointer is resolved and null-checked only when the
callback fires. Cancel in-flight permission requests from ~WebContents()
via a new ElectronPermissionManager::CancelPendingRequests()` so stale
callbacks are never handed back to JS.
Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
fix: update label/sublabel/icon in macOS item on open
Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
fix: apply zoomFactor from setWindowOpenHandler to window.open() windows
Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
* Revert "fix: fix Windows MSIX release build errors (#49613)"
This reverts commit 4b5d5f9dd5.
Co-authored-by: Keeley Hammond <khammond@slack-corp.com>
* refactor: use WRL ComPtr pattern for MSIX to avoid exception handling
The MSIX auto-updater code was using C++/WinRT (winrt::* namespace), which requires exception handling (/EHsc). Mixing exception and non-exception handling code in the same binary is problematic at runtime. This commit refactors electron_api_msix_updater.cc to use an upstream Chromium pattern and eliminates the need for special exception handling build flags
Co-authored-by: Keeley Hammond <khammond@slack-corp.com>
* build: import correct packages
Co-authored-by: Keeley Hammond <khammond@slack-corp.com>
* build: consolidate IPackage declarations
Co-authored-by: Keeley Hammond <khammond@slack-corp.com>
* refactor: use IPackageManager/IPackageManager5/IPackageManager9 and IPackage/IPackage2/IPackage4/IPackage6 interfaces as needed for different API methods.
Also consolidates duplicate completion handler logic, fixes a bug in
RegisterRestartOnUpdate where the command line string could go out of
scope, and removes unused includes.
Co-authored-by: Keeley Hammond <khammond@slack-corp.com>
---------
Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Keeley Hammond <khammond@slack-corp.com>
* chore: bump chromium in DEPS to 143.0.7485.0
* chore: update allow_disabling_blink_scheduler_throttling_per_renderview.patch
Move SetSupportsDraggableRegions mojom IPC from chrome/ and extensions/ to blink/ | https://chromium-review.googlesource.com/c/chromium/src/+/7043264
Patch applied manually due to context shear
* Move SetSupportsDraggableRegions mojom IPC from chrome/ and extensions/ to blink/ | https://chromium-review.googlesource.com/c/chromium/src/+/7043264
* chore: e patches all
* chore: check for file existence before setting utime
* chore: stop disabling kWinDelaySpellcheckServiceInit
[cleanup] Remove feature WinDelaySpellcheckServiceInit | https://chromium-review.googlesource.com/c/chromium/src/+/7012087
This flag has been removed upstream. We've used it since c2d7164 (#38248)
to fix a crash originally described in 97b353a (#34993):
> Delaying spell check initialization is causing specs for
> 'custom dictionary word list API' to fail in Electron.
Since we haven't touched this in a few years, and since there's a
clear success criteria (a passing/failing spec), and since the patch
to restore this flag would be pretty large (~750 lines), I'm going
to try just removing the flag from our source to see if the spec
passes or fails.
* Revert "chore: stop disabling kWinDelaySpellcheckServiceInit"
This reverts commit e688880396.
Unfortunately, the crash persists.
* Revert [cleanup] Remove feature WinDelaySpellcheckServiceInit | https://chromium-review.googlesource.com/c/chromium/src/+/7012087
We currently need this feature
* fixup! chore: check for file existence before setting utime
* fixup! Move SetSupportsDraggableRegions mojom IPC from chrome/ and extensions/ to blink/ | https://chromium-review.googlesource.com/c/chromium/src/+/7043264
Address Robo's code review @ 64c7fd21ed
* fixup! fixup! chore: check for file existence before setting utime
fix: oops typo
---------
Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Charles Kerr <charles@charleskerr.com>
