Terminate execution on internal navigation

Currently, if there's an internal navigation event, we continue to process the
page normally. This has negative performance implication, and can result in
user-scripts producing unexpected results.

For example, imagine a page with a script that does.

```js
if (x) {
   form.submit();
}

reloadProduct();
```

The call to `form.submit()` should stop the script from executing. And, if the
page has 10 other <script> tags after this, they shouldn't be loaded nor
executed.

This code terminates the execution of the current script on an internal
navigation event, and stops the rest of the page from load.

While I believe this creates a more "correct" behavior, it also introduces new
edge cases. There's no a period of time, between the termination being stopped
and then being resumed, where executing code is not safe.
This commit is contained in:
Karl Seguin
2025-06-12 16:26:27 +08:00
parent 97c769e805
commit b1ca242d89
2 changed files with 77 additions and 32 deletions

View File

@@ -388,11 +388,15 @@ pub const Page = struct {
// > immediately before the browser continues to parse the // > immediately before the browser continues to parse the
// > page. // > page.
// https://developer.mozilla.org/en-US/docs/Web/HTML/Element/script#notes // https://developer.mozilla.org/en-US/docs/Web/HTML/Element/script#notes
self.evalScript(&script); if (self.evalScript(&script) == false) {
return;
}
} }
for (defer_scripts.items) |*script| { for (defer_scripts.items) |*script| {
self.evalScript(script); if (self.evalScript(script) == false) {
return;
}
} }
// dispatch DOMContentLoaded before the transition to "complete", // dispatch DOMContentLoaded before the transition to "complete",
// at the point where all subresources apart from async script elements // at the point where all subresources apart from async script elements
@@ -402,7 +406,9 @@ pub const Page = struct {
// eval async scripts. // eval async scripts.
for (async_scripts.items) |*script| { for (async_scripts.items) |*script| {
self.evalScript(script); if (self.evalScript(script) == false) {
return;
}
} }
try HTMLDocument.documentIsComplete(html_doc, self); try HTMLDocument.documentIsComplete(html_doc, self);
@@ -419,10 +425,13 @@ pub const Page = struct {
); );
} }
fn evalScript(self: *Page, script: *const Script) void { fn evalScript(self: *Page, script: *const Script) bool {
self.tryEvalScript(script) catch |err| { self.tryEvalScript(script) catch |err| switch (err) {
log.err(.js, "eval script error", .{ .err = err, .src = script.src }); error.JsErr => {}, // already been logged with detail
error.Terminated => return false,
else => log.err(.js, "eval script error", .{ .err = err, .src = script.src }),
}; };
return true;
} }
// evalScript evaluates the src in priority. // evalScript evaluates the src in priority.
@@ -436,29 +445,26 @@ pub const Page = struct {
log.err(.browser, "clear document script", .{ .err = err }); log.err(.browser, "clear document script", .{ .err = err });
}; };
const src = script.src orelse { var script_source: ?[]const u8 = null;
if (script.src) |src| {
self.current_script = script;
defer self.current_script = null;
// https://html.spec.whatwg.org/multipage/webappapis.html#fetch-a-classic-script
script_source = (try self.fetchData(src, null)) orelse {
// TODO If el's result is null, then fire an event named error at
// el, and return
return;
};
} else {
// source is inline // source is inline
// TODO handle charset attribute // TODO handle charset attribute
if (try parser.nodeTextContent(parser.elementToNode(script.element))) |text| { script_source = try parser.nodeTextContent(parser.elementToNode(script.element));
try script.eval(self, text); }
}
return;
};
self.current_script = script; if (script_source) |ss| {
defer self.current_script = null; try script.eval(self, ss);
}
// https://html.spec.whatwg.org/multipage/webappapis.html#fetch-a-classic-script
const body = (try self.fetchData(src, null)) orelse {
// TODO If el's result is null, then fire an event named error at
// el, and return
return;
};
script.eval(self, body) catch |err| switch (err) {
error.JsErr => {}, // nothing to do here.
else => return err,
};
// TODO If el's from an external file is true, then fire an event // TODO If el's from an external file is true, then fire an event
// named load at el. // named load at el.
@@ -701,13 +707,18 @@ pub const Page = struct {
.reason = opts.reason, .reason = opts.reason,
}); });
self.delayed_navigation = true; self.delayed_navigation = true;
const arena = self.session.transfer_arena;
const session = self.session;
const arena = session.transfer_arena;
const navi = try arena.create(DelayedNavigation); const navi = try arena.create(DelayedNavigation);
navi.* = .{ navi.* = .{
.opts = opts, .opts = opts,
.session = self.session, .session = session,
.url = try self.url.resolve(arena, url), .url = try self.url.resolve(arena, url),
}; };
// In v8, this throws an exception which JS code cannot catch.
session.executor.terminateExecution();
_ = try self.loop.timeout(0, &navi.navigate_node); _ = try self.loop.timeout(0, &navi.navigate_node);
} }
@@ -822,6 +833,11 @@ const DelayedNavigation = struct {
const initial = self.initial; const initial = self.initial;
if (initial) { if (initial) {
// Prior to schedule this task, we terminated excution to stop
// the running script. If we don't resume it before doing a shutdown
// we'll get an error.
session.executor.resumeExecution();
session.removePage(); session.removePage();
_ = session.createPage() catch |err| { _ = session.createPage() catch |err| {
log.err(.browser, "delayed navigation page error", .{ log.err(.browser, "delayed navigation page error", .{
@@ -985,9 +1001,17 @@ const Script = struct {
} }
}, },
} catch { } catch {
if (try try_catch.err(page.arena)) |msg| { if (page.delayed_navigation) {
log.warn(.user_script, "eval script", .{ .src = src, .err = msg }); return error.Terminated;
} }
if (try try_catch.err(page.arena)) |msg| {
log.warn(.user_script, "eval script", .{
.src = src,
.err = msg,
});
}
try self.executeCallback("onerror", page); try self.executeCallback("onerror", page);
return error.JsErr; return error.JsErr;
}; };
@@ -1060,10 +1084,15 @@ fn timestamp() u32 {
// immediately. // immediately.
pub export fn scriptAddedCallback(ctx: ?*anyopaque, element: ?*parser.Element) callconv(.C) void { pub export fn scriptAddedCallback(ctx: ?*anyopaque, element: ?*parser.Element) callconv(.C) void {
const self: *Page = @alignCast(@ptrCast(ctx.?)); const self: *Page = @alignCast(@ptrCast(ctx.?));
if (self.delayed_navigation) {
// if we're planning on navigating to another page, don't run this script
return;
}
var script = Script.init(element.?, self) catch |err| { var script = Script.init(element.?, self) catch |err| {
log.warn(.browser, "script added init error", .{ .err = err }); log.warn(.browser, "script added init error", .{ .err = err });
return; return;
} orelse return; } orelse return;
self.evalScript(&script); _ = self.evalScript(&script);
} }

View File

@@ -472,6 +472,14 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
self.scope = null; self.scope = null;
_ = self.scope_arena.reset(.{ .retain_with_limit = SCOPE_ARENA_RETAIN }); _ = self.scope_arena.reset(.{ .retain_with_limit = SCOPE_ARENA_RETAIN });
} }
pub fn terminateExecution(self: *const ExecutionWorld) void {
self.env.isolate.terminateExecution();
}
pub fn resumeExecution(self: *const ExecutionWorld) void {
self.env.isolate.cancelTerminateExecution();
}
}; };
const PersistentObject = v8.Persistent(v8.Object); const PersistentObject = v8.Persistent(v8.Object);
@@ -1562,7 +1570,15 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
} }
pub fn send(self: *const Inspector, msg: []const u8) void { pub fn send(self: *const Inspector, msg: []const u8) void {
self.session.dispatchProtocolMessage(self.isolate, msg); // Can't assume there's an existing Scope (with its HandleScope)
// available when doing this. Pages (and thus Scopes) come and
// go, but CDP can keep sending messages.
const isolate = self.isolate;
var temp_scope: v8.HandleScope = undefined;
v8.HandleScope.init(&temp_scope, isolate);
defer temp_scope.deinit();
self.session.dispatchProtocolMessage(isolate, msg);
} }
// From CDP docs // From CDP docs