Merge pull request #477 from lisa0314/fix/474-dynamic-script-settle

fix(js): allow pending dynamic scripts to finish settling

crates/obscura-browser/src/page.rs

@@ -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.
+            // with a watchdog that fires 250ms past the longest deadline.
-            let settle_wd = js.arm_watchdog(std::time::Duration::from_millis(750));
+            let settle_wd = js.arm_watchdog(std::time::Duration::from_millis(dynamic_settle_ms + 250));
-            let deadline = tokio::time::Instant::now() + tokio::time::Duration::from_millis(500);
+            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(

crates/obscura-cdp/tests/dynamic_script_onload_fires.rs

@@ -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"
+    );
+}

crates/obscura-js/src/runtime.rs

@@ -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