fix(js): let dynamic scripts finish settling

Keep pumping the event loop past the general 500ms settle limit only while the dynamic-script queue remains pending, with a 3s hard cap.

Add a deterministic CDP regression for direct and nested external scripts whose fetch crosses the original settle deadline.
This commit is contained in:
lisa0314
2026-07-24 15:03:53 +08:00
parent 485497393d
commit a6bb7411b2
3 changed files with 148 additions and 4 deletions

View File

@@ -744,6 +744,11 @@ impl Page {
}
if let Some(js) = &mut self.js {
let dynamic_settle_ms = std::env::var("OBSCURA_DYNAMIC_SCRIPT_SETTLE_MS")
.ok()
.and_then(|s| s.parse::<u64>().ok())
.unwrap_or(3_000)
.max(500);
// Bound the post-script settle loop by wall clock, not just by the
// 10ms-tick branch. The old code only consulted `deadline` inside
// the `Err(_)` arm (when the inner tick timed out), so a steady
@@ -753,14 +758,22 @@ impl Page {
// On busy sites this could keep the V8 lock held for tens of
// seconds, wedging the entire CDP dispatcher (see triage for
// issue series around the 40-site compat sweep).
// A dynamic external script may still be in flight at 500ms. Keep
// pumping only while that queue is pending, up to a separate bounded
// budget, so normal pages and unrelated fetches retain the fast path.
// A single run_event_loop poll that pins the thread inside V8 makes
// the per-poll tokio timeouts below useless, so guard the whole loop
// with a watchdog that fires ~250ms past its 500ms deadline.
let settle_wd = js.arm_watchdog(std::time::Duration::from_millis(750));
let deadline = tokio::time::Instant::now() + tokio::time::Duration::from_millis(500);
// with a watchdog that fires 250ms past the longest deadline.
let settle_wd = js.arm_watchdog(std::time::Duration::from_millis(dynamic_settle_ms + 250));
let started = tokio::time::Instant::now();
let deadline = started + tokio::time::Duration::from_millis(500);
let dynamic_deadline = started + tokio::time::Duration::from_millis(dynamic_settle_ms);
let mut idle_count = 0u32;
loop {
if tokio::time::Instant::now() >= deadline {
let now = tokio::time::Instant::now();
if now >= deadline
&& (now >= dynamic_deadline || !js.has_pending_dynamic_scripts())
{
break;
}
let result = tokio::time::timeout(

View File

@@ -0,0 +1,119 @@
// Regression for issue #474: external scripts inserted after a timer must be
// fetched and execute before the post-navigation event-loop settle completes.
use obscura_cdp::dispatch::{dispatch, CdpContext};
use obscura_cdp::types::CdpRequest;
use serde_json::{json, Value};
use tokio::io::{AsyncReadExt, AsyncWriteExt};
use tokio::net::TcpListener;
async fn serve() -> String {
let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
let addr = listener.local_addr().unwrap();
tokio::spawn(async move {
loop {
let (mut socket, _) = listener.accept().await.unwrap();
tokio::spawn(async move {
let mut buf = [0u8; 2048];
let read = socket.read(&mut buf).await.unwrap();
let request = String::from_utf8_lossy(&buf[..read]);
let (content_type, body) = if request.starts_with("GET /direct.js") {
tokio::time::sleep(std::time::Duration::from_millis(600)).await;
("application/javascript", "window.__directExecuted = true;")
} else if request.starts_with("GET /nested.js") {
tokio::time::sleep(std::time::Duration::from_millis(50)).await;
("application/javascript", "window.__nestedExecuted = true;")
} else {
(
"text/html",
r#"<html><body>
<div id="r">stage1</div>
<script>
setTimeout(function () {
var direct = document.createElement("script");
direct.src = "/direct.js";
direct.onload = function () { window.__directLoaded = true; };
document.body.appendChild(direct);
var box = document.createElement("div");
var nested = document.createElement("script");
nested.src = "/nested.js";
nested.onload = function () { window.__nestedLoaded = true; };
box.appendChild(nested);
document.body.appendChild(box);
}, 100);
</script>
</body></html>"#,
)
};
let response = format!(
"HTTP/1.1 200 OK\r\nContent-Type: {content_type}\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{body}",
body.len()
);
let _ = socket.write_all(response.as_bytes()).await;
});
}
});
format!("http://{addr}/")
}
async fn cdp(
ctx: &mut CdpContext,
id: u64,
method: &str,
params: Value,
session_id: &str,
) -> Value {
let response = dispatch(
&CdpRequest {
id,
method: method.to_string(),
params,
session_id: Some(session_id.to_string()),
},
ctx,
)
.await;
assert!(
response.error.is_none(),
"CDP {method} failed: {:?}",
response.error
);
response.result.unwrap_or_else(|| json!({}))
}
#[tokio::test(flavor = "current_thread")]
async fn dynamic_external_scripts_execute_and_fire_load() {
std::env::set_var("OBSCURA_ALLOW_PRIVATE_NETWORK", "1");
let url = serve().await;
let mut ctx = CdpContext::new();
let page_id = ctx.create_page();
let session_id = "session-1";
ctx.sessions.insert(session_id.to_string(), page_id);
cdp(
&mut ctx,
1,
"Page.navigate",
json!({"url": url, "waitUntil": "load"}),
session_id,
)
.await;
let result = cdp(
&mut ctx,
2,
"Runtime.evaluate",
json!({
"expression": "JSON.stringify({directExecuted: !!window.__directExecuted, directLoaded: !!window.__directLoaded, nestedExecuted: !!window.__nestedExecuted, nestedLoaded: !!window.__nestedLoaded})",
"returnByValue": true,
}),
session_id,
)
.await;
assert_eq!(
result["result"]["value"],
r#"{"directExecuted":true,"directLoaded":true,"nestedExecuted":true,"nestedLoaded":true}"#,
"dynamic scripts must execute and fire load before navigation settles"
);
}

View File

@@ -859,6 +859,18 @@ impl ObscuraJsRuntime {
.map_err(|e| format!("Event loop error: {}", e))
}
/// Whether the serialized dynamic-script queue is still fetching or
/// evaluating a script. The queue variables are global lexicals rather
/// than window properties, so page code cannot overwrite this state.
pub fn has_pending_dynamic_scripts(&mut self) -> bool {
self.evaluate(
"typeof __dynScriptBusy !== 'undefined' && (__dynScriptBusy || __dynScriptQueue.length > 0)",
)
.ok()
.and_then(|value| value.as_bool())
.unwrap_or(false)
}
/// Arm a hard wall-clock backstop on synchronous V8 work. A page stuck in a
/// synchronous loop or a microtask storm pins the OS thread inside V8, so
/// `tokio::time::timeout` (which can only cancel at await points) never