fix: coalesce queued scroll input

2026-07-10 17:17:34 -04:00
parent 3c3d7edcd6
commit 37eb0a7825
5 changed files with 158 additions and 8 deletions
@@ -88,6 +88,10 @@ Real and verified:
  (LOAD 1 → LOAD 2). The webview loading state reconciles through
  redirects/pushState (`awaiting_url_change`), and `run_dev.sh` resolves the
  macOS Metal toolchain so `cargo run`'s shader-build failure is fixed.
+- Scroll input: consecutive same-document wheel requests coalesce at the
+  profile worker boundary while preserving total device-pixel distance;
+  clicks, text, and allow-once permission transfers retain strict ordering.
+  Verified by a blocked-worker burst regression and the live-site scroll e2e.
 - Cross-platform: download Open/Reveal use per-OS launchers (macOS `open`,
  Windows `cmd start`/`explorer /select`, Linux `xdg-open`); the command
  overlay closes on Escape.
@@ -13,8 +13,8 @@ use crate::services::servo_live::{
 #[path = "web_surface_worker_dispatch.rs"]
 mod dispatch;
 use dispatch::{
-    dispatch_result, forward_permission_consumptions, preserve_latest_hover,
-    request_has_ordered_input,
+    can_merge_consecutive_scroll, dispatch_result, forward_permission_consumptions,
+    merge_consecutive_scroll, preserve_latest_hover, request_has_ordered_input,
 };

 /// Blocking transport for one profile-scoped Servo sidecar.
@@ -225,11 +225,12 @@ impl LiveRuntimeWorker {
        }
        let mut request = WorkerRequest::Ensure { generation, request: Box::new(request) };
        if let Some(pending) = q.pending.get_mut(&tab_id) {
-            let replace_tail = pending.back().is_some_and(|tail| {
-                matches!(tail, WorkerRequest::Poll { .. })
-                    || (!request_has_ordered_input(&request) && !request_has_ordered_input(tail))
-            });
-            if replace_tail && let Some(tail) = pending.back_mut() {
+            if let Some(tail) = pending.back_mut()
+                && (can_merge_consecutive_scroll(&request, tail)
+                    || matches!(tail, WorkerRequest::Poll { .. })
+                    || (!request_has_ordered_input(&request) && !request_has_ordered_input(tail)))
+            {
+                merge_consecutive_scroll(&mut request, tail);
                preserve_latest_hover(&mut request, tail);
                *tail = request;
            } else {
@@ -241,7 +242,6 @@ impl LiveRuntimeWorker {
        }
        cvar.notify_one();
    }
-
    pub(super) fn submit_poll(&self, generation: RequestGeneration, tab_id: String) -> bool {
        let (lock, cvar) = &*self.queue;
        let mut q = match lock.lock() {
@@ -6,6 +6,55 @@ use super::{
    LiveRuntimeClient, LiveRuntimeClientError, RequestGeneration, WorkerRequest, WorkerResponse,
 };

+pub(super) fn can_merge_consecutive_scroll(
+    latest: &WorkerRequest,
+    previous: &WorkerRequest,
+) -> bool {
+    let (
+        WorkerRequest::Ensure { request: latest, .. },
+        WorkerRequest::Ensure { request: previous, .. },
+    ) = (latest, previous)
+    else {
+        return false;
+    };
+    is_scroll_only(latest)
+        && is_scroll_only(previous)
+        && latest.profile_id == previous.profile_id
+        && latest.url == previous.url
+        && latest.width == previous.width
+        && latest.height == previous.height
+        && latest.page_zoom_percent == previous.page_zoom_percent
+        && latest.device_pixel_ratio == previous.device_pixel_ratio
+}
+
+pub(super) fn merge_consecutive_scroll(latest: &mut WorkerRequest, previous: &WorkerRequest) {
+    if !can_merge_consecutive_scroll(latest, previous) {
+        return;
+    }
+    let (
+        WorkerRequest::Ensure { request: latest, .. },
+        WorkerRequest::Ensure { request: previous, .. },
+    ) = (latest, previous)
+    else {
+        return;
+    };
+
+    latest.scroll_delta_x = previous.scroll_delta_x.saturating_add(latest.scroll_delta_x);
+    latest.scroll_delta_y = previous.scroll_delta_y.saturating_add(latest.scroll_delta_y);
+    if latest.hover_x.is_none() && latest.hover_y.is_none() {
+        latest.hover_x = previous.hover_x;
+        latest.hover_y = previous.hover_y;
+    }
+}
+
+fn is_scroll_only(request: &crate::services::servo_live::ServoLiveEnsureRequest) -> bool {
+    (request.scroll_delta_x != 0 || request.scroll_delta_y != 0)
+        && request.click_x.is_none()
+        && request.click_y.is_none()
+        && request.typed_text.is_none()
+        && !request.site_permissions.iter().any(|permission| permission.state == "allow-once")
+}
+
 pub(super) fn forward_permission_consumptions(
    client: &mut dyn LiveRuntimeClient,
    response_tx: &mpsc::Sender<WorkerResponse>,
@@ -0,0 +1,94 @@
+use std::{
+    sync::{Arc, Mutex, mpsc},
+    time::Duration,
+};
+
+use crate::services::servo_live::{ServoLiveEnsureRequest, ServoLiveFrame};
+
+use super::super::{
+    LiveRuntimeClient, LiveRuntimeClientError, LiveRuntimeWorker, RequestGeneration,
+};
+
+struct BlockingScrollClient {
+    deltas: Arc<Mutex<Vec<(i32, i32)>>>,
+    first_started_tx: Option<mpsc::Sender<()>>,
+    release_first_rx: mpsc::Receiver<()>,
+}
+
+impl LiveRuntimeClient for BlockingScrollClient {
+    fn ensure(
+        &mut self,
+        request: ServoLiveEnsureRequest,
+    ) -> Result<Option<ServoLiveFrame>, LiveRuntimeClientError> {
+        self.deltas
+            .lock()
+            .map_err(|_| "scroll recorder lock was poisoned".to_string())?
+            .push((request.scroll_delta_x, request.scroll_delta_y));
+        if let Some(first_started_tx) = self.first_started_tx.take() {
+            first_started_tx.send(()).map_err(|error| error.to_string())?;
+            self.release_first_rx.recv().map_err(|error| error.to_string())?;
+        }
+        Ok(None)
+    }
+
+    fn poll(&mut self, _tab_id: String) -> Result<Option<ServoLiveFrame>, LiveRuntimeClientError> {
+        Ok(None)
+    }
+
+    fn close(&mut self, _tab_id: String) -> Result<(), LiveRuntimeClientError> {
+        Ok(())
+    }
+}
+
+#[test]
+fn consecutive_scroll_updates_coalesce_without_losing_distance() -> Result<(), String> {
+    let deltas = Arc::new(Mutex::new(Vec::new()));
+    let client_deltas = deltas.clone();
+    let (first_started_tx, first_started_rx) = mpsc::channel();
+    let (release_first_tx, release_first_rx) = mpsc::channel();
+    let worker = LiveRuntimeWorker::new(move || {
+        Ok(Box::new(BlockingScrollClient {
+            deltas: client_deltas,
+            first_started_tx: Some(first_started_tx),
+            release_first_rx,
+        }))
+    })?;
+
+    worker.submit_ensure(RequestGeneration::new(1), ensure_request(0));
+    first_started_rx.recv_timeout(Duration::from_secs(1)).map_err(|error| error.to_string())?;
+    for generation in 2..=101 {
+        worker.submit_ensure(RequestGeneration::new(generation), ensure_request(1));
+    }
+    release_first_tx.send(()).map_err(|error| error.to_string())?;
+    worker.wait_until_idle();
+
+    assert_eq!(
+        *deltas.lock().map_err(|_| "scroll recorder lock was poisoned".to_string())?,
+        vec![(0, 0), (100, 100)]
+    );
+    Ok(())
+}
+
+fn ensure_request(scroll_delta: i32) -> ServoLiveEnsureRequest {
+    ServoLiveEnsureRequest {
+        tab_id: "tab-a".to_string(),
+        profile_id: "profile".to_string(),
+        url: "https://example.com/".to_string(),
+        width: 640,
+        height: 480,
+        page_zoom_percent: 100,
+        device_pixel_ratio: 1.0,
+        scroll_delta_x: scroll_delta,
+        scroll_delta_y: scroll_delta,
+        scroll_point_x: (scroll_delta != 0).then_some(1),
+        scroll_point_y: (scroll_delta != 0).then_some(1),
+        click_x: None,
+        click_y: None,
+        hover_x: None,
+        hover_y: None,
+        typed_text: None,
+        site_permission_generation: 1,
+        site_permissions: Vec::new(),
+        allow_once_grants: Vec::new(),
+    }
+}
@@ -464,3 +464,6 @@ fn ensure_request(tab_id: &str, input: RecordedInput) -> ServoLiveEnsureRequest
        allow_once_grants: Vec::new(),
    }
 }
+
+#[path = "web_surface_worker_scroll_tests.rs"]
+mod scroll_tests;