Add retry for hit tests with expired epoch in result

components/compositing/compositor.rs

@@ -281,6 +281,11 @@ impl PipelineDetails {
     }
 }
 
+pub enum HitTestError {
+    EpochMismatch,
+    Others,
+}
+
 impl ServoRenderer {
     pub fn shutdown_state(&self) -> ShutdownState {
         self.shutdown_state.get()
@@ -290,15 +295,19 @@ impl ServoRenderer {
         &self,
         point: DevicePoint,
         details_for_pipeline: impl Fn(PipelineId) -> Option<&'a PipelineDetails>,
-    ) -> Option<CompositorHitTestResult> {
-        self.hit_test_at_point_with_flags_and_pipeline(
+    ) -> Result<CompositorHitTestResult, HitTestError> {
+        match self.hit_test_at_point_with_flags_and_pipeline(
             point,
             HitTestFlags::empty(),
             None,
             details_for_pipeline,
-        )
-        .first()
-        .cloned()
+        ) {
+            Ok(hit_test_results) => hit_test_results
+                .first()
+                .cloned()
+                .ok_or(HitTestError::Others),
+            Err(error) => Err(error),
+        }
     }
 
     // TODO: split this into first half (global) and second half (one for whole compositor, one for webview)
@@ -308,25 +317,32 @@ impl ServoRenderer {
         flags: HitTestFlags,
         pipeline_id: Option<WebRenderPipelineId>,
         details_for_pipeline: impl Fn(PipelineId) -> Option<&'a PipelineDetails>,
-    ) -> Vec<CompositorHitTestResult> {
+    ) -> Result<Vec<CompositorHitTestResult>, HitTestError> {
         // DevicePoint and WorldPoint are the same for us.
         let world_point = WorldPoint::from_untyped(point.to_untyped());
         let results =
             self.webrender_api
                 .hit_test(self.webrender_document, pipeline_id, world_point, flags);
 
-        results
+        let mut epoch_mismatch = false;
+        let results = results
             .items
             .iter()
             .filter_map(|item| {
                 let pipeline_id = item.pipeline.into();
                 let details = details_for_pipeline(pipeline_id)?;
 
                 // If the epoch in the tag does not match the current epoch of the pipeline,
-                // then the hit test is against an old version of the display list and we
-                // should ignore this hit test for now.
+                // then the hit test is against an old version of the display list.
                 match details.most_recent_display_list_epoch {
-                    Some(epoch) if epoch.as_u16() == item.tag.1 => {},
+                    Some(epoch) => {
+                        if epoch.as_u16() != item.tag.1 {
+                            // It's too early to hit test for now.
+                            // New scene building is in progress.
+                            epoch_mismatch = true;
+                            return None;
+                        }
+                    },
                     _ => return None,
                 }
 
@@ -340,7 +356,13 @@ impl ServoRenderer {
                     scroll_tree_node: info.scroll_tree_node,
                 })
             })
-            .collect()
+            .collect();
+
+        if epoch_mismatch {
+            return Err(HitTestError::EpochMismatch);
+        }
+
+        Ok(results)
     }
 
     pub(crate) fn send_transaction(&mut self, transaction: Transaction) {
@@ -619,7 +641,7 @@ impl IOCompositor {
                         .global
                         .borrow()
                         .hit_test_at_point(point, details_for_pipeline);
-                    if let Some(result) = result {
+                    if let Ok(result) = result {
                         self.global.borrow_mut().update_cursor(point, &result);
                     }
                 }
@@ -649,7 +671,7 @@ impl IOCompositor {
                 };
                 let dppx = webview_renderer.device_pixels_per_page_pixel();
                 let point = dppx.transform_point(Point2D::new(x, y));
-                webview_renderer.dispatch_input_event(
+                webview_renderer.dispatch_point_input_event(
                     InputEvent::MouseButton(MouseButtonEvent::new(action, button, point))
                         .with_webdriver_message_id(Some(message_id)),
                 );
@@ -662,7 +684,7 @@ impl IOCompositor {
                 };
                 let dppx = webview_renderer.device_pixels_per_page_pixel();
                 let point = dppx.transform_point(Point2D::new(x, y));
-                webview_renderer.dispatch_input_event(
+                webview_renderer.dispatch_point_input_event(
                     InputEvent::MouseMove(MouseMoveEvent::new(point))
                         .with_webdriver_message_id(Some(message_id)),
                 );
@@ -684,7 +706,7 @@ impl IOCompositor {
                 let scroll_delta =
                     dppx.transform_vector(Vector2D::new(delta_x as f32, delta_y as f32));
                 webview_renderer
-                    .dispatch_input_event(InputEvent::Wheel(WheelEvent { delta, point }));
+                    .dispatch_point_input_event(InputEvent::Wheel(WheelEvent { delta, point }));
                 webview_renderer.on_webdriver_wheel_action(scroll_delta, point);
             },
 
@@ -792,6 +814,8 @@ impl IOCompositor {
                 let Some(webview_renderer) = self.webview_renderers.get_mut(webview_id) else {
                     return warn!("Could not find WebView for incoming display list");
                 };
+                // WebRender is not ready until we receive "NewWebRenderFrameReady"
+                webview_renderer.webrender_frame_ready.set(false);
 
                 let pipeline_id = display_list_info.pipeline_id;
                 let details = webview_renderer.ensure_pipeline_details(pipeline_id.into());
@@ -841,7 +865,8 @@ impl IOCompositor {
                         flags,
                         pipeline,
                         details_for_pipeline,
-                    );
+                    )
+                    .unwrap_or_default();
                 let _ = sender.send(result);
             },
 
@@ -1629,11 +1654,20 @@ impl IOCompositor {
                 },
                 CompositorMsg::NewWebRenderFrameReady(..) => {
                     found_recomposite_msg = true;
-                    compositor_messages.push(msg)
+                    compositor_messages.push(msg);
                 },
                 _ => compositor_messages.push(msg),
             }
         }
+
+        if found_recomposite_msg {
+            // Process all pending events
+            self.webview_renderers.iter().for_each(|webview| {
+                webview.dispatch_pending_point_input_events();
+                webview.webrender_frame_ready.set(true);
+            });
+        }
+
         for msg in compositor_messages {
             self.handle_browser_message(msg);
 

components/compositing/webview_renderer.rs

@@ -2,9 +2,9 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
-use std::cell::RefCell;
-use std::collections::HashMap;
+use std::cell::{Cell, RefCell};
 use std::collections::hash_map::Keys;
+use std::collections::{HashMap, VecDeque};
 use std::rc::Rc;
 
 use base::id::{PipelineId, WebViewId};
@@ -25,7 +25,7 @@ use webrender_api::units::{
 };
 use webrender_api::{ExternalScrollId, HitTestFlags, ScrollLocation};
 
-use crate::compositor::{PipelineDetails, ServoRenderer};
+use crate::compositor::{HitTestError, PipelineDetails, ServoRenderer};
 use crate::touch::{TouchHandler, TouchMoveAction, TouchMoveAllowed, TouchSequenceState};
 
 // Default viewport constraints
@@ -98,6 +98,10 @@ pub(crate) struct WebViewRenderer {
     /// Whether or not this [`WebViewRenderer`] isn't throttled and has a pipeline with
     /// active animations or animation frame callbacks.
     animating: bool,
+    /// Pending input events queue. Priavte and only this thread pushes events to it.
+    pending_point_input_events: RefCell<VecDeque<InputEvent>>,
+    /// WebRender is not ready between `SendDisplayList` and `WebRenderFrameReady` messages.
+    pub webrender_frame_ready: Cell<bool>,
 }
 
 impl Drop for WebViewRenderer {
@@ -132,6 +136,8 @@ impl WebViewRenderer {
             max_viewport_zoom: None,
             hidpi_scale_factor: Scale::new(hidpi_scale_factor.0),
             animating: false,
+            pending_point_input_events: Default::default(),
+            webrender_frame_ready: Cell::default(),
         }
     }
 
@@ -309,29 +315,89 @@ impl WebViewRenderer {
         }
     }
 
-    pub(crate) fn dispatch_input_event(&mut self, event: InputEvent) {
+    pub(crate) fn dispatch_point_input_event(&mut self, mut event: InputEvent) -> bool {
         // Events that do not need to do hit testing are sent directly to the
         // constellation to filter down.
         let Some(point) = event.point() else {
-            return;
+            return false;
         };
 
+        // Delay the event if the epoch is not synchronized yet (new frame is not ready),
+        // or hit test result would fail and the event is rejected anyway.
+        if !self.webrender_frame_ready.get() || !self.pending_point_input_events.borrow().is_empty()
+        {
+            self.pending_point_input_events
+                .borrow_mut()
+                .push_back(event);
+            return false;
+        }
+
         // If we can't find a pipeline to send this event to, we cannot continue.
         let get_pipeline_details = |pipeline_id| self.pipelines.get(&pipeline_id);
-        let Some(result) = self
+        let result = match self
             .global
             .borrow()
             .hit_test_at_point(point, get_pipeline_details)
-        else {
-            return;
+        {
+            Ok(hit_test_results) => hit_test_results,
+            Err(HitTestError::EpochMismatch) => {
+                self.pending_point_input_events
+                    .borrow_mut()
+                    .push_back(event);
+                return false;
+            },
+            _ => {
+                return false;
+            },
         };
 
-        self.global.borrow_mut().update_cursor(point, &result);
+        match event {
+            InputEvent::Touch(ref mut touch_event) => {
+                touch_event.init_sequence_id(self.touch_handler.current_sequence_id);
+            },
+            InputEvent::MouseButton(_) | InputEvent::MouseMove(_) | InputEvent::Wheel(_) => {
+                self.global.borrow_mut().update_cursor(point, &result);
+            },
+            _ => unreachable!("Unexpected input event type: {event:?}"),
+        }
 
         if let Err(error) = self.global.borrow().constellation_sender.send(
             EmbedderToConstellationMessage::ForwardInputEvent(self.id, event, Some(result)),
         ) {
             warn!("Sending event to constellation failed ({error:?}).");
+            false
+        } else {
+            true
+        }
+    }
+
+    pub(crate) fn dispatch_pending_point_input_events(&self) {
+        while let Some(event) = self.pending_point_input_events.borrow_mut().pop_front() {
+            // Events that do not need to do hit testing are sent directly to the
+            // constellation to filter down.
+            let Some(point) = event.point() else {
+                continue;
+            };
+
+            // If we can't find a pipeline to send this event to, we cannot continue.
+            let get_pipeline_details = |pipeline_id| self.pipelines.get(&pipeline_id);
+            let Ok(result) = self
+                .global
+                .borrow()
+                .hit_test_at_point(point, get_pipeline_details)
+            else {
+                // Don't need to process pending input events in this frame any more.
+                // TODO: Add multiple retry later if needed.
+                return;
+            };
+
+            self.global.borrow_mut().update_cursor(point, &result);
+
+            if let Err(error) = self.global.borrow().constellation_sender.send(
+                EmbedderToConstellationMessage::ForwardInputEvent(self.id, event, Some(result)),
+            ) {
+                warn!("Sending event to constellation failed ({error:?}).");
+            }
         }
     }
 
@@ -401,29 +467,11 @@ impl WebViewRenderer {
             }
         }
 
-        self.dispatch_input_event(event);
+        self.dispatch_point_input_event(event);
     }
 
-    fn send_touch_event(&self, mut event: TouchEvent) -> bool {
-        let get_pipeline_details = |pipeline_id| self.pipelines.get(&pipeline_id);
-        let Some(result) = self
-            .global
-            .borrow()
-            .hit_test_at_point(event.point, get_pipeline_details)
-        else {
-            return false;
-        };
-
-        event.init_sequence_id(self.touch_handler.current_sequence_id);
-        let event = InputEvent::Touch(event);
-        if let Err(e) = self.global.borrow().constellation_sender.send(
-            EmbedderToConstellationMessage::ForwardInputEvent(self.id, event, Some(result)),
-        ) {
-            warn!("Sending event to constellation failed ({:?}).", e);
-            false
-        } else {
-            true
-        }
+    fn send_touch_event(&mut self, event: TouchEvent) -> bool {
+        self.dispatch_point_input_event(InputEvent::Touch(event))
     }
 
     pub(crate) fn on_touch_event(&mut self, event: TouchEvent) {
@@ -687,13 +735,13 @@ impl WebViewRenderer {
     /// <http://w3c.github.io/touch-events/#mouse-events>
     fn simulate_mouse_click(&mut self, point: DevicePoint) {
         let button = MouseButton::Left;
-        self.dispatch_input_event(InputEvent::MouseMove(MouseMoveEvent::new(point)));
-        self.dispatch_input_event(InputEvent::MouseButton(MouseButtonEvent::new(
+        self.dispatch_point_input_event(InputEvent::MouseMove(MouseMoveEvent::new(point)));
+        self.dispatch_point_input_event(InputEvent::MouseButton(MouseButtonEvent::new(
             MouseButtonAction::Down,
             button,
             point,
         )));
-        self.dispatch_input_event(InputEvent::MouseButton(MouseButtonEvent::new(
+        self.dispatch_point_input_event(InputEvent::MouseButton(MouseButtonEvent::new(
             MouseButtonAction::Up,
             button,
             point,
@@ -858,7 +906,8 @@ impl WebViewRenderer {
                 HitTestFlags::FIND_ALL,
                 None,
                 get_pipeline_details,
-            );
+            )
+            .unwrap_or_default();
 
         // Iterate through all hit test results, processing only the first node of each pipeline.
         // This is needed to propagate the scroll events from a pipeline representing an iframe to