From a6260ed26e4925acce2574d60a2e5169d60b8372 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 8 Sep 2026 16:50:54 +0000 Subject: [PATCH 1/3] Exploit actionability and CDP hot-path performance constraints. Move default optionless sync Locator.click onto a core locator_click_actionable path that keeps the poll loop and mouse dispatch in Rust with cached probe expressions and optional on_poll handlers. Cache retention_gate on CdpClient, stop cloning full outstanding commands on the writer, and approximate retained CDP event sizes without re-serializing Values on the event path. Co-authored-by: Jared Lunde --- python/rustwright/sync_api.py | 36 +++ src/lib.rs | 331 ++++++++++++++++++---- tests/test_rustwright_sync_api.py | 15 + tools/measure_click_actionability_host.py | 76 +++++ 4 files changed, 408 insertions(+), 50 deletions(-) create mode 100755 tools/measure_click_actionability_host.py diff --git a/python/rustwright/sync_api.py b/python/rustwright/sync_api.py index 79a949e..7fb1699 100644 --- a/python/rustwright/sync_api.py +++ b/python/rustwright/sync_api.py @@ -24470,6 +24470,42 @@ def run_post_action_locator_handlers() -> None: deadline = time.monotonic() + max(timeout_ms, 1.0) / 1000 self._page._run_locator_handlers(deadline) + use_legacy_click = ( + forced + or bool(trial) + or getattr(self._page, "_active_page_cdp_event_contexts", 0) > 0 + or any( + value is not None + for value in (modifiers, position, delay, button, click_count, steps) + ) + ) + if not use_legacy_click: + timeout_ms = _default_timeout_for_method(self._page, timeout, method=method) + mouse = self._page.mouse + on_poll = ( + self._page._run_locator_handlers_for_remaining + if getattr(self._page, "_locator_handlers", None) + else None + ) + target_x, target_y = _call( + self._page._core.locator_click_actionable, + _json(self._spec), + self._index, + bool(self._strict and not self._explicit_index), + timeout_ms, + mouse._x, + mouse._y, + mouse._buttons, + self._page.keyboard._modifiers_mask(), + on_poll, + ) + mouse._x = float(target_x) + mouse._y = float(target_y) + mouse._buttons &= ~1 + run_post_action_locator_handlers() + self._page._slow_mo() + return + target_info: Optional[dict[str, Any]] = None if forced: target_info = self._wait_for_forced_visible_pointer_action( diff --git a/src/lib.rs b/src/lib.rs index 532bbc9..e6cab27 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -3442,6 +3442,7 @@ mod tests { outstanding: Arc::new(Mutex::new(HashMap::new())), events: events.clone(), event_log: Arc::clone(&event_log), + retention_gate: event_log.lock().unwrap_or_else(|poisoned| poisoned.into_inner()).retention_gate.clone(), traffic_log: Arc::new(Mutex::new(CdpTrafficLog::new())), runtime_state: Arc::new(Mutex::new(CdpRuntimeState::new(None))), next_id: AtomicU64::new(1), @@ -5284,6 +5285,7 @@ multiline-compatible = """4.5.6""" outstanding: Arc::new(Mutex::new(HashMap::new())), events: events.clone(), event_log: Arc::clone(&event_log), + retention_gate: event_log.lock().unwrap_or_else(|poisoned| poisoned.into_inner()).retention_gate.clone(), traffic_log: Arc::new(Mutex::new(CdpTrafficLog::new())), runtime_state: Arc::new(Mutex::new(CdpRuntimeState::new(None))), next_id: AtomicU64::new(1), @@ -5444,6 +5446,7 @@ multiline-compatible = """4.5.6""" outstanding: Arc::new(Mutex::new(HashMap::new())), events: events.clone(), event_log: Arc::clone(&event_log), + retention_gate: event_log.lock().unwrap_or_else(|poisoned| poisoned.into_inner()).retention_gate.clone(), traffic_log: Arc::new(Mutex::new(CdpTrafficLog::new())), runtime_state: Arc::new(Mutex::new(CdpRuntimeState::new(None))), next_id: AtomicU64::new(1), @@ -5557,6 +5560,7 @@ multiline-compatible = """4.5.6""" outstanding: Arc::new(Mutex::new(HashMap::new())), events: events.clone(), event_log: Arc::clone(&event_log), + retention_gate: event_log.lock().unwrap_or_else(|poisoned| poisoned.into_inner()).retention_gate.clone(), traffic_log: Arc::new(Mutex::new(CdpTrafficLog::new())), runtime_state: Arc::new(Mutex::new(CdpRuntimeState::new(None))), next_id: AtomicU64::new(1), @@ -5957,6 +5961,7 @@ multiline-compatible = """4.5.6""" outstanding: Arc::new(Mutex::new(HashMap::new())), events: events.clone(), event_log: Arc::clone(&event_log), + retention_gate: event_log.lock().unwrap_or_else(|poisoned| poisoned.into_inner()).retention_gate.clone(), traffic_log: Arc::new(Mutex::new(CdpTrafficLog::new())), runtime_state: Arc::new(Mutex::new(CdpRuntimeState::new(None))), next_id: AtomicU64::new(1), @@ -6030,12 +6035,18 @@ multiline-compatible = """4.5.6""" let (events, _) = broadcast::channel(4); let event_log = Arc::new(Mutex::new(CdpEventLog::new())); let (alive_tx, _) = watch::channel(true); + let retention_gate = event_log + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .retention_gate + .clone(); let client = CdpClient { write_tx, pending, outstanding: Arc::new(Mutex::new(HashMap::new())), events, event_log, + retention_gate, traffic_log: Arc::new(Mutex::new(CdpTrafficLog::new())), runtime_state: Arc::new(Mutex::new(CdpRuntimeState::new(None))), next_id: AtomicU64::new(1), @@ -6130,6 +6141,11 @@ multiline-compatible = """4.5.6""" let (write_tx, write_rx) = mpsc::unbounded_channel(); let (events, _) = broadcast::channel(4); let (alive_tx, _) = watch::channel(true); + let (event_log, retention_gate) = { + let event_log = Arc::new(Mutex::new(CdpEventLog::new())); + let retention_gate = event_log.lock().unwrap_or_else(|poisoned| poisoned.into_inner()).retention_gate.clone(); + (event_log, retention_gate) + }; let browser = Arc::new(BrowserInner { runtime: OwnedRuntime::new(runtime), client: Arc::new(CdpClient { @@ -6137,7 +6153,8 @@ multiline-compatible = """4.5.6""" pending: Arc::new(Mutex::new(HashMap::new())), outstanding: Arc::new(Mutex::new(HashMap::new())), events, - event_log: Arc::new(Mutex::new(CdpEventLog::new())), + event_log: Arc::clone(&event_log), + retention_gate, traffic_log: Arc::new(Mutex::new(CdpTrafficLog::new())), runtime_state: Arc::new(Mutex::new(CdpRuntimeState::new(None))), next_id: AtomicU64::new(1), @@ -6810,6 +6827,54 @@ multiline-compatible = """4.5.6""" assert!(ACTIONABILITY_PROBE_BODY.contains("__STRICT__")); } + #[test] + fn retained_cdp_event_size_avoids_full_reserialize_and_tracks_growth() { + let small = json!({"method": "Page.loadEventFired", "params": {}}); + let large = json!({ + "method": "Network.responseReceived", + "params": { + "requestId": "r1", + "payload": "x".repeat(10_000), + } + }); + let small_size = retained_cdp_event_size(&small); + let large_size = retained_cdp_event_size(&large); + assert!(small_size > 0); + assert!(large_size > small_size); + assert!(large_size >= 10_000); + } + + #[test] + fn native_action_body_and_locator_script_are_stable_for_cache_reuse() { + let body = native_action_body(LOCATOR_TARGET_STATE_TEMPLATE); + let locator_json = "{\"kind\":\"css\",\"selector\":\"#ready\"}"; + let first = locator_script(locator_json, 0, &body); + let second = locator_script(locator_json, 0, &body); + assert_eq!(first, second); + assert!(first.len() > 100); + // Rebuilding the script for every poll is the cost we cache away on the + // native actionable paths; keep the assembly itself deterministic. + let start = Instant::now(); + for _ in 0..200 { + let _ = locator_script(locator_json, 0, &body); + } + let rebuild_ms = start.elapsed().as_secs_f64() * 1_000.0; + let start = Instant::now(); + let cached = locator_script(locator_json, 0, &body); + for _ in 0..200 { + let _ = cached.len(); + } + let cached_ms = start.elapsed().as_secs_f64() * 1_000.0; + assert!( + rebuild_ms > cached_ms, + "expected script rebuild ({rebuild_ms:.3}ms) to exceed cached reuse ({cached_ms:.3}ms)" + ); + eprintln!( + "locator_script_cache_microbench rebuild_ms={rebuild_ms:.3} cached_ms={cached_ms:.3} script_bytes={}", + cached.len() + ); + } + #[test] fn actionability_poll_backoff_doubles_while_state_is_unchanged() { assert_eq!( @@ -8116,6 +8181,7 @@ multiline-compatible = """4.5.6""" 0, Some(40.0), true, + None, )); let deadline = tokio::time::Instant::now() + Duration::from_secs(3); loop { @@ -9140,6 +9206,11 @@ multiline-compatible = """4.5.6""" let (write_tx, write_rx) = mpsc::unbounded_channel(); let (events, _) = broadcast::channel(4); let (alive_tx, _) = watch::channel(true); + let (event_log, retention_gate) = { + let event_log = Arc::new(Mutex::new(CdpEventLog::new())); + let retention_gate = event_log.lock().unwrap_or_else(|poisoned| poisoned.into_inner()).retention_gate.clone(); + (event_log, retention_gate) + }; let inner = Arc::new(BrowserInner { runtime: OwnedRuntime::new(runtime), client: Arc::new(CdpClient { @@ -9147,7 +9218,8 @@ multiline-compatible = """4.5.6""" pending: Arc::new(Mutex::new(HashMap::new())), outstanding: Arc::new(Mutex::new(HashMap::new())), events, - event_log: Arc::new(Mutex::new(CdpEventLog::new())), + event_log: Arc::clone(&event_log), + retention_gate, traffic_log: Arc::new(Mutex::new(CdpTrafficLog::new())), runtime_state: Arc::new(Mutex::new(CdpRuntimeState::new(None))), next_id: AtomicU64::new(1), @@ -9209,6 +9281,11 @@ multiline-compatible = """4.5.6""" let (write_tx, _write_rx) = mpsc::unbounded_channel(); let (events, _) = broadcast::channel(4); let (alive_tx, _) = watch::channel(true); + let (event_log, retention_gate) = { + let event_log = Arc::new(Mutex::new(CdpEventLog::new())); + let retention_gate = event_log.lock().unwrap_or_else(|poisoned| poisoned.into_inner()).retention_gate.clone(); + (event_log, retention_gate) + }; let browser = Arc::new(BrowserInner { runtime: OwnedRuntime::new(runtime), client: Arc::new(CdpClient { @@ -9216,7 +9293,8 @@ multiline-compatible = """4.5.6""" pending: Arc::new(Mutex::new(HashMap::new())), outstanding: Arc::new(Mutex::new(HashMap::new())), events, - event_log: Arc::new(Mutex::new(CdpEventLog::new())), + event_log: Arc::clone(&event_log), + retention_gate, traffic_log: Arc::new(Mutex::new(CdpTrafficLog::new())), runtime_state: Arc::new(Mutex::new(CdpRuntimeState::new(None))), next_id: AtomicU64::new(1), @@ -9305,6 +9383,7 @@ multiline-compatible = """4.5.6""" outstanding: Arc::new(Mutex::new(HashMap::new())), events: events.clone(), event_log: Arc::clone(&event_log), + retention_gate: event_log.lock().unwrap_or_else(|poisoned| poisoned.into_inner()).retention_gate.clone(), traffic_log: Arc::new(Mutex::new(CdpTrafficLog::new())), runtime_state: Arc::new(Mutex::new(CdpRuntimeState::new(None))), next_id: AtomicU64::new(1), @@ -9375,13 +9454,19 @@ multiline-compatible = """4.5.6""" let (write_tx, write_rx) = mpsc::unbounded_channel(); let (events, _) = broadcast::channel(4); let (alive_tx, _) = watch::channel(true); + let (event_log, retention_gate) = { + let event_log = Arc::new(Mutex::new(CdpEventLog::new())); + let retention_gate = event_log.lock().unwrap_or_else(|poisoned| poisoned.into_inner()).retention_gate.clone(); + (event_log, retention_gate) + }; ( CdpClient { write_tx, pending: Arc::new(Mutex::new(HashMap::new())), outstanding: Arc::new(Mutex::new(HashMap::new())), events, - event_log: Arc::new(Mutex::new(CdpEventLog::new())), + event_log: Arc::clone(&event_log), + retention_gate, traffic_log: Arc::new(Mutex::new(CdpTrafficLog::new())), runtime_state: Arc::new(Mutex::new(CdpRuntimeState::new(None))), next_id: AtomicU64::new(1), @@ -9525,6 +9610,7 @@ multiline-compatible = """4.5.6""" outstanding: Arc::new(Mutex::new(HashMap::new())), events: events.clone(), event_log: Arc::clone(&event_log), + retention_gate: event_log.lock().unwrap_or_else(|poisoned| poisoned.into_inner()).retention_gate.clone(), traffic_log: Arc::new(Mutex::new(CdpTrafficLog::new())), runtime_state: Arc::new(Mutex::new(CdpRuntimeState::new(None))), next_id: AtomicU64::new(1), @@ -9611,6 +9697,7 @@ multiline-compatible = """4.5.6""" outstanding: Arc::new(Mutex::new(HashMap::new())), events: events.clone(), event_log: Arc::clone(&event_log), + retention_gate: event_log.lock().unwrap_or_else(|poisoned| poisoned.into_inner()).retention_gate.clone(), traffic_log: Arc::new(Mutex::new(CdpTrafficLog::new())), runtime_state: Arc::new(Mutex::new(CdpRuntimeState::new(None))), next_id: AtomicU64::new(1), @@ -9681,12 +9768,18 @@ multiline-compatible = """4.5.6""" let pending = Arc::new(Mutex::new(HashMap::new())); let (events, _) = broadcast::channel(4); let (alive_tx, _) = watch::channel(true); + let (event_log, retention_gate) = { + let event_log = Arc::new(Mutex::new(CdpEventLog::new())); + let retention_gate = event_log.lock().unwrap_or_else(|poisoned| poisoned.into_inner()).retention_gate.clone(); + (event_log, retention_gate) + }; let client = Arc::new(CdpClient { write_tx, pending: Arc::clone(&pending), outstanding: Arc::new(Mutex::new(HashMap::new())), events, - event_log: Arc::new(Mutex::new(CdpEventLog::new())), + event_log: Arc::clone(&event_log), + retention_gate, traffic_log: Arc::new(Mutex::new(CdpTrafficLog::new())), runtime_state: Arc::new(Mutex::new(CdpRuntimeState::new(None))), next_id: AtomicU64::new(1), @@ -9855,6 +9948,11 @@ multiline-compatible = """4.5.6""" let (write_tx, _write_rx) = mpsc::unbounded_channel(); let (events, _) = broadcast::channel(4); let (alive_tx, _) = watch::channel(true); + let (event_log, retention_gate) = { + let event_log = Arc::new(Mutex::new(CdpEventLog::new())); + let retention_gate = event_log.lock().unwrap_or_else(|poisoned| poisoned.into_inner()).retention_gate.clone(); + (event_log, retention_gate) + }; let browser = Arc::new(BrowserInner { runtime: OwnedRuntime(None), client: Arc::new(CdpClient { @@ -9862,7 +9960,8 @@ multiline-compatible = """4.5.6""" pending: Arc::new(Mutex::new(HashMap::new())), outstanding: Arc::new(Mutex::new(HashMap::new())), events, - event_log: Arc::new(Mutex::new(CdpEventLog::new())), + event_log: Arc::clone(&event_log), + retention_gate, traffic_log: Arc::new(Mutex::new(CdpTrafficLog::new())), runtime_state: Arc::new(Mutex::new(CdpRuntimeState::new(None))), next_id: AtomicU64::new(1), @@ -11376,6 +11475,7 @@ multiline-compatible = """4.5.6""" outstanding: Arc::new(Mutex::new(HashMap::new())), events: events.clone(), event_log: Arc::clone(&event_log), + retention_gate: event_log.lock().unwrap_or_else(|poisoned| poisoned.into_inner()).retention_gate.clone(), traffic_log: Arc::new(Mutex::new(CdpTrafficLog::new())), runtime_state: Arc::new(Mutex::new(CdpRuntimeState::new(None))), next_id: AtomicU64::new(1), @@ -25647,6 +25747,7 @@ struct CdpClient { outstanding: CdpOutstandingMap, events: broadcast::Sender, event_log: Arc>, + retention_gate: Arc, traffic_log: Arc>, runtime_state: Arc>, next_id: AtomicU64, @@ -25882,26 +25983,31 @@ fn strip_retained_heavy_payloads(event: &mut Value) { } } -struct CountingWriter { - bytes: usize, +fn retained_cdp_event_size(event: &Value) -> usize { + // Approximate JSON byte cost without re-serializing the Value on the event hot path. + // Retention caps are soft budgets; exact serde formatting is unnecessary here. + estimate_retained_json_size(event) } -impl Write for CountingWriter { - fn write(&mut self, bytes: &[u8]) -> std::io::Result { - self.bytes = self.bytes.saturating_add(bytes.len()); - Ok(bytes.len()) - } - - fn flush(&mut self) -> std::io::Result<()> { - Ok(()) +fn estimate_retained_json_size(value: &Value) -> usize { + match value { + Value::Null => 4, + Value::Bool(true) => 4, + Value::Bool(false) => 5, + Value::Number(number) => number.to_string().len(), + Value::String(text) => text.len().saturating_add(2), + Value::Array(items) => items + .iter() + .map(estimate_retained_json_size) + .fold(2usize, |acc, size| acc.saturating_add(size).saturating_add(1)), + Value::Object(entries) => entries.iter().fold(2usize, |acc, (key, child)| { + acc.saturating_add(key.len()) + .saturating_add(3) + .saturating_add(estimate_retained_json_size(child)) + }), } } -fn retained_cdp_event_size(event: &Value) -> usize { - let mut writer = CountingWriter { bytes: 0 }; - serde_json::to_writer(&mut writer, event).map_or(usize::MAX, |_| writer.bytes) -} - fn retained_object_fields(value: &Value, fields: &[&str]) -> Value { let mut retained = serde_json::Map::new(); if let Some(object) = value.as_object() { @@ -26807,7 +26913,11 @@ impl CdpClient { let (events, _) = broadcast::channel(4096); let events_reader = events.clone(); let event_log = Arc::new(Mutex::new(CdpEventLog::new())); - let event_log_writer = Arc::clone(&event_log); + let retention_gate = event_log + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .retention_gate + .clone(); let event_log_reader = Arc::clone(&event_log); let traffic_log = Arc::new(Mutex::new(CdpTrafficLog::new())); let traffic_log_writer = Arc::clone(&traffic_log); @@ -26827,6 +26937,7 @@ impl CdpClient { let (alive_tx, _) = watch::channel(true); let alive_tx_writer = alive_tx.clone(); let alive_tx_reader = alive_tx.clone(); + let retention_gate_writer = Arc::clone(&retention_gate); tokio::spawn(async move { while let Some(message) = write_rx.recv().await { @@ -26836,32 +26947,43 @@ impl CdpClient { tracker, diagnostic_id, } => { - let diagnostic = diagnostic_id - .and_then(|id| outstanding_writer.lock().unwrap_or_else(|poisoned| poisoned.into_inner()).get(&id).cloned()); + // Clone only the write-state Arc under the outstanding lock; build + // traffic diagnostics without cloning the full outstanding command. + let write_state = diagnostic_id.and_then(|id| { + outstanding_writer + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .get(&id) + .map(|command| Arc::clone(&command.write_state)) + }); // The sender abandons a command whose deadline expired; skipping it here is // what makes "not written" a promise rather than a guess. if let Some(tracker) = &tracker { if !tracker.begin_write() { continue; } - } else if let Some(diagnostic) = &diagnostic { - let _ = diagnostic.write_state.begin_write(); + } else if let Some(write_state) = &write_state { + let _ = write_state.begin_write(); } let written = write.send(Message::Text(payload.into())).await.is_ok(); if let Some(tracker) = tracker { tracker.finish(written); - } else if let Some(diagnostic) = &diagnostic { - diagnostic.write_state.finish(written); + } else if let Some(write_state) = &write_state { + write_state.finish(written); } if written { - if let Some(diagnostic) = diagnostic { - let retention_gate = - event_log_writer.lock().unwrap_or_else(|poisoned| poisoned.into_inner()).retention_gate.clone(); - if let Some(_retention_guard) = retention_gate.lock_for_write() { - traffic_log_writer + if let Some(id) = diagnostic_id { + if let Some(_retention_guard) = retention_gate_writer.lock_for_write() + { + let outstanding = outstanding_writer .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()) - .push(diagnostic.traffic_entry("sent")); + .unwrap_or_else(|poisoned| poisoned.into_inner()); + if let Some(command) = outstanding.get(&id) { + traffic_log_writer + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .push(command.traffic_entry("sent")); + } }; } } @@ -26918,6 +27040,7 @@ impl CdpClient { outstanding, events, event_log, + retention_gate, traffic_log, runtime_state, next_id: AtomicU64::new(1), @@ -26945,8 +27068,13 @@ impl CdpClient { let (events, _) = broadcast::channel(4096); let events_dispatcher = events.clone(); let event_log = Arc::new(Mutex::new(CdpEventLog::new())); - let event_log_writer = Arc::clone(&event_log); + let retention_gate = event_log + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .retention_gate + .clone(); let event_log_dispatcher = Arc::clone(&event_log); + let retention_gate_writer = Arc::clone(&retention_gate); let traffic_log = Arc::new(Mutex::new(CdpTrafficLog::new())); let traffic_log_writer = Arc::clone(&traffic_log); let traffic_log_dispatcher = Arc::clone(&traffic_log); @@ -26971,33 +27099,41 @@ impl CdpClient { tracker, diagnostic_id, } => { - let diagnostic = diagnostic_id - .and_then(|id| outstanding_writer.lock().unwrap_or_else(|poisoned| poisoned.into_inner()).get(&id).cloned()); + let write_state = diagnostic_id.and_then(|id| { + outstanding_writer + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .get(&id) + .map(|command| Arc::clone(&command.write_state)) + }); // The sender abandons a command whose deadline expired; skipping it here is // what makes "not written" a promise rather than a guess. if let Some(tracker) = &tracker { if !tracker.begin_write() { continue; } - } else if let Some(diagnostic) = &diagnostic { - let _ = diagnostic.write_state.begin_write(); + } else if let Some(write_state) = &write_state { + let _ = write_state.begin_write(); } let written = pipe_write.write_all(payload.as_bytes()).is_ok() && pipe_write.write_all(&[0]).is_ok(); if let Some(tracker) = tracker { tracker.finish(written); - } else if let Some(diagnostic) = &diagnostic { - diagnostic.write_state.finish(written); + } else if let Some(write_state) = &write_state { + write_state.finish(written); } if written { - if let Some(diagnostic) = diagnostic { - let retention_gate = - event_log_writer.lock().unwrap_or_else(|poisoned| poisoned.into_inner()).retention_gate.clone(); - if let Some(_retention_guard) = retention_gate.lock_for_write() { - traffic_log_writer + if let Some(id) = diagnostic_id { + if let Some(_retention_guard) = retention_gate_writer.lock_for_write() { + let outstanding = outstanding_writer .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()) - .push(diagnostic.traffic_entry("sent")); + .unwrap_or_else(|poisoned| poisoned.into_inner()); + if let Some(command) = outstanding.get(&id) { + traffic_log_writer + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .push(command.traffic_entry("sent")); + } }; } } @@ -27067,6 +27203,7 @@ impl CdpClient { outstanding, events, event_log, + retention_gate, traffic_log, runtime_state, next_id: AtomicU64::new(1), @@ -27146,7 +27283,7 @@ impl CdpClient { } fn retention_gate(&self) -> Arc { - self.event_log.lock().unwrap_or_else(|poisoned| poisoned.into_inner()).retention_gate.clone() + Arc::clone(&self.retention_gate) } fn retention_enabled(&self) -> bool { @@ -38570,6 +38707,7 @@ async fn page_click_actionable_wait_async( index: usize, timeout_ms: Option, strict: bool, + on_poll: Option, ) -> RwResult<(f64, f64, f64)> { let timeout_ms = Some(sanitize_action_timeout_ms(timeout_ms, true)); let deadline = action_deadline(action_timeout_duration(timeout_ms, true)); @@ -38579,6 +38717,8 @@ async fn page_click_actionable_wait_async( let mut pinned_resolution = None; let mut cached_expression = None; let actionable_info = loop { + let remaining = deadline.saturating_duration_since(Instant::now()); + run_action_poll_hook(&on_poll, remaining).await?; ensure_native_action_owner_available(&page, "click")?; let remaining = deadline.saturating_duration_since(Instant::now()); let command_timeout = action_poll_timeout(timeout_ms, true, remaining); @@ -38659,6 +38799,42 @@ async fn page_click_actionable_wait_async( Ok((target_x, target_y, remaining_ms)) } +#[allow(clippy::too_many_arguments)] +async fn page_click_actionable_async( + page: Arc, + locator_json: String, + index: usize, + timeout_ms: Option, + strict: bool, + start_x: f64, + start_y: f64, + initial_buttons: i64, + modifiers: i64, + on_poll: Option, +) -> RwResult<(f64, f64)> { + let (target_x, target_y, remaining_ms) = page_click_actionable_wait_async( + Arc::clone(&page), + locator_json, + index, + timeout_ms, + strict, + on_poll, + ) + .await?; + dispatch_mouse_click_async( + page, + target_x, + target_y, + start_x, + start_y, + initial_buttons, + modifiers, + remaining_ms, + ) + .await?; + Ok((target_x, target_y)) +} + #[derive(Debug, PartialEq, Eq)] enum FillAttempt { Success, @@ -39432,6 +39608,7 @@ impl PyPage { index, timeout_ms, strict, + None, ), |py, value| Ok(value.into_pyobject(py)?.unbind().into_any()), ) @@ -41602,6 +41779,58 @@ return win.__rustwrightCleanupDrag ? win.__rustwrightCleanupDrag() : false; .map_err(py_err) } + #[pyo3(signature = ( + locator_json, + index, + strict, + timeout_ms=None, + start_x=0.0, + start_y=0.0, + initial_buttons=0, + modifiers=0, + on_poll=None, + cancel=None + ))] + #[allow(clippy::too_many_arguments)] + fn locator_click_actionable( + &self, + py: Python<'_>, + locator_json: &str, + index: usize, + strict: bool, + timeout_ms: Option, + start_x: f64, + start_y: f64, + initial_buttons: i64, + modifiers: i64, + on_poll: Option>, + cancel: Option>, + ) -> PyResult<(f64, f64)> { + let page = Arc::clone(&self.inner); + let browser = Arc::clone(&page.browser); + let locator_json = locator_json.to_string(); + let on_poll = on_poll.map(python_action_poll_hook); + let cancel = cancel.map(|cancel| cancel.token.clone()); + py.detach(move || { + browser.block_on(cancelable( + cancel, + page_click_actionable_async( + page, + locator_json, + index, + timeout_ms, + strict, + start_x, + start_y, + initial_buttons, + modifiers, + on_poll, + ), + )) + }) + .map_err(py_err) + } + #[pyo3(signature = (locator_json, index, text, timeout_ms=None))] fn r#type( &self, @@ -44673,6 +44902,7 @@ impl RustwrightNavigationHarness { outstanding: Arc::new(Mutex::new(HashMap::new())), events: events.clone(), event_log: Arc::clone(&event_log), + retention_gate: event_log.lock().unwrap_or_else(|poisoned| poisoned.into_inner()).retention_gate.clone(), traffic_log: Arc::new(Mutex::new(CdpTrafficLog::new())), runtime_state: Arc::new(Mutex::new(CdpRuntimeState::new(None))), next_id: AtomicU64::new(1), @@ -51470,6 +51700,7 @@ mod native_console_record_tests { outstanding: Arc::new(Mutex::new(HashMap::new())), events: events.clone(), event_log: Arc::clone(&event_log), + retention_gate: event_log.lock().unwrap_or_else(|poisoned| poisoned.into_inner()).retention_gate.clone(), traffic_log: Arc::new(Mutex::new(CdpTrafficLog::new())), runtime_state: Arc::new(Mutex::new(CdpRuntimeState::new(None))), next_id: AtomicU64::new(1), diff --git a/tests/test_rustwright_sync_api.py b/tests/test_rustwright_sync_api.py index 13159ef..e6a281a 100644 --- a/tests/test_rustwright_sync_api.py +++ b/tests/test_rustwright_sync_api.py @@ -14049,6 +14049,21 @@ def test_locator_fill_routes_deadline_retry_and_result_classification_through_co assert "time.monotonic" not in source +def test_locator_click_routes_default_optionless_through_core(): + from rustwright.sync_api import Locator + + source = inspect.getsource(Locator._click_impl) + + assert "locator_click_actionable" in source + assert "use_legacy_click" in source + # Default optionless path must not fall through to the Python actionable poll loop. + default_branch = source.split("if not use_legacy_click:", 1)[1].split("target_info:", 1)[0] + assert "locator_click_actionable" in default_branch + assert "_wait_for_single" not in default_branch + assert "_mouse_click_at_point" not in default_branch + assert "while True" not in default_branch + + def test_locator_fill_timeout_zero_disables_timeout(page): page.set_content("") diff --git a/tools/measure_click_actionability_host.py b/tools/measure_click_actionability_host.py new file mode 100755 index 0000000..dc1459a --- /dev/null +++ b/tools/measure_click_actionability_host.py @@ -0,0 +1,76 @@ +#!/usr/bin/env python3 +"""Diagnostic microbench for sync optionless click actionability. + +Measures host-visible wall time for a delayed-visibility click. Not launch +evidence — local verification only. Uses mock keychain Chromium args. +""" + +from __future__ import annotations + +import argparse +import json +import statistics +import time + + +def _run(iterations: int) -> dict: + from rustwright.sync_api import sync_playwright + + samples_ms: list[float] = [] + with sync_playwright() as p: + browser = p.chromium.launch(headless=True) + page = browser.new_page() + for i in range(iterations): + delay_ms = 120 + page.set_content( + f""" + + + """ + ) + start = time.perf_counter() + page.locator("#ready").click(timeout=5_000) + elapsed_ms = (time.perf_counter() - start) * 1000.0 + clicked = page.locator("#ready").evaluate("el => el.dataset.clicked") + if clicked != "1": + raise RuntimeError(f"click did not land on iteration {i}") + samples_ms.append(elapsed_ms) + browser.close() + + return { + "iterations": iterations, + "delay_ms": 120, + "mean_ms": statistics.mean(samples_ms), + "median_ms": statistics.median(samples_ms), + "p95_ms": sorted(samples_ms)[max(0, int(len(samples_ms) * 0.95) - 1)], + "min_ms": min(samples_ms), + "max_ms": max(samples_ms), + "samples_ms": samples_ms, + "path": "locator_click_actionable", + } + + +def main() -> None: + parser = argparse.ArgumentParser() + parser.add_argument("--iterations", type=int, default=12) + parser.add_argument("--json", action="store_true") + args = parser.parse_args() + result = _run(args.iterations) + if args.json: + print(json.dumps(result, indent=2)) + else: + print( + f"click_actionability iterations={result['iterations']} " + f"mean={result['mean_ms']:.1f}ms median={result['median_ms']:.1f}ms " + f"p95={result['p95_ms']:.1f}ms path={result['path']}" + ) + + +if __name__ == "__main__": + main() From 443df2b5444fbb315b2c6e7665779d5fb4c625bc Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 8 Sep 2026 16:58:53 +0000 Subject: [PATCH 2/3] Improve click actionability host microbench for delayed polls. Add --delay-ms and overhead reporting so before/after comparisons isolate host cost above the artificial visibility delay. Co-authored-by: Jared Lunde --- tools/measure_click_actionability_host.py | 32 ++++++++++++++++------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/tools/measure_click_actionability_host.py b/tools/measure_click_actionability_host.py index dc1459a..56baa32 100755 --- a/tools/measure_click_actionability_host.py +++ b/tools/measure_click_actionability_host.py @@ -2,7 +2,7 @@ """Diagnostic microbench for sync optionless click actionability. Measures host-visible wall time for a delayed-visibility click. Not launch -evidence — local verification only. Uses mock keychain Chromium args. +evidence — local verification only. """ from __future__ import annotations @@ -13,7 +13,7 @@ import time -def _run(iterations: int) -> dict: +def _run(iterations: int, delay_ms: int) -> dict: from rustwright.sync_api import sync_playwright samples_ms: list[float] = [] @@ -21,7 +21,6 @@ def _run(iterations: int) -> dict: browser = p.chromium.launch(headless=True) page = browser.new_page() for i in range(iterations): - delay_ms = 120 page.set_content( f""" @@ -35,7 +34,7 @@ def _run(iterations: int) -> dict: """ ) start = time.perf_counter() - page.locator("#ready").click(timeout=5_000) + page.locator("#ready").click(timeout=10_000) elapsed_ms = (time.perf_counter() - start) * 1000.0 clicked = page.locator("#ready").evaluate("el => el.dataset.clicked") if clicked != "1": @@ -43,32 +42,45 @@ def _run(iterations: int) -> dict: samples_ms.append(elapsed_ms) browser.close() + overhead = [s - delay_ms for s in samples_ms] + path = "locator_click_actionable" + try: + from rustwright.sync_api import Locator + import inspect + if "locator_click_actionable" not in inspect.getsource(Locator._click_impl): + path = "python_wait_for_single" + except Exception: + path = "unknown" + return { "iterations": iterations, - "delay_ms": 120, + "delay_ms": delay_ms, "mean_ms": statistics.mean(samples_ms), "median_ms": statistics.median(samples_ms), "p95_ms": sorted(samples_ms)[max(0, int(len(samples_ms) * 0.95) - 1)], "min_ms": min(samples_ms), "max_ms": max(samples_ms), + "mean_overhead_ms": statistics.mean(overhead), + "median_overhead_ms": statistics.median(overhead), "samples_ms": samples_ms, - "path": "locator_click_actionable", + "path": path, } def main() -> None: parser = argparse.ArgumentParser() parser.add_argument("--iterations", type=int, default=12) + parser.add_argument("--delay-ms", type=int, default=400) parser.add_argument("--json", action="store_true") args = parser.parse_args() - result = _run(args.iterations) + result = _run(args.iterations, args.delay_ms) if args.json: print(json.dumps(result, indent=2)) else: print( - f"click_actionability iterations={result['iterations']} " - f"mean={result['mean_ms']:.1f}ms median={result['median_ms']:.1f}ms " - f"p95={result['p95_ms']:.1f}ms path={result['path']}" + f"click_actionability iterations={result['iterations']} delay={result['delay_ms']}ms " + f"mean={result['mean_ms']:.1f}ms overhead={result['mean_overhead_ms']:.1f}ms " + f"path={result['path']}" ) From 4abb9edbed6ce48c09ef7e6aee2993f3005782e8 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 8 Sep 2026 17:26:28 +0000 Subject: [PATCH 3/3] Fix bindings artifact upload retry 409 after FinalizeArtifact 403. A failed Finalize can still reserve the artifact name, so retrying with overwrite:true hits Conflict. Upload the retry under a distinct results--retry- name that equivalence still matches via results-*. Co-authored-by: Jared Lunde --- .github/workflows/bindings.yml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/.github/workflows/bindings.yml b/.github/workflows/bindings.yml index 18b4101..571413f 100644 --- a/.github/workflows/bindings.yml +++ b/.github/workflows/bindings.yml @@ -145,8 +145,10 @@ jobs: print(f"{d['lang']}: {len(d['results'])} cases OK") PY # Upload can hit intermittent FinalizeArtifact 403s from the Actions - # intermediary; retry a few times before failing the job. The runner - # assertions above already validated the binding itself. + # intermediary; retry before failing the job. The runner assertions above + # already validated the binding itself. A failed Finalize can still reserve + # the artifact name, so overwrite:true often 409s — use a distinct retry name + # that the equivalence job still picks up via results-*. - name: Upload results if: steps.has.outputs.ok == 'true' uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 @@ -160,10 +162,9 @@ jobs: if: steps.has.outputs.ok == 'true' && steps.upload-results.outcome != 'success' uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: - name: results-${{ matrix.lang }} + name: results-${{ matrix.lang }}-retry-${{ github.run_attempt }} path: ${{ env.RW_OUT }} if-no-files-found: error - overwrite: true equivalence: name: Cross-language equivalence