Skip to content

GitLab

T
tangled
Commit 2d5f6468 authored by jdduke's avatar jdduke Committed by Commit bot
Browse files
Options

Defer flushing the touch queue when all touch handlers removed 

Currently, the touch event queue will flush itself when it's notified
that all touch handlers have been removed. However, this effectively
overrides the ack of any outstanding touch that is being handled. In
particular, the touch may have been the cause of handler removal *and*
been preventDefault'ed, in which case we should respect the
preventDeafult.

Defer flushing the queue when all handlers have been removed, instead
waiting for any outstanding touch ack. Any additional pending events
will be automatically flushed when the ack is received.

BUG=406916

Review URL: https://codereview.chromium.org/521453002

Cr-Commit-Position: refs/heads/master@{#292685}
parent 8d7df02b
Hide whitespace changes
Inline Side-by-side
Showing with 70 additions and 50 deletions
content/browser/renderer_host/input/input_router_impl_unittest.cc
View file @ 2d5f6468
......@@ -619,8 +619,8 @@ TEST_F(InputRouterImplTest, TouchEventQueue) {
EXPECT_EQ(0U, GetSentMessageCountAndResetSink());
}
// Tests that the touch-queue is emptied if a page stops listening for touch
// events.
// Tests that the touch-queue is emptied after a page stops listening for touch
// events and the outstanding ack is received.
TEST_F(InputRouterImplTest, TouchEventQueueFlush) {
OnHasTouchEventHandlers(true);
EXPECT_TRUE(client_->has_touch_handler());
......@@ -632,15 +632,23 @@ TEST_F(InputRouterImplTest, TouchEventQueueFlush) {
// Send a touch-press event.
PressTouchPoint(1, 1);
SendTouchEvent();
MoveTouchPoint(0, 2, 2);
MoveTouchPoint(0, 3, 3);
EXPECT_FALSE(TouchEventQueueEmpty());
EXPECT_EQ(1U, GetSentMessageCountAndResetSink());
// The page stops listening for touch-events. The touch-event queue should now
// be emptied, but none of the queued touch-events should be sent to the
// renderer.
// The page stops listening for touch-events. Note that flushing is deferred
// until the outstanding ack is received.
OnHasTouchEventHandlers(false);
EXPECT_FALSE(client_->has_touch_handler());
EXPECT_EQ(0U, GetSentMessageCountAndResetSink());
EXPECT_FALSE(TouchEventQueueEmpty());
EXPECT_TRUE(input_router_->ShouldForwardTouchEvent());
// After the ack, the touch-event queue should be empty, and none of the
// flushed touch-events should have been sent to the renderer.
SendInputEventACK(WebInputEvent::TouchStart, INPUT_EVENT_ACK_STATE_CONSUMED);
EXPECT_EQ(0U, GetSentMessageCountAndResetSink());
EXPECT_TRUE(TouchEventQueueEmpty());
EXPECT_FALSE(input_router_->ShouldForwardTouchEvent());
}
......
content/browser/renderer_host/input/touch_event_queue.cc
View file @ 2d5f6468
......@@ -637,10 +637,6 @@ void TouchEventQueue::OnHasTouchEventHandlers(bool has_handlers) {
pending_async_touchmove_.reset();
if (timeout_handler_)
timeout_handler_->Reset();
if (!touch_queue_.empty())
ProcessTouchAck(INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS, LatencyInfo());
// As there is no touch handler, ack'ing the event should flush the queue.
DCHECK(touch_queue_.empty());
}
}
......
content/browser/renderer_host/input/touch_event_queue_unittest.cc
View file @ 2d5f6468
......@@ -285,9 +285,9 @@ TEST_F(TouchEventQueueTest, Basic) {
EXPECT_TRUE(acked_event().cancelable);
}
// Tests that the touch-queue is emptied if a page stops listening for touch
// events.
TEST_F(TouchEventQueueTest, QueueFlushedWhenHandlersRemoved) {
// Tests that the touch-queue is emptied after the outstanding ack is received
// if a page stops listening for touch events.
TEST_F(TouchEventQueueTest, QueueFlushedOnAckAfterHandlersRemoved) {
OnHasTouchEventHandlers(true);
EXPECT_EQ(0U, queued_event_count());
EXPECT_EQ(0U, GetAndResetSentEventCount());
......@@ -295,32 +295,51 @@ TEST_F(TouchEventQueueTest, QueueFlushedWhenHandlersRemoved) {
// Send a touch-press event.
PressTouchPoint(1, 1);
EXPECT_EQ(1U, GetAndResetSentEventCount());
EXPECT_EQ(1U, queued_event_count());
// Signal that all touch handlers have been removed.
OnHasTouchEventHandlers(false);
EXPECT_EQ(0U, GetAndResetAckedEventCount());
EXPECT_EQ(1U, queued_event_count());
// Process the ack for the sent touch, ensuring that it is honored (despite
// the touch handler having been removed).
SendTouchEventAck(INPUT_EVENT_ACK_STATE_CONSUMED);
EXPECT_EQ(1U, GetAndResetAckedEventCount());
EXPECT_EQ(0U, queued_event_count());
EXPECT_EQ(INPUT_EVENT_ACK_STATE_CONSUMED, acked_event_state());
// The release should not be forwarded.
ReleaseTouchPoint(0);
EXPECT_EQ(1U, GetAndResetAckedEventCount());
EXPECT_EQ(0U, queued_event_count());
EXPECT_EQ(INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS, acked_event_state());
OnHasTouchEventHandlers(true);
// Events will be queued until the first sent event is ack'ed.
for (int i = 5; i < 15; ++i) {
for (int i = 0; i < 10; ++i) {
PressTouchPoint(1, 1);
MoveTouchPoint(0, i, i);
ReleaseTouchPoint(0);
}
EXPECT_EQ(32U, queued_event_count());
EXPECT_EQ(0U, GetAndResetSentEventCount());
// Receive an ACK for the first touch-event. One of the queued touch-event
// should be forwarded.
SendTouchEventAck(INPUT_EVENT_ACK_STATE_CONSUMED);
EXPECT_EQ(31U, queued_event_count());
EXPECT_EQ(30U, queued_event_count());
EXPECT_EQ(1U, GetAndResetSentEventCount());
EXPECT_EQ(1U, GetAndResetAckedEventCount());
EXPECT_EQ(WebInputEvent::TouchStart, acked_event().type);
// Flush the queue. The touch-event queue should now be emptied, but none of
// the queued touch-events should be sent to the renderer.
// Signal that all touch handlers have been removed. Note that flushing of
// the queue will not occur until *after* the outstanding ack is received.
OnHasTouchEventHandlers(false);
EXPECT_EQ(30U, queued_event_count());
EXPECT_EQ(0U, GetAndResetSentEventCount());
EXPECT_EQ(0U, GetAndResetAckedEventCount());
// Receive an ACK for the first touch-event. All remaining touch events should
// be flushed with the appropriate ack type.
SendTouchEventAck(INPUT_EVENT_ACK_STATE_CONSUMED);
EXPECT_EQ(0U, queued_event_count());
EXPECT_EQ(0U, GetAndResetSentEventCount());
EXPECT_EQ(31U, GetAndResetAckedEventCount());
EXPECT_EQ(30U, GetAndResetAckedEventCount());
EXPECT_EQ(INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS, acked_event_state());
}
// Tests that addition of a touch handler during a touch sequence will not cause
......@@ -364,14 +383,14 @@ TEST_F(TouchEventQueueTest, ActiveSequenceDroppedWhenHandlersRemoved) {
EXPECT_EQ(0U, GetAndResetAckedEventCount());
EXPECT_EQ(0U, GetAndResetSentEventCount());
// Touch handle deregistration should flush the queue.
// Unregister all touch handlers.
OnHasTouchEventHandlers(false);
EXPECT_EQ(2U, GetAndResetAckedEventCount());
EXPECT_EQ(0U, queued_event_count());
EXPECT_EQ(0U, GetAndResetAckedEventCount());
EXPECT_EQ(2U, queued_event_count());
// The ack should be ignored as the touch queue is now empty.
// The ack should be flush the queue.
SendTouchEventAck(INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS);
EXPECT_EQ(0U, GetAndResetAckedEventCount());
EXPECT_EQ(2U, GetAndResetAckedEventCount());
EXPECT_EQ(0U, queued_event_count());
// Events should be dropped while there is no touch handler.
......@@ -496,33 +515,23 @@ TEST_F(TouchEventQueueTest, MultiTouch) {
EXPECT_EQ(WebTouchPoint::StateMoved, event.touches[1].state);
}
// Tests that if a touch-event queue is destroyed in response to a touch-event
// in the renderer, then there is no crash when the ACK for that touch-event
// comes back.
TEST_F(TouchEventQueueTest, AckAfterQueueFlushed) {
// Send some touch-events to the renderer.
// Tests that the touch-event queue is robust to redundant acks.
TEST_F(TouchEventQueueTest, SpuriousAcksIgnored) {
// Trigger a spurious ack.
SendTouchEventAck(INPUT_EVENT_ACK_STATE_CONSUMED);
EXPECT_EQ(0U, GetAndResetAckedEventCount());
// Send and ack a touch press.
PressTouchPoint(1, 1);
EXPECT_EQ(1U, GetAndResetSentEventCount());
EXPECT_EQ(1U, queued_event_count());
MoveTouchPoint(0, 10, 10);
EXPECT_EQ(0U, GetAndResetSentEventCount());
EXPECT_EQ(2U, queued_event_count());
// Receive an ACK for the press. This should cause the queued touch-move to
// be sent to the renderer.
SendTouchEventAck(INPUT_EVENT_ACK_STATE_CONSUMED);
EXPECT_EQ(1U, GetAndResetSentEventCount());
EXPECT_EQ(1U, queued_event_count());
OnHasTouchEventHandlers(false);
EXPECT_EQ(0U, GetAndResetSentEventCount());
EXPECT_EQ(1U, GetAndResetAckedEventCount());
EXPECT_EQ(0U, queued_event_count());
// Now receive an ACK for the move.
// Trigger a spurious ack.
SendTouchEventAck(INPUT_EVENT_ACK_STATE_CONSUMED);
EXPECT_EQ(0U, GetAndResetSentEventCount());
EXPECT_EQ(0U, queued_event_count());
EXPECT_EQ(0U, GetAndResetAckedEventCount());
}
// Tests that touch-move events are not sent to the renderer if the preceding
......
content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
View file @ 2d5f6468
......@@ -889,6 +889,13 @@ TEST_F(RenderWidgetHostViewAuraTest, TouchEventState) {
view_->touch_event_.touches[0].state);
widget_host_->OnMessageReceived(ViewHostMsg_HasTouchEventHandlers(0, false));
EXPECT_TRUE(widget_host_->ShouldForwardTouchEvent());
// Ack'ing the outstanding event should flush the pending touch queue.
InputHostMsg_HandleInputEvent_ACK_Params ack;
ack.type = blink::WebInputEvent::TouchStart;
ack.state = INPUT_EVENT_ACK_STATE_CONSUMED;
widget_host_->OnMessageReceived(InputHostMsg_HandleInputEvent_ACK(0, ack));
EXPECT_FALSE(widget_host_->ShouldForwardTouchEvent());
ui::TouchEvent move2(ui::ET_TOUCH_MOVED, gfx::Point(20, 20), 0,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment