mirror of
https://github.com/lightpanda-io/browser.git
synced 2026-03-22 04:34:44 +00:00
Use a single HandleScope for event dispatch
https://github.com/lightpanda-io/browser/pull/1690 narrowed the lifetime of HandleScopes to once per listener. I think that was just an accident of refactoring, and not some intentional choice. The narrower HandleScope lifetime makes it so that when we do run microtask queue at the end of event dispatching, some locals in the queue may not longer be valid. HS1 HS2 queueMicrotask(func) runMicrotask In the above flow, `func` is only valid while HS2 is alive, so when we run the microtask queue in HS1, it is no longer valid.
This commit is contained in:
@@ -377,12 +377,17 @@ fn dispatchNode(self: *EventManager, target: *Node, event: *Event, comptime opts
|
|||||||
const page = self.page;
|
const page = self.page;
|
||||||
var was_handled = false;
|
var was_handled = false;
|
||||||
|
|
||||||
defer if (was_handled) {
|
// Create a single scope for all event handlers in this dispatch.
|
||||||
var ls: js.Local.Scope = undefined;
|
// This ensures function handles passed to queueMicrotask remain valid
|
||||||
page.js.localScope(&ls);
|
// throughout the entire dispatch, preventing crashes when microtasks run.
|
||||||
defer ls.deinit();
|
var ls: js.Local.Scope = undefined;
|
||||||
ls.local.runMicrotasks();
|
page.js.localScope(&ls);
|
||||||
};
|
defer {
|
||||||
|
if (was_handled) {
|
||||||
|
ls.local.runMicrotasks();
|
||||||
|
}
|
||||||
|
ls.deinit();
|
||||||
|
}
|
||||||
|
|
||||||
const activation_state = ActivationState.create(event, target, page);
|
const activation_state = ActivationState.create(event, target, page);
|
||||||
|
|
||||||
@@ -461,7 +466,7 @@ fn dispatchNode(self: *EventManager, target: *Node, event: *Event, comptime opts
|
|||||||
.event_target = @intFromPtr(current_target),
|
.event_target = @intFromPtr(current_target),
|
||||||
.type_string = event._type_string,
|
.type_string = event._type_string,
|
||||||
})) |list| {
|
})) |list| {
|
||||||
try self.dispatchPhase(list, current_target, event, &was_handled, comptime .init(true, opts));
|
try self.dispatchPhase(list, current_target, event, &was_handled, &ls.local, comptime .init(true, opts));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -476,10 +481,6 @@ fn dispatchNode(self: *EventManager, target: *Node, event: *Event, comptime opts
|
|||||||
was_handled = true;
|
was_handled = true;
|
||||||
event._current_target = target_et;
|
event._current_target = target_et;
|
||||||
|
|
||||||
var ls: js.Local.Scope = undefined;
|
|
||||||
self.page.js.localScope(&ls);
|
|
||||||
defer ls.deinit();
|
|
||||||
|
|
||||||
try ls.toLocal(inline_handler).callWithThis(void, target_et, .{event});
|
try ls.toLocal(inline_handler).callWithThis(void, target_et, .{event});
|
||||||
|
|
||||||
if (event._stop_propagation) {
|
if (event._stop_propagation) {
|
||||||
@@ -495,7 +496,7 @@ fn dispatchNode(self: *EventManager, target: *Node, event: *Event, comptime opts
|
|||||||
.type_string = event._type_string,
|
.type_string = event._type_string,
|
||||||
.event_target = @intFromPtr(target_et),
|
.event_target = @intFromPtr(target_et),
|
||||||
})) |list| {
|
})) |list| {
|
||||||
try self.dispatchPhase(list, target_et, event, &was_handled, comptime .init(null, opts));
|
try self.dispatchPhase(list, target_et, event, &was_handled, &ls.local, comptime .init(null, opts));
|
||||||
if (event._stop_propagation) {
|
if (event._stop_propagation) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
@@ -512,7 +513,7 @@ fn dispatchNode(self: *EventManager, target: *Node, event: *Event, comptime opts
|
|||||||
.type_string = event._type_string,
|
.type_string = event._type_string,
|
||||||
.event_target = @intFromPtr(current_target),
|
.event_target = @intFromPtr(current_target),
|
||||||
})) |list| {
|
})) |list| {
|
||||||
try self.dispatchPhase(list, current_target, event, &was_handled, comptime .init(false, opts));
|
try self.dispatchPhase(list, current_target, event, &was_handled, &ls.local, comptime .init(false, opts));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -530,7 +531,7 @@ const DispatchPhaseOpts = struct {
|
|||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
fn dispatchPhase(self: *EventManager, list: *std.DoublyLinkedList, current_target: *EventTarget, event: *Event, was_handled: *bool, comptime opts: DispatchPhaseOpts) !void {
|
fn dispatchPhase(self: *EventManager, list: *std.DoublyLinkedList, current_target: *EventTarget, event: *Event, was_handled: *bool, local: *const js.Local, comptime opts: DispatchPhaseOpts) !void {
|
||||||
const page = self.page;
|
const page = self.page;
|
||||||
|
|
||||||
// Track dispatch depth for deferred removal
|
// Track dispatch depth for deferred removal
|
||||||
@@ -607,18 +608,14 @@ fn dispatchPhase(self: *EventManager, list: *std.DoublyLinkedList, current_targe
|
|||||||
event._target = getAdjustedTarget(original_target, current_target);
|
event._target = getAdjustedTarget(original_target, current_target);
|
||||||
}
|
}
|
||||||
|
|
||||||
var ls: js.Local.Scope = undefined;
|
|
||||||
page.js.localScope(&ls);
|
|
||||||
defer ls.deinit();
|
|
||||||
|
|
||||||
switch (listener.function) {
|
switch (listener.function) {
|
||||||
.value => |value| try ls.toLocal(value).callWithThis(void, current_target, .{event}),
|
.value => |value| try local.toLocal(value).callWithThis(void, current_target, .{event}),
|
||||||
.string => |string| {
|
.string => |string| {
|
||||||
const str = try page.call_arena.dupeZ(u8, string.str());
|
const str = try page.call_arena.dupeZ(u8, string.str());
|
||||||
try ls.local.eval(str, null);
|
try local.eval(str, null);
|
||||||
},
|
},
|
||||||
.object => |obj_global| {
|
.object => |obj_global| {
|
||||||
const obj = ls.toLocal(obj_global);
|
const obj = local.toLocal(obj_global);
|
||||||
if (try obj.getFunction("handleEvent")) |handleEvent| {
|
if (try obj.getFunction("handleEvent")) |handleEvent| {
|
||||||
try handleEvent.callWithThis(void, obj, .{event});
|
try handleEvent.callWithThis(void, obj, .{event});
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1007,6 +1007,13 @@ fn enqueueMicrotask(self: *Context, callback: anytype) void {
|
|||||||
}.run, self);
|
}.run, self);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// There's an assumption here: the js.Function will be alive when microtasks are
|
||||||
|
// run. If we're Env.runMicrotasks in all the places that we're supposed to, then
|
||||||
|
// this should be safe (I think). In whatever HandleScope a microtask is enqueued,
|
||||||
|
// PerformCheckpoint should be run. So the v8::Local<v8::Function> should remain
|
||||||
|
// valid. If we have problems with this, a simple solution is to provide a Zig
|
||||||
|
// wrapper for these callbacks which references a js.Function.Temp, on callback
|
||||||
|
// it executes the function and then releases the global.
|
||||||
pub fn queueMicrotaskFunc(self: *Context, cb: js.Function) void {
|
pub fn queueMicrotaskFunc(self: *Context, cb: js.Function) void {
|
||||||
// Use context-specific microtask queue instead of isolate queue
|
// Use context-specific microtask queue instead of isolate queue
|
||||||
v8.v8__MicrotaskQueue__EnqueueMicrotaskFunc(self.microtask_queue, self.isolate.handle, cb.handle);
|
v8.v8__MicrotaskQueue__EnqueueMicrotaskFunc(self.microtask_queue, self.isolate.handle, cb.handle);
|
||||||
|
|||||||
Reference in New Issue
Block a user