Remove the single-blocking-import restrictions

Remove the is_blocking variable (and checks) from the ScriptManager. This is a
holdover to when blocking imports had a dedicated connections, and thus couldn't
be loaded concurrently.

This changes two obvious things (and probably a few subtle ones). The first is
that plain async scripts are now free to be executed as soon as they are
completed. As far as I can tell, this is a safe behavior, is simpler and should
be a bit faster.

More importantly, it allows for chains of imports. normal import (A) -> dynamic
import (B) -> normal import (C). This would previously fail an assertion. The
superficial issue was that dynamic import handling didn't respect the
`is_blocking` flag. But if we did respect it, than module B would never be able
to load module C, which would block module A from ever completing. By removing
the flag, module C will now be properly evaluated, which unblocks module B which
allows module A to unblock.

(1) I believe this is the issue seen here:
    https://github.com/lightpanda-io/browser/issues/1121
This commit is contained in:
Karl Seguin
2025-10-06 09:48:57 +08:00
parent 432e3c3a5e
commit ef363209a4

View File

@@ -38,9 +38,6 @@ page: *Page,
// used to prevent recursive evalutaion
is_evaluating: bool,
// used to prevent executing scripts while we're doing a blocking load
is_blocking: bool = false,
// Only once this is true can deferred scripts be run
static_scripts_done: bool,
@@ -48,12 +45,6 @@ static_scripts_done: bool,
// on shutdown/abort, we need to cleanup any pending ones.
asyncs: OrderList,
// When an async script is ready to be evaluated, it's moved from asyncs to
// this list. You might think we can evaluate an async script as soon as it's
// done, but we can only evaluate scripts when `is_blocking == false`. So this
// becomes a list of scripts to execute on the next evaluate().
asyncs_ready: OrderList,
// Normal scripts (non-deferred & non-async). These must be executed in order
scripts: OrderList,
@@ -89,7 +80,6 @@ pub fn init(browser: *Browser, page: *Page) ScriptManager {
.asyncs = .{},
.scripts = .{},
.deferreds = .{},
.asyncs_ready = .{},
.sync_modules = .empty,
.is_evaluating = false,
.allocator = allocator,
@@ -129,7 +119,6 @@ pub fn reset(self: *ScriptManager) void {
self.clearList(&self.asyncs);
self.clearList(&self.scripts);
self.clearList(&self.deferreds);
self.clearList(&self.asyncs_ready);
self.static_scripts_done = false;
}
@@ -321,10 +310,6 @@ pub fn getModule(self: *ScriptManager, url: [:0]const u8) !void {
}
pub fn waitForModule(self: *ScriptManager, url: [:0]const u8) !GetResult {
std.debug.assert(self.is_blocking == false);
self.is_blocking = true;
defer self.is_blocking = false;
// Normally it's dangerous to hold on to map pointers. But here, the map
// can't change. It's possible that by calling `tick`, other entries within
// the map will have their value change, but the map itself is immutable
@@ -398,24 +383,10 @@ fn evaluate(self: *ScriptManager) void {
return;
}
if (self.is_blocking) {
// Cannot evaluate scripts while a blocking-load is in progress. Not
// only could that result in incorrect evaluation order, it could
// trigger another blocking request, while we're doing a blocking request.
return;
}
const page = self.page;
self.is_evaluating = true;
defer self.is_evaluating = false;
// every script in asyncs_ready is ready to be evaluated.
while (self.asyncs_ready.first) |n| {
var pending_script: *PendingScript = @fieldParentPtr("node", n);
defer pending_script.deinit();
pending_script.script.eval(page);
}
while (self.scripts.first) |n| {
var pending_script: *PendingScript = @fieldParentPtr("node", n);
if (pending_script.complete == false) {
@@ -573,11 +544,13 @@ pub const PendingScript = struct {
const manager = self.manager;
self.complete = true;
if (self.script.is_async) {
manager.asyncs.remove(&self.node);
manager.asyncs_ready.append(&self.node);
}
if (!self.script.is_async) {
manager.evaluate();
return;
}
// async script can be evaluated immediately
defer self.deinit();
self.script.eval(manager.page);
}
fn errorCallback(self: *PendingScript, err: anyerror) void {
@@ -601,7 +574,7 @@ pub const PendingScript = struct {
const script = &self.script;
if (script.is_async) {
return if (self.complete) &self.manager.asyncs_ready else &self.manager.asyncs;
return &self.manager.asyncs;
}
if (script.is_defer) {