Compare commits

...

12 Commits

Author SHA1 Message Date
deepak1556
3fa35aa02f chore: update patches 2026-03-24 09:24:54 +09:00
deepak1556
19f4f382ac fix: crash during gc from incorrect cppgc object headers 2026-03-24 08:59:00 +09:00
deepak1556
2f02988871 fix: destruction order
- Setup isolate dispose observer to run destruction sequences
and remove self persistent reference
- Skip NOTREACHED check during destruction, it can happen
as a result of plaform listeners scheduling callbacks when Unregister is invoked.
- Fix the order of unregistration in GlobalShortcut::Unregister
- Add GlobalShortcut::UnregisterAllInternal to avoid any callsites
that can re-enter V8
2026-03-24 08:55:38 +09:00
Charles Kerr
7e9fb5897a fixup! refactor: use gin::WeakCellFactory in GlobalCallbacks
fix: must Trace() the weak cell factory
2026-03-24 08:55:37 +09:00
Charles Kerr
088d57814f chore: reduce unnecessary diffs with main 2026-03-24 08:55:37 +09:00
Charles Kerr
69c05c6533 fix: make a copy of callback before running it
safeguard against the callback changing the map, invalidating `cb`
2026-03-24 08:55:37 +09:00
Charles Kerr
d38e6ef426 refactor: use gin::WeakCellFactory in GlobalCallbacks 2026-03-24 08:55:37 +09:00
Charles Kerr
7f695c278c chore: update chore_add_electron_objects_to_wrappablepointertag.patch 2026-03-24 08:55:37 +09:00
Charles Kerr
275fbc743b refactor: lazy-create electron::api::GlobalShortcut
copy the lazy-create idom used by electron::api::Screen
2026-03-24 08:55:37 +09:00
Charles Kerr
df52ad22ba refactor: migrate electron::api::GlobalShortcut to cppgc 2026-03-24 08:55:36 +09:00
Samuel Attard
dbcf0fb5f0 fix: lazily initialize safeStorage async encryptor (#50419)
* fix: lazily initialize safeStorage async encryptor

The SafeStorage constructor previously registered a browser observer that
called os_crypt_async()->GetInstance() on app-ready. Because ESM named
imports (import { x } from 'electron') eagerly evaluate all electron
module getters, simply importing electron in an ESM entrypoint would
construct SafeStorage and touch the OS keychain on app-ready, even when
safeStorage was never used.

This showed up as a macOS CI hang: the esm-spec import-meta fixture
triggers a keychain access prompt that blocks the test runner until
timeout.

Now the async encryptor is requested lazily on the first call to
encryptStringAsync, decryptStringAsync, or isAsyncEncryptionAvailable.
isAsyncEncryptionAvailable now returns a Promise that resolves once
initialization completes, matching what the docs already stated.

* chore: lint

* fix: add HandleScope in OnOsCryptReady for pending operations

OnOsCryptReady fires asynchronously from a posted task without an active
V8 HandleScope. Previously this was harmless because eager init meant the
pending queues were always empty when it fired. With lazy init, operations
queue up first, then the callback processes them and needs to create V8
handles (Buffer::Copy, Dictionary::CreateEmpty, Promise::Resolve).
2026-03-23 10:47:14 -07:00
Samuel Attard
29750dda08 build: enable V8 builtins PGO (#50416)
* build: enable V8 builtins PGO

Removes the gn arg that disabled V8 builtins profile-guided optimization
and adds a V8 patch to warn instead of abort when the builtin PGO profile
data does not match. Also strips the PGO-related flags from the generated
mksnapshot_args so they are not passed through to downstream mksnapshot
invocations.

* docs: clarify Node.js async_hooks as reason for promise_hooks flag

Addresses review feedback: the v8_enable_javascript_promise_hooks flag
is set to support Node.js async_hooks, not used directly by Electron.
2026-03-23 11:54:43 -04:00
14 changed files with 572 additions and 89 deletions

View File

@@ -128,6 +128,9 @@ runs:
fi
sed $SEDOPTION '/.*builtins-pgo/d' out/Default/mksnapshot_args
sed $SEDOPTION '/--turbo-profiling-input/d' out/Default/mksnapshot_args
sed $SEDOPTION '/--reorder-builtins/d' out/Default/mksnapshot_args
sed $SEDOPTION '/--warn-about-builtin-profile-data/d' out/Default/mksnapshot_args
sed $SEDOPTION '/--abort-on-bad-builtin-profile-data/d' out/Default/mksnapshot_args
if [ "${{ inputs.target-platform }}" = "win" ]; then
cd out/Default

View File

@@ -51,9 +51,6 @@ is_cfi = false
use_qt5 = false
use_qt6 = false
# Disables the builtins PGO for V8
v8_builtins_profiling_log_file = ""
# https://chromium.googlesource.com/chromium/src/+/main/docs/dangling_ptr.md
# TODO(vertedinde): hunt down dangling pointers on Linux
enable_dangling_raw_ptr_checks = false

View File

@@ -59,7 +59,12 @@ On Windows, returns true once the app has emitted the `ready` event.
### `safeStorage.isAsyncEncryptionAvailable()`
Returns `Promise<Boolean>` - Whether encryption is available for asynchronous safeStorage operations.
Returns `Promise<boolean>` - Resolves with whether encryption is available for
asynchronous safeStorage operations.
The asynchronous encryptor is initialized lazily the first time this method,
`encryptStringAsync`, or `decryptStringAsync` is called after the app is ready.
The returned promise resolves once initialization completes.
### `safeStorage.encryptString(plainText)`

View File

@@ -1,2 +1,36 @@
const { globalShortcut } = process._linkedBinding('electron_browser_global_shortcut');
export default globalShortcut;
const { createGlobalShortcut } = process._linkedBinding('electron_browser_global_shortcut');
let globalShortcut: Electron.GlobalShortcut;
const createGlobalShortcutIfNeeded = () => {
if (globalShortcut === undefined) {
globalShortcut = createGlobalShortcut();
}
};
export default new Proxy({}, {
get: (_target, property: keyof Electron.GlobalShortcut) => {
createGlobalShortcutIfNeeded();
const value = globalShortcut[property];
if (typeof value === 'function') {
return value.bind(globalShortcut);
}
return value;
},
set: (_target, property: string, value: unknown) => {
createGlobalShortcutIfNeeded();
return Reflect.set(globalShortcut, property, value);
},
ownKeys: () => {
createGlobalShortcutIfNeeded();
return Reflect.ownKeys(globalShortcut);
},
has: (_target, property: string) => {
createGlobalShortcutIfNeeded();
return property in globalShortcut;
},
getOwnPropertyDescriptor: (_target, property: string) => {
createGlobalShortcutIfNeeded();
return Reflect.getOwnPropertyDescriptor(globalShortcut, property);
}
});

View File

@@ -8,10 +8,10 @@ electron objects that extend gin::Wrappable and gets
allocated on the cpp heap
diff --git a/gin/public/wrappable_pointer_tags.h b/gin/public/wrappable_pointer_tags.h
index fee622ebde42211de6f702b754cfa38595df5a1c..6b524632ebb405e473cf4fe8e253bd13bf7b67e5 100644
index fee622ebde42211de6f702b754cfa38595df5a1c..3bdcd3f3f0a36314694495ca7361be14d95da911 100644
--- a/gin/public/wrappable_pointer_tags.h
+++ b/gin/public/wrappable_pointer_tags.h
@@ -77,7 +77,20 @@ enum WrappablePointerTag : uint16_t {
@@ -77,7 +77,21 @@ enum WrappablePointerTag : uint16_t {
kWebAXObjectProxy, // content::WebAXObjectProxy
kWrappedExceptionHandler, // extensions::WrappedExceptionHandler
kIndigoContext, // indigo::IndigoContext
@@ -20,6 +20,7 @@ index fee622ebde42211de6f702b754cfa38595df5a1c..6b524632ebb405e473cf4fe8e253bd13
+ kElectronDataPipeHolder, // electron::api::DataPipeHolder
+ kElectronDebugger, // electron::api::Debugger
+ kElectronEvent, // gin_helper::internal::Event
+ kElectronGlobalShortcut, // electron::api::GlobalShortcut
+ kElectronMenu, // electron::api::Menu
+ kElectronNetLog, // electron::api::NetLog
+ kElectronPowerMonitor, // electron::api::PowerMonitor

View File

@@ -1 +1,3 @@
chore_allow_customizing_microtask_policy_per_context.patch
build_warn_instead_of_abort_on_builtin_pgo_profile_mismatch.patch
cppgc_tale_of_bad_inheritance_order.patch

View File

@@ -0,0 +1,35 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Sam Attard <sattard@anthropic.com>
Date: Sun, 22 Mar 2026 10:51:26 +0000
Subject: build: warn instead of abort on builtin PGO profile mismatch
Electron sets v8_enable_javascript_promise_hooks = true to support
Node.js async_hooks (see node/src/env.cc SetPromiseHooks usage:
https://github.com/nodejs/node/blob/abff716eaccd0c4f4949d1315cb057a45979649d/src/env.cc#L223-L236).
This flag adds conditional branches to builtins-microtask-queue-gen.cc
and promise-misc.tq, changing the control-flow graph hash of several
Promise/async builtins. This invalidates V8's pre-generated PGO profile
for those builtins (built with Chrome defaults where the flag is off).
Rather than disabling builtins PGO entirely, warn and skip mismatched
builtins so all other builtins still benefit from PGO.
diff --git a/BUILD.gn b/BUILD.gn
index 078b63b2bdbb3f952bd0f579e84fb691e308fb64..985f946d5f87b4e6eb32a011ac47a0073248e5f2 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -2803,9 +2803,11 @@ template("run_mksnapshot") {
"--turbo-profiling-input",
rebase_path(v8_builtins_profiling_log_file, root_build_dir),
- # Replace this with --warn-about-builtin-profile-data to see the full
- # list of builtins with incompatible profiles.
- "--abort-on-bad-builtin-profile-data",
+ # Electron: Use warn instead of abort so that builtins whose control
+ # flow is changed by Electron's build flags (e.g. RunMicrotasks via
+ # v8_enable_javascript_promise_hooks) are skipped rather than failing
+ # the build. All other builtins still receive PGO.
+ "--warn-about-builtin-profile-data",
]
if (!v8_enable_builtins_profiling && v8_enable_builtins_reordering) {

View File

@@ -0,0 +1,319 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: deepak1556 <hop2deep@gmail.com>
Date: Tue, 24 Mar 2026 08:52:10 +0900
Subject: cppgc: tale of bad inheritance order
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Some background on certain V8 concepts needed to understand the
crux of this issue.
1) cppgc HeapObjectHeader
When a C++ object is allocated on cppgc's managed heap via
MakeGarbageCollected<T>(), a HeapObjectHeader (8 bytes) is placed
immediately before the object's payload in memory:
[4 bytes padding][encoded_high: gc_info_index + fully_constructed bit]
[encoded_low: size + mark_bit]
[payload...]
The gc_info_index points into a global GCInfo table containing
callbacks (trace, finalize, name) for each registered cppgc type.
When the sweeper frees an object, it overwrites the header with a
free-list entry (gc_info_index = 0). IsFree() checks this atomically.
2) JSObject ↔ cppgc bindings
When a cppgc object is associated with a JSObject (via gin::Wrappable
or v8::Object::Wrap), two references are created:
- cppgc → JSObject: TracedReference<v8::Object>, traced by cppgc's
Trace() method. Keeps the JSObject alive while the cppgc object
is alive.
- JSObject → cppgc: a CppHeapPointer in the JSObject's embedder
slot. This is NOT a GC root — it does not keep the cppgc object
alive on its own.
In the unified heap, V8 and cppgc mark together. When the marker
visits a JSObject with a CppHeapPointer slot, VisitCppHeapPointer
reads the pointer and calls MarkAndPush to mark the cppgc object.
3) CppHeapPointerTable (V8_COMPRESS_POINTERS)
With pointer compression enabled, JSObject embedder slots do not
store raw pointers. Instead, a CppHeapPointerHandle (uint32_t) is
stored at the embedder slot, which indexes into the Isolate's
CppHeapPointerTable — a flat array of 8-byte entries at a virtual
address reservation (base_).
JSObject layout for JSAPIObjectWithEmbedderSlots:
Raw offset Field
0x0 map (compressed tagged)
0x4 properties_or_hash (compressed tagged)
0x8 elements (compressed tagged)
0xc CppHeapPointerHandle (uint32_t)
The handle is converted to a table index via:
index = handle >> kCppHeapPointerIndexShift (6 on desktop)
Each 8-byte table entry encodes:
bits 63-16: cppgc object payload address
bits 15-1: type tag
bit 0: mark bit (for table GC)
During marking, VisitCppHeapPointer:
1. Loads the handle from the JSObject
2. Calls table->Mark() to set the entry's mark bit
3. Calls table->Get() to decode the cppgc pointer (payload >> 16)
4. Calls MarkAndPush on the cppgc object
During sweep, CppHeapPointerTable::SweepAndCompact frees unmarked
entries and clears the mark bit on live entries.
The crash symptoms:
Two crash scenarios were observed during cppgc migration of Electron
API globalshortcut module
Scenario 1 — concurrent marker (V8Worker thread):
Header at 0x12402000ec0: gc_info_index=308, size=0
Stack: ConcurrentMarking::RunMajor → VisitJSObjectSubclass
<JSAPIObjectWithEmbedderSlots> → MarkAndPush → V8_Fatal
The concurrent marker reads a CppHeapPointer from a JSObject,
follows it to freed memory, interprets garbage as a header.
Scenario 2 — atomic pause (CrBrowserMain thread):
Header at 0x12402000ec0: gc_info_index=324, size=0, mark_bit=1
Stack: MarkNotFullyConstructedObjects → TraceConservatively
→ ObjectView → V8_Fatal (page->is_large() DCHECK)
Investigation:
Since the marker and tracer were crashing on a
cppgc object with size 0 which shouldn't be possible for a live object,
started with adding GCInfo trace callback validation at three
processing points — CppMarkingState::MarkAndPush, MarkerBase::
MarkNotFullyConstructedObjects, and MutatorMarkingState::
FlushNotFullyConstructedObjects. Checking gc_info.trace != nullptr
before processing reliably detected stale headers because the GCInfo
table is global and valid for any index. This stopped the crashes.
However, this was treating symptoms. To find the root cause, we need
to trace the stale pointer backwards from the crash site to its
origin in the CppHeapPointerTable.
From the Scenario 1 crash backtrace:
frame #0: CppMarkingState::MarkAndPush(header=0x12402000ec0)
frame #1: IterateJSAPIObjectWithEmbedderSlotsHeader(...)
frame #2: ConcurrentMarking::RunMajor(...)
Register read in frame #0 (MarkAndPush) showed x19 = 0x12402000ec0
— the HeapObjectHeader* argument. To trace how this value was
computed, disassembling the caller in frame #1
(IterateJSAPIObjectWithEmbedderSlotsHeader, which inlines
VisitCppHeapPointer) revealed the instruction sequence:
+72: ldr w21, [x20] ← load handle from JSObject+0xc
+80: ldr x22, [x19, #0x68] ← CppHeapPointerTable* from visitor
+120: ldr x8, [x22] ← table base_ (first field)
+132: ldr x20, [x8, x9, lsl #3] ← entry = base_[handle >> 6]
+148: lsr x8, x20, #16 ← decode: cppgc_ptr = payload >> 16
+168: sub x1, x8, #0x8 ← header = cppgc_ptr - 8
+188: b MarkAndPush ← tail call with stale header
Reading the actual values:
Handle: 0x00080380 (from JSObject embedder slot at raw+0xc)
Index: 0x00080380 >> 6 = 8206
Table base: 0x0000000320000000 (visitor+0x68 → table ptr → base_)
Entry: base_[8206] = 0x012402000ec810e1
Decoded: 0x012402000ec810e1 >> 16 = 0x12402000ec8 (payload addr)
Header: 0x12402000ec8 - 8 = 0x12402000ec0
This exactly matched x19 in the crash frame. The header at
0x12402000ec0 had encoded_low=0x0000 (freed memory), confirming
the table entry was pointing to some garbage cppgc object.
Theory: Stale table entries
VisitCppHeapPointer calls table->Mark() unconditionally before
validating the cppgc object, which could preserve stale
entries across GC cycles. Moving table->Mark() after an IsFree()
check stopped the crashes in the marker thread.
But this was still treating symptoms — it did not explain how the table entry
became stale and we still saw the DCHECK during atomic pause
on the main thread.
All that's left is to trace what headers are being pushed
into the not_fully_constructed_worklist and using hardware
watchpoint identify if a valid header is being written to a non header
value.
With debug traps at all three push sites to the
not_fully_constructed_worklist (MarkAndPush in marking-state.h,
ProcessWeakContainer in marking-state.h, and WriteBarrierForObject
in marker.h), found that the write barrier was the active push
path. The trap fired during:
CppHeap::WriteBarrier
← v8::Object::Wrap
← gin::WrappableBase::AssociateWithWrapper
← electron::api::GlobalShortcut::Create
At this point reading the register state revealed that the write barrier
was pushing header 0xec0 to the worklist (didn't require any watchpoint).
But the real GlobalShortcut header was at 0xea0 (with payload at 0xea8 and
size 0x88 = 136 bytes). Address 0xec0 fell inside the GlobalShortcut
payload — 0x20 bytes past the real header. It was not a header at
all, but interior object data misinterpreted as one.
Same address in both crashes — in Scenario 1 the concurrent marker
read it via CppHeapPointerTable; in Scenario 2 the atomic pause
read it from the not_fully_constructed_worklist. Both interpret
interior object data at 0xec0 as a HeapObjectHeader,
yielding different garbage gc_info_index values.
Root cause: multiple inheritance base class ordering
Tracing the call chain revealed the source of the incorrect header
address:
gin::WrappableBase::AssociateWithWrapper calls:
v8::Object::Wrap(isolate, wrapper, this, tag)
v8::Object::Wrap stores the pointer in the CppHeapPointerTable via:
CppHeapObjectWrapper::SetCppHeapWrappable(isolate, wrappable, tag)
The write barrier fires inside SetCppHeapWrappable:
WriteBarrier::ForCppHeapPointer(host, slot, value)
→ MarkingSlowFromCppHeapWrappable(heap, host, slot, object)
→ CppHeap::WriteBarrier(object)
→ HeapObjectHeader::FromObject(object)
= *(object - 8) ← WRONG when object is interior
CppHeap::WriteBarrier computes:
HeapObjectHeader::FromObject(object) → object - 8
This assumes `object` points to the cppgc allocation base. But in
AssociateWithWrapper, `this` is a WrappableBase* — the gin::Wrappable
subobject within GlobalShortcut. Due to multiple inheritance, this
subobject sat at offset +0x20 from the cppgc allocation base.
GlobalShortcut's original declaration was:
class GlobalShortcut final
: private ui::GlobalAcceleratorListener::Observer, // offset 0
public gin::PerIsolateData::DisposeObserver, // offset +X
public gin::Wrappable<GlobalShortcut> { // offset +0x20
gin::Wrappable<GlobalShortcut>
→ gin::WrappableBase
→ v8::Object::Wrappable
→ cppgc::GarbageCollected<Wrappable>
With the cppgc object allocated at 0xea8 (header at 0xea0), the
gin::Wrappable subobject sat at 0xea8 + 0x20 = 0xec8. This 0xec8
was stored in the CppHeapPointerTable. When the write barrier called
HeapObjectHeader::FromObject(0xec8), it computed 0xec8 - 8 = 0xec0
— a location inside the GlobalShortcut payload, not the real header
at 0xea0.
The cppgc documentation in garbage-collected.h explicitly requires:
"Must be inherited from as left-most base class."
GlobalShortcut violated this by placing Observer and DisposeObserver
before Wrappable. It was the only Electron API class with this
ordering — all others had gin::Wrappable as the first base by coincidence.
The fix
1. Reorder GlobalShortcut's base classes so gin::Wrappable is first:
class GlobalShortcut final
: public gin::Wrappable<GlobalShortcut>,
private ui::GlobalAcceleratorListener::Observer,
public gin::PerIsolateData::DisposeObserver {
This ensures the WrappableBase* stored in the CppHeapPointerTable
coincides with the cppgc allocation base, so FromObject() computes
the correct header.
2. Add a CPPGC_DCHECK in MakeGarbageCollectedTrait::Call() that
verifies GarbageCollected<U> is at offset zero in T:
CPPGC_DCHECK_MSG(
reinterpret_cast<void*>(object) ==
reinterpret_cast<void*>(
static_cast<ParentMostGarbageCollectedType*>(object)),
"GarbageCollected must be the left-most base of T, "
"otherwise incorrect HeapObjectHeader will be "
"computed for the object.");
This catches the violation at construction time in debug builds,
before it can manifest as a GC crash.
With the DCHECK we now get the following message at runtime during
object construction:
----- Native stack trace -----
1: 0x11bbd05fc node::NodePlatform::GetStackTracePrinter()::$_0::__invoke() [/Users/demohan/github/electron-oss/src/out/Testing/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
2: 0x123626290 V8_Fatal(char const*, int, char const*, ...) [/Users/demohan/github/electron-oss/src/out/Testing/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
3: 0x123625d84 v8::base::SetFatalFunction(void (*)(char const*, int, char const*)) [/Users/demohan/github/electron-oss/src/out/Testing/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
4: 0x11b7f2768 electron::api::GlobalShortcut* cppgc::MakeGarbageCollectedTrait<electron::api::GlobalShortcut>::Call<v8::Isolate*&>(cppgc::AllocationHandle&, v8::Isolate*&) [/Users/demohan/github/electron-oss/src/out/Testing/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
5: 0x11b7f063c electron::api::GlobalShortcut::Create(v8::Isolate*) [/Users/demohan/github/electron-oss/src/out/Testing/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
6: 0x11b7928e8 base::RepeatingCallback<v8::Local<v8::Value> (v8::Isolate*)>::Run(v8::Isolate*) const & [/Users/demohan/github/electron-oss/src/out/Testing/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
7: 0x11b7f159c void gin_helper::Invoker<std::__Cr::integer_sequence<unsigned long, 0ul>, v8::Isolate*>::DispatchToCallback<electron::api::GlobalShortcut*>(base::RepeatingCallback<electron::api::GlobalShortcut* (v8::Isolate*)>) [/Users/demohan/github/electron-oss/src/out/Testing/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
8: 0x11b7f1390 gin_helper::Dispatcher<electron::api::GlobalShortcut* (v8::Isolate*)>::DispatchToCallbackImpl(gin::Arguments*) [/Users/demohan/github/electron-oss/src/out/Testing/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
9: 0x11b7f128c gin_helper::Dispatcher<electron::api::GlobalShortcut* (v8::Isolate*)>::DispatchToCallback(v8::FunctionCallbackInfo<v8::Value> const&) [/Users/demohan/github/electron-oss/src/out/Testing/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
diff --git a/include/cppgc/allocation.h b/include/cppgc/allocation.h
index 450db00479e87a8a460289f7e717296f431c3c40..5762d737de9ef460b1a447a457dd373ac08e24e8 100644
--- a/include/cppgc/allocation.h
+++ b/include/cppgc/allocation.h
@@ -15,6 +15,7 @@
#include "cppgc/custom-space.h"
#include "cppgc/internal/api-constants.h"
#include "cppgc/internal/gc-info.h"
+#include "cppgc/internal/logging.h"
#include "cppgc/type-traits.h"
#include "v8config.h" // NOLINT(build/include_directory)
@@ -237,6 +238,18 @@ class MakeGarbageCollectedTrait : public MakeGarbageCollectedTraitBase<T> {
void* memory =
MakeGarbageCollectedTraitBase<T>::Allocate(handle, sizeof(T));
T* object = ::new (memory) T(std::forward<Args>(args)...);
+ // Verify GarbageCollected<U> is the left-most base class of T, as
+ // required by cppgc. If it isn't, HeapObjectHeader::FromObject() will
+ // compute the wrong header address for any pointer obtained via
+ // static_cast to a non-first base (e.g. through multiple inheritance).
+ CPPGC_DCHECK_MSG(
+ static_cast<const void*>(object) ==
+ static_cast<const void*>(static_cast<typename std::remove_const_t<
+ T>::ParentMostGarbageCollectedType*>(
+ const_cast<std::remove_const_t<T>*>(object))),
+ "GarbageCollected must be the left-most base of T, "
+ "otherwise incorrect HeapObjectHeader will be "
+ "computed for the object.");
MakeGarbageCollectedTraitBase<T>::MarkObjectAsFullyConstructed(object);
return object;
}
@@ -247,6 +260,14 @@ class MakeGarbageCollectedTrait : public MakeGarbageCollectedTraitBase<T> {
void* memory = MakeGarbageCollectedTraitBase<T>::Allocate(
handle, sizeof(T) + additional_bytes.value);
T* object = ::new (memory) T(std::forward<Args>(args)...);
+ CPPGC_DCHECK_MSG(
+ static_cast<const void*>(object) ==
+ static_cast<const void*>(static_cast<typename std::remove_const_t<
+ T>::ParentMostGarbageCollectedType*>(
+ const_cast<std::remove_const_t<T>*>(object))),
+ "GarbageCollected must be the left-most base of T, "
+ "otherwise incorrect HeapObjectHeader will be "
+ "computed for the object.");
MakeGarbageCollectedTraitBase<T>::MarkObjectAsFullyConstructed(object);
return object;
}

View File

@@ -15,14 +15,16 @@
#include "electron/shell/browser/electron_browser_context.h"
#include "electron/shell/common/electron_constants.h"
#include "extensions/common/command.h"
#include "gin/dictionary.h"
#include "gin/object_template_builder.h"
#include "gin/persistent.h"
#include "shell/browser/api/electron_api_system_preferences.h"
#include "shell/browser/browser.h"
#include "shell/common/gin_converters/accelerator_converter.h"
#include "shell/common/gin_converters/callback_converter.h"
#include "shell/common/gin_helper/handle.h"
#include "shell/common/gin_helper/dictionary.h"
#include "shell/common/node_includes.h"
#include "v8/include/cppgc/allocation.h"
#include "v8/include/v8-cppgc.h"
#if BUILDFLAG(IS_MAC)
#include "base/mac/mac_util.h"
@@ -50,41 +52,59 @@ bool MapHasMediaKeys(
namespace electron::api {
gin::DeprecatedWrapperInfo GlobalShortcut::kWrapperInfo = {
gin::kEmbedderNativeGin};
const gin::WrapperInfo GlobalShortcut::kWrapperInfo = {
{gin::kEmbedderNativeGin},
gin::kElectronGlobalShortcut};
GlobalShortcut::GlobalShortcut() = default;
GlobalShortcut::GlobalShortcut(v8::Isolate* isolate) {
gin::PerIsolateData* data = gin::PerIsolateData::From(isolate);
data->AddDisposeObserver(this);
}
GlobalShortcut::~GlobalShortcut() {
Dispose();
}
void GlobalShortcut::Dispose() {
if (is_disposed_)
return;
is_disposed_ = true;
auto* instance = ui::GlobalAcceleratorListener::GetInstance();
if (instance && instance->IsRegistrationHandledExternally()) {
// Eagerly cancel callbacks so PruneStaleCommands() can clear them before
// the WeakPtrFactory destructor runs.
weak_ptr_factory_.InvalidateWeakPtrs();
weak_factory_.Invalidate();
instance->PruneStaleCommands();
}
UnregisterAll();
UnregisterAllInternal();
}
void GlobalShortcut::OnKeyPressed(const ui::Accelerator& accelerator) {
if (auto* cb = base::FindOrNull(accelerator_callback_map_, accelerator)) {
cb->Run();
auto callback = *cb;
callback.Run();
} else {
// This should never occur, because if it does,
// ui::GlobalAcceleratorListener notifies us with wrong accelerator.
NOTREACHED();
if (!is_disposed_) {
NOTREACHED();
}
}
}
void GlobalShortcut::ExecuteCommand(const extensions::ExtensionId& extension_id,
const std::string& command_id) {
if (auto* cb = base::FindOrNull(command_callback_map_, command_id)) {
cb->Run();
auto callback = *cb;
callback.Run();
} else {
// This should never occur, because if it does, GlobalAcceleratorListener
// notifies us with wrong command.
NOTREACHED();
if (!is_disposed_) {
NOTREACHED();
}
}
}
@@ -112,11 +132,14 @@ bool GlobalShortcut::RegisterAll(
bool GlobalShortcut::Register(const ui::Accelerator& accelerator,
const base::RepeatingClosure& callback) {
v8::Isolate* const isolate = JavascriptEnvironment::GetIsolate();
if (!electron::Browser::Get()->is_ready()) {
gin_helper::ErrorThrower(JavascriptEnvironment::GetIsolate())
.ThrowError("globalShortcut cannot be used before the app is ready");
gin_helper::ErrorThrower(isolate).ThrowError(
"globalShortcut cannot be used before the app is ready");
return false;
}
#if BUILDFLAG(IS_MAC)
if (accelerator.IsMediaKey()) {
if (RegisteringMediaKeyForUntrustedClient(accelerator))
@@ -170,8 +193,10 @@ bool GlobalShortcut::Register(const ui::Accelerator& accelerator,
const std::string fake_extension_id = command_str + "+" + profile_id;
instance->OnCommandsChanged(
fake_extension_id, profile_id, commands, gfx::kNullAcceleratedWidget,
base::BindRepeating(&GlobalShortcut::ExecuteCommand,
weak_ptr_factory_.GetWeakPtr()));
base::BindRepeating(
&GlobalShortcut::ExecuteCommand,
gin::WrapPersistent(weak_factory_.GetWeakCell(
isolate->GetCppHeap()->GetAllocationHandle()))));
command_callback_map_[command_str] = callback;
return true;
} else {
@@ -189,19 +214,24 @@ void GlobalShortcut::Unregister(const ui::Accelerator& accelerator) {
.ThrowError("globalShortcut cannot be used before the app is ready");
return;
}
if (accelerator_callback_map_.erase(accelerator) == 0)
if (!accelerator_callback_map_.contains(accelerator))
return;
if (ui::GlobalAcceleratorListener::GetInstance()) {
ui::GlobalAcceleratorListener::GetInstance()->UnregisterAccelerator(
accelerator, this);
}
// Remove from local callback map after unregistering from UI listener to
// avoid reentrancy races where in-flight key notifications arrive while the
// platform listener is stopping.
accelerator_callback_map_.erase(accelerator);
#if BUILDFLAG(IS_MAC)
if (accelerator.IsMediaKey() && !MapHasMediaKeys(accelerator_callback_map_)) {
ui::GlobalAcceleratorListener::SetShouldUseInternalMediaKeyHandling(true);
}
#endif
if (ui::GlobalAcceleratorListener::GetInstance()) {
ui::GlobalAcceleratorListener::GetInstance()->UnregisterAccelerator(
accelerator, this);
}
}
void GlobalShortcut::UnregisterSome(
@@ -226,23 +256,27 @@ void GlobalShortcut::UnregisterAll() {
.ThrowError("globalShortcut cannot be used before the app is ready");
return;
}
accelerator_callback_map_.clear();
UnregisterAllInternal();
}
void GlobalShortcut::UnregisterAllInternal() {
if (ui::GlobalAcceleratorListener::GetInstance()) {
ui::GlobalAcceleratorListener::GetInstance()->UnregisterAccelerators(this);
}
accelerator_callback_map_.clear();
command_callback_map_.clear();
}
// static
gin_helper::Handle<GlobalShortcut> GlobalShortcut::Create(
v8::Isolate* isolate) {
return gin_helper::CreateHandle(isolate, new GlobalShortcut());
GlobalShortcut* GlobalShortcut::Create(v8::Isolate* isolate) {
return cppgc::MakeGarbageCollected<GlobalShortcut>(
isolate->GetCppHeap()->GetAllocationHandle(), isolate);
}
// static
gin::ObjectTemplateBuilder GlobalShortcut::GetObjectTemplateBuilder(
v8::Isolate* isolate) {
return gin_helper::DeprecatedWrappable<
GlobalShortcut>::GetObjectTemplateBuilder(isolate)
return gin::Wrappable<GlobalShortcut>::GetObjectTemplateBuilder(isolate)
.SetMethod("registerAll", &GlobalShortcut::RegisterAll)
.SetMethod("register", &GlobalShortcut::Register)
.SetMethod("isRegistered", &GlobalShortcut::IsRegistered)
@@ -250,8 +284,24 @@ gin::ObjectTemplateBuilder GlobalShortcut::GetObjectTemplateBuilder(
.SetMethod("unregisterAll", &GlobalShortcut::UnregisterAll);
}
const char* GlobalShortcut::GetTypeName() {
return "GlobalShortcut";
void GlobalShortcut::Trace(cppgc::Visitor* visitor) const {
gin::Wrappable<GlobalShortcut>::Trace(visitor);
visitor->Trace(weak_factory_);
}
const gin::WrapperInfo* GlobalShortcut::wrapper_info() const {
return &kWrapperInfo;
}
const char* GlobalShortcut::GetHumanReadableName() const {
return "Electron / GlobalShortcut";
}
void GlobalShortcut::OnBeforeMicrotasksRunnerDispose(v8::Isolate* isolate) {
gin::PerIsolateData* data = gin::PerIsolateData::From(isolate);
data->RemoveDisposeObserver(this);
Dispose();
keep_alive_.Clear();
}
} // namespace electron::api
@@ -263,8 +313,9 @@ void Initialize(v8::Local<v8::Object> exports,
v8::Local<v8::Context> context,
void* priv) {
v8::Isolate* const isolate = electron::JavascriptEnvironment::GetIsolate();
gin::Dictionary dict{isolate, exports};
dict.Set("globalShortcut", electron::api::GlobalShortcut::Create(isolate));
gin_helper::Dictionary dict{isolate, exports};
dict.SetMethod("createGlobalShortcut",
base::BindRepeating(&electron::api::GlobalShortcut::Create));
}
} // namespace

View File

@@ -11,42 +11,48 @@
#include "base/functional/callback_forward.h"
#include "base/memory/weak_ptr.h"
#include "extensions/common/extension_id.h"
#include "shell/common/gin_helper/wrappable.h"
#include "gin/per_isolate_data.h"
#include "gin/weak_cell.h"
#include "gin/wrappable.h"
#include "shell/common/gin_helper/self_keep_alive.h"
#include "ui/base/accelerators/accelerator.h"
#include "ui/base/accelerators/global_accelerator_listener/global_accelerator_listener.h"
namespace gin_helper {
template <typename T>
class Handle;
} // namespace gin_helper
namespace electron::api {
class GlobalShortcut final
: private ui::GlobalAcceleratorListener::Observer,
public gin_helper::DeprecatedWrappable<GlobalShortcut> {
class GlobalShortcut final : public gin::Wrappable<GlobalShortcut>,
private ui::GlobalAcceleratorListener::Observer,
public gin::PerIsolateData::DisposeObserver {
public:
static gin_helper::Handle<GlobalShortcut> Create(v8::Isolate* isolate);
static GlobalShortcut* Create(v8::Isolate* isolate);
// gin_helper::Wrappable
static gin::DeprecatedWrapperInfo kWrapperInfo;
// gin::Wrappable
static const gin::WrapperInfo kWrapperInfo;
void Trace(cppgc::Visitor* visitor) const override;
const gin::WrapperInfo* wrapper_info() const override;
const char* GetHumanReadableName() const override;
gin::ObjectTemplateBuilder GetObjectTemplateBuilder(
v8::Isolate* isolate) override;
const char* GetTypeName() override;
// gin::PerIsolateData::DisposeObserver
void OnBeforeDispose(v8::Isolate* isolate) override {}
void OnBeforeMicrotasksRunnerDispose(v8::Isolate* isolate) override;
void OnDisposed() override {}
// Make public for cppgc::MakeGarbageCollected.
explicit GlobalShortcut(v8::Isolate* isolate);
~GlobalShortcut() override;
// disable copy
GlobalShortcut(const GlobalShortcut&) = delete;
GlobalShortcut& operator=(const GlobalShortcut&) = delete;
protected:
GlobalShortcut();
~GlobalShortcut() override;
private:
typedef std::map<ui::Accelerator, base::RepeatingClosure>
AcceleratorCallbackMap;
typedef std::map<std::string, base::RepeatingClosure> CommandCallbackMap;
void Dispose();
bool RegisterAll(const std::vector<ui::Accelerator>& accelerators,
const base::RepeatingClosure& callback);
bool Register(const ui::Accelerator& accelerator,
@@ -55,6 +61,7 @@ class GlobalShortcut final
void Unregister(const ui::Accelerator& accelerator);
void UnregisterSome(const std::vector<ui::Accelerator>& accelerators);
void UnregisterAll();
void UnregisterAllInternal();
// GlobalAcceleratorListener::Observer implementation.
void OnKeyPressed(const ui::Accelerator& accelerator) override;
@@ -63,8 +70,11 @@ class GlobalShortcut final
AcceleratorCallbackMap accelerator_callback_map_;
CommandCallbackMap command_callback_map_;
bool is_disposed_ = false;
base::WeakPtrFactory<GlobalShortcut> weak_ptr_factory_{this};
gin::WeakCellFactory<GlobalShortcut> weak_factory_{this};
gin_helper::SelfKeepAlive<GlobalShortcut> keep_alive_{this};
};
} // namespace electron::api

View File

@@ -54,17 +54,9 @@ gin_helper::Handle<SafeStorage> SafeStorage::Create(v8::Isolate* isolate) {
return gin_helper::CreateHandle(isolate, new SafeStorage(isolate));
}
SafeStorage::SafeStorage(v8::Isolate* isolate) {
if (electron::Browser::Get()->is_ready()) {
OnFinishLaunching({});
} else {
Browser::Get()->AddObserver(this);
}
}
SafeStorage::SafeStorage(v8::Isolate* isolate) {}
SafeStorage::~SafeStorage() {
Browser::Get()->RemoveObserver(this);
}
SafeStorage::~SafeStorage() = default;
gin::ObjectTemplateBuilder SafeStorage::GetObjectTemplateBuilder(
v8::Isolate* isolate) {
@@ -85,7 +77,11 @@ gin::ObjectTemplateBuilder SafeStorage::GetObjectTemplateBuilder(
;
}
void SafeStorage::OnFinishLaunching(base::DictValue launch_info) {
void SafeStorage::EnsureAsyncEncryptorRequested() {
DCHECK(electron::Browser::Get()->is_ready());
if (encryptor_requested_)
return;
encryptor_requested_ = true;
g_browser_process->os_crypt_async()->GetInstance(
base::BindOnce(&SafeStorage::OnOsCryptReady, base::Unretained(this)),
os_crypt_async::Encryptor::Option::kEncryptSyncCompat);
@@ -95,13 +91,21 @@ void SafeStorage::OnOsCryptReady(os_crypt_async::Encryptor encryptor) {
encryptor_ = std::move(encryptor);
is_available_ = true;
// This callback may fire from a posted task without an active V8 scope.
v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
v8::HandleScope handle_scope(isolate);
for (auto& pending : pending_availability_checks_) {
pending.Resolve(true);
}
pending_availability_checks_.clear();
for (auto& pending : pending_encrypts_) {
std::string ciphertext;
bool encrypted = encryptor_->EncryptString(pending.plaintext, &ciphertext);
if (encrypted) {
pending.promise.Resolve(
electron::Buffer::Copy(pending.promise.isolate(), ciphertext)
.ToLocalChecked());
electron::Buffer::Copy(isolate, ciphertext).ToLocalChecked());
} else {
pending.promise.RejectWithErrorMessage(
"Error while encrypting the text provided to "
@@ -117,8 +121,6 @@ void SafeStorage::OnOsCryptReady(os_crypt_async::Encryptor encryptor) {
encryptor_->DecryptString(pending.ciphertext, &plaintext, &flags);
if (decrypted) {
v8::Isolate* isolate = pending.promise.isolate();
v8::HandleScope handle_scope(isolate);
auto dict = gin_helper::Dictionary::CreateEmpty(isolate);
dict.Set("shouldReEncrypt", flags.should_reencrypt);
@@ -155,16 +157,33 @@ bool SafeStorage::IsEncryptionAvailable() {
#endif
}
bool SafeStorage::IsAsyncEncryptionAvailable() {
if (!electron::Browser::Get()->is_ready())
return false;
v8::Local<v8::Promise> SafeStorage::IsAsyncEncryptionAvailable(
v8::Isolate* isolate) {
gin_helper::Promise<bool> promise(isolate);
v8::Local<v8::Promise> handle = promise.GetHandle();
if (!electron::Browser::Get()->is_ready()) {
promise.Resolve(false);
return handle;
}
#if BUILDFLAG(IS_LINUX)
return is_available_ || (use_password_v10_ &&
static_cast<BrowserProcessImpl*>(g_browser_process)
->linux_storage_backend() == "basic_text");
#else
return is_available_;
if (use_password_v10_ && static_cast<BrowserProcessImpl*>(g_browser_process)
->linux_storage_backend() == "basic_text") {
promise.Resolve(true);
return handle;
}
#endif
EnsureAsyncEncryptorRequested();
if (is_available_) {
promise.Resolve(true);
return handle;
}
pending_availability_checks_.push_back(std::move(promise));
return handle;
}
void SafeStorage::SetUsePasswordV10(bool use) {
@@ -270,6 +289,8 @@ v8::Local<v8::Promise> SafeStorage::encryptStringAsync(
return handle;
}
EnsureAsyncEncryptorRequested();
if (is_available_) {
std::string ciphertext;
bool encrypted = encryptor_->EncryptString(plaintext, &ciphertext);
@@ -318,6 +339,8 @@ v8::Local<v8::Promise> SafeStorage::decryptStringAsync(
return handle;
}
EnsureAsyncEncryptorRequested();
if (is_available_) {
std::string plaintext;
os_crypt_async::Encryptor::DecryptFlags flags;

View File

@@ -10,7 +10,6 @@
#include "build/build_config.h"
#include "components/os_crypt/async/common/encryptor.h"
#include "shell/browser/browser_observer.h"
#include "shell/browser/event_emitter_mixin.h"
#include "shell/common/gin_helper/dictionary.h"
#include "shell/common/gin_helper/promise.h"
@@ -37,8 +36,7 @@ class Handle;
namespace electron::api {
class SafeStorage final : public gin_helper::DeprecatedWrappable<SafeStorage>,
public gin_helper::EventEmitterMixin<SafeStorage>,
private BrowserObserver {
public gin_helper::EventEmitterMixin<SafeStorage> {
public:
static gin_helper::Handle<SafeStorage> Create(v8::Isolate* isolate);
@@ -57,14 +55,16 @@ class SafeStorage final : public gin_helper::DeprecatedWrappable<SafeStorage>,
~SafeStorage() override;
private:
// BrowserObserver:
void OnFinishLaunching(base::DictValue launch_info) override;
// Lazily request the async encryptor on first use. ESM named imports
// eagerly evaluate all electron module getters, so requesting in the
// constructor would touch the OS keychain even when safeStorage is unused.
void EnsureAsyncEncryptorRequested();
void OnOsCryptReady(os_crypt_async::Encryptor encryptor);
bool IsEncryptionAvailable();
bool IsAsyncEncryptionAvailable();
v8::Local<v8::Promise> IsAsyncEncryptionAvailable(v8::Isolate* isolate);
void SetUsePasswordV10(bool use);
@@ -85,6 +85,7 @@ class SafeStorage final : public gin_helper::DeprecatedWrappable<SafeStorage>,
bool use_password_v10_ = false;
bool encryptor_requested_ = false;
bool is_available_ = false;
std::optional<os_crypt_async::Encryptor> encryptor_;
@@ -114,6 +115,8 @@ class SafeStorage final : public gin_helper::DeprecatedWrappable<SafeStorage>,
std::string ciphertext;
};
std::vector<PendingDecrypt> pending_decrypts_;
std::vector<gin_helper::Promise<bool>> pending_availability_checks_;
};
} // namespace electron::api

View File

@@ -81,8 +81,8 @@ describe('safeStorage module', () => {
});
describe('SafeStorage.isAsyncEncryptionAvailable()', () => {
it('should return true when async encryption is available', () => {
expect(safeStorage.isAsyncEncryptionAvailable()).to.equal(true);
it('should resolve true when async encryption is available', async () => {
expect(await safeStorage.isAsyncEncryptionAvailable()).to.equal(true);
});
});

View File

@@ -242,7 +242,7 @@ declare namespace NodeJS {
_linkedBinding(name: 'electron_browser_crash_reporter'): CrashReporterBinding;
_linkedBinding(name: 'electron_browser_desktop_capturer'): { createDesktopCapturer(): ElectronInternal.DesktopCapturer; isDisplayMediaSystemPickerAvailable(): boolean; };
_linkedBinding(name: 'electron_browser_event_emitter'): { setEventEmitterPrototype(prototype: Object): void; };
_linkedBinding(name: 'electron_browser_global_shortcut'): { globalShortcut: Electron.GlobalShortcut };
_linkedBinding(name: 'electron_browser_global_shortcut'): { createGlobalShortcut(): Electron.GlobalShortcut };
_linkedBinding(name: 'electron_browser_image_view'): { ImageView: any };
_linkedBinding(name: 'electron_browser_in_app_purchase'): { inAppPurchase: Electron.InAppPurchase };
_linkedBinding(name: 'electron_browser_message_port'): { createPair(): { port1: Electron.MessagePortMain, port2: Electron.MessagePortMain }; };