Fix a crash in the touch handler.

If some move events take the long route over the script thread but get
cleaned up by a fling the late one will crash the handler.
Now we just ignore move events that are already cleaned up.

Signed-off-by: Narfinger <Narfinger@users.noreply.github.com>
This commit is contained in:
Narfinger 2025-05-08 15:51:20 +02:00
parent 5bb60f47fc
commit 7de9716dc0
4 changed files with 129 additions and 129 deletions

View file

@ -292,6 +292,10 @@ impl TouchHandler {
.expect("Current Touch sequence does not exist")
}
fn try_get_current_touch_sequence_mut(&mut self) -> Option<&mut TouchSequenceInfo> {
self.touch_sequence_map.get_mut(&self.current_sequence_id)
}
pub(crate) fn get_touch_sequence(&self, sequence_id: TouchSequenceId) -> &TouchSequenceInfo {
self.touch_sequence_map
.get(&sequence_id)
@ -374,81 +378,87 @@ impl TouchHandler {
id: TouchId,
point: Point2D<f32, DevicePixel>,
) -> TouchMoveAction {
let touch_sequence = self.get_current_touch_sequence_mut();
let idx = match touch_sequence
.active_touch_points
.iter_mut()
.position(|t| t.id == id)
{
Some(i) => i,
None => {
error!("Got a touchmove event for a non-active touch point");
return TouchMoveAction::NoAction;
},
};
let old_point = touch_sequence.active_touch_points[idx].point;
let delta = point - old_point;
// As `TouchHandler` is per `WebViewRenderer` which is per `WebView` we might get a Touch Sequence Move that
// started with a down on a different webview. As the touch_sequence id is only changed on touch_down this
// move event gets a touch id which is already cleaned up.
if let Some(touch_sequence) = self.try_get_current_touch_sequence_mut() {
let idx = match touch_sequence
.active_touch_points
.iter_mut()
.position(|t| t.id == id)
{
Some(i) => i,
None => {
error!("Got a touchmove event for a non-active touch point");
return TouchMoveAction::NoAction;
},
};
let old_point = touch_sequence.active_touch_points[idx].point;
let delta = point - old_point;
let action = match touch_sequence.touch_count() {
1 => {
if let Panning { ref mut velocity } = touch_sequence.state {
// TODO: Probably we should track 1-3 more points and use a smarter algorithm
*velocity += delta;
*velocity /= 2.0;
// update the touch point every time when panning.
let action = match touch_sequence.touch_count() {
1 => {
if let Panning { ref mut velocity } = touch_sequence.state {
// TODO: Probably we should track 1-3 more points and use a smarter algorithm
*velocity += delta;
*velocity /= 2.0;
// update the touch point every time when panning.
touch_sequence.active_touch_points[idx].point = point;
TouchMoveAction::Scroll(delta, point)
} else if delta.x.abs() > TOUCH_PAN_MIN_SCREEN_PX ||
delta.y.abs() > TOUCH_PAN_MIN_SCREEN_PX
{
touch_sequence.state = Panning {
velocity: Vector2D::new(delta.x, delta.y),
};
// No clicks should be issued after we transitioned to move.
touch_sequence.prevent_click = true;
// update the touch point
touch_sequence.active_touch_points[idx].point = point;
TouchMoveAction::Scroll(delta, point)
} else {
// We don't update the touchpoint, so multiple small moves can
// accumulate and merge into a larger move.
TouchMoveAction::NoAction
}
},
2 => {
if touch_sequence.state == Pinching ||
delta.x.abs() > TOUCH_PAN_MIN_SCREEN_PX ||
delta.y.abs() > TOUCH_PAN_MIN_SCREEN_PX
{
touch_sequence.state = Pinching;
let (d0, c0) = touch_sequence.pinch_distance_and_center();
// update the touch point with the enough distance or pinching.
touch_sequence.active_touch_points[idx].point = point;
let (d1, c1) = touch_sequence.pinch_distance_and_center();
let magnification = d1 / d0;
let scroll_delta = c1 - c0 * Scale::new(magnification);
TouchMoveAction::Zoom(magnification, scroll_delta)
} else {
// We don't update the touchpoint, so multiple small moves can
// accumulate and merge into a larger move.
TouchMoveAction::NoAction
}
},
_ => {
touch_sequence.active_touch_points[idx].point = point;
TouchMoveAction::Scroll(delta, point)
} else if delta.x.abs() > TOUCH_PAN_MIN_SCREEN_PX ||
delta.y.abs() > TOUCH_PAN_MIN_SCREEN_PX
{
touch_sequence.state = Panning {
velocity: Vector2D::new(delta.x, delta.y),
};
// No clicks should be issued after we transitioned to move.
touch_sequence.prevent_click = true;
// update the touch point
touch_sequence.active_touch_points[idx].point = point;
TouchMoveAction::Scroll(delta, point)
} else {
// We don't update the touchpoint, so multiple small moves can
// accumulate and merge into a larger move.
touch_sequence.state = MultiTouch;
TouchMoveAction::NoAction
}
},
2 => {
if touch_sequence.state == Pinching ||
delta.x.abs() > TOUCH_PAN_MIN_SCREEN_PX ||
delta.y.abs() > TOUCH_PAN_MIN_SCREEN_PX
{
touch_sequence.state = Pinching;
let (d0, c0) = touch_sequence.pinch_distance_and_center();
// update the touch point with the enough distance or pinching.
touch_sequence.active_touch_points[idx].point = point;
let (d1, c1) = touch_sequence.pinch_distance_and_center();
let magnification = d1 / d0;
let scroll_delta = c1 - c0 * Scale::new(magnification);
TouchMoveAction::Zoom(magnification, scroll_delta)
} else {
// We don't update the touchpoint, so multiple small moves can
// accumulate and merge into a larger move.
TouchMoveAction::NoAction
}
},
_ => {
touch_sequence.active_touch_points[idx].point = point;
touch_sequence.state = MultiTouch;
TouchMoveAction::NoAction
},
};
// If the touch action is not `NoAction` and the first move has not been processed,
// set pending_touch_move_action.
if TouchMoveAction::NoAction != action &&
touch_sequence.prevent_move == TouchMoveAllowed::Pending
{
touch_sequence.update_pending_touch_move_action(action);
},
};
// If the touch action is not `NoAction` and the first move has not been processed,
// set pending_touch_move_action.
if TouchMoveAction::NoAction != action &&
touch_sequence.prevent_move == TouchMoveAllowed::Pending
{
touch_sequence.update_pending_touch_move_action(action);
}
action
} else {
TouchMoveAction::NoAction
}
action
}
pub fn on_touch_up(&mut self, id: TouchId, point: Point2D<f32, DevicePixel>) {
@ -529,22 +539,24 @@ impl TouchHandler {
}
pub fn on_touch_cancel(&mut self, id: TouchId, _point: Point2D<f32, DevicePixel>) {
let touch_sequence = self.get_current_touch_sequence_mut();
match touch_sequence
.active_touch_points
.iter()
.position(|t| t.id == id)
{
Some(i) => {
touch_sequence.active_touch_points.swap_remove(i);
},
None => {
warn!("Got a touchcancel event for a non-active touch point");
return;
},
}
if touch_sequence.active_touch_points.is_empty() {
touch_sequence.state = Finished;
// A similar thing with touch move can happen here where the event is coming from a different webview.
if let Some(touch_sequence) = self.try_get_current_touch_sequence_mut() {
match touch_sequence
.active_touch_points
.iter()
.position(|t| t.id == id)
{
Some(i) => {
touch_sequence.active_touch_points.swap_remove(i);
},
None => {
warn!("Got a touchcancel event for a non-active touch point");
return;
},
}
if touch_sequence.active_touch_points.is_empty() {
touch_sequence.state = Finished;
}
}
}
}

View file

@ -404,6 +404,7 @@ impl WebViewRenderer {
self.dispatch_input_event(event);
}
/// Send touch event to the pipeline. Returns true if the event was send
fn send_touch_event(&self, mut event: TouchEvent) -> bool {
let get_pipeline_details = |pipeline_id| self.pipelines.get(&pipeline_id);
let Some(result) = self

View file

@ -124,6 +124,7 @@ pub enum TouchEventType {
pub struct TouchId(pub i32);
/// An ID for a sequence of touch events between a `Down` and the `Up` or `Cancel` event.
/// The ID is the same for all events between `Down`` and `Up`` or `Cancel``
#[repr(transparent)]
#[derive(Clone, Copy, Debug, Default, Deserialize, Eq, Hash, PartialEq, Serialize)]
pub struct TouchSequenceId(u32);

View file

@ -96,9 +96,6 @@ struct RunningAppStateInner {
/// The HiDPI scaling factor to use for the display of [`WebView`]s.
hidpi_scale_factor: Scale<f32, DeviceIndependentPixel, DevicePixel>,
/// Touch events should not be processed if the compositor is paused
compositor_paused: bool,
}
struct ServoShellServoDelegate {
@ -317,7 +314,6 @@ impl RunningAppState {
focused_webview_id: None,
animating_state_changed,
hidpi_scale_factor: Scale::new(hidpi_scale_factor),
compositor_paused: false,
}),
});
@ -509,54 +505,46 @@ impl RunningAppState {
/// Touch event: press down
pub fn touch_down(&self, x: f32, y: f32, pointer_id: i32) {
if !self.inner().compositor_paused {
self.active_webview()
.notify_input_event(InputEvent::Touch(TouchEvent::new(
TouchEventType::Down,
TouchId(pointer_id),
Point2D::new(x, y),
)));
self.perform_updates();
}
self.active_webview()
.notify_input_event(InputEvent::Touch(TouchEvent::new(
TouchEventType::Down,
TouchId(pointer_id),
Point2D::new(x, y),
)));
self.perform_updates();
}
/// Touch event: move touching finger
pub fn touch_move(&self, x: f32, y: f32, pointer_id: i32) {
if !self.inner().compositor_paused {
self.active_webview()
.notify_input_event(InputEvent::Touch(TouchEvent::new(
TouchEventType::Move,
TouchId(pointer_id),
Point2D::new(x, y),
)));
self.perform_updates();
}
self.active_webview()
.notify_input_event(InputEvent::Touch(TouchEvent::new(
TouchEventType::Move,
TouchId(pointer_id),
Point2D::new(x, y),
)));
self.perform_updates();
}
/// Touch event: Lift touching finger
pub fn touch_up(&self, x: f32, y: f32, pointer_id: i32) {
if !self.inner().compositor_paused {
self.active_webview()
.notify_input_event(InputEvent::Touch(TouchEvent::new(
TouchEventType::Up,
TouchId(pointer_id),
Point2D::new(x, y),
)));
self.perform_updates();
}
self.active_webview()
.notify_input_event(InputEvent::Touch(TouchEvent::new(
TouchEventType::Up,
TouchId(pointer_id),
Point2D::new(x, y),
)));
self.perform_updates();
}
/// Cancel touch event
pub fn touch_cancel(&self, x: f32, y: f32, pointer_id: i32) {
if !self.inner().compositor_paused {
self.active_webview()
.notify_input_event(InputEvent::Touch(TouchEvent::new(
TouchEventType::Cancel,
TouchId(pointer_id),
Point2D::new(x, y),
)));
self.perform_updates();
}
self.active_webview()
.notify_input_event(InputEvent::Touch(TouchEvent::new(
TouchEventType::Cancel,
TouchId(pointer_id),
Point2D::new(x, y),
)));
self.perform_updates();
}
/// Register a mouse movement.
@ -662,7 +650,6 @@ impl RunningAppState {
if let Err(e) = self.rendering_context.take_window() {
warn!("Unbinding native surface from context failed ({:?})", e);
}
self.inner_mut().compositor_paused = true;
self.perform_updates();
}
@ -675,7 +662,6 @@ impl RunningAppState {
{
warn!("Binding native surface to context failed ({:?})", e);
}
self.inner_mut().compositor_paused = false;
self.perform_updates();
}