diff options
author | jdduke <jdduke@chromium.org> | 2014-09-23 09:49:20 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2014-09-23 16:49:46 +0000 |
commit | c883176fd86dee2be74dd5e3c6a101f0e4f93bee (patch) | |
tree | fd21e30a1c4ef84b7dd6dd9da093f021d1cddb39 /content/browser | |
parent | c23aafa62cd29ef0cd48938e16370913d513b529 (diff) | |
download | chromium_src-c883176fd86dee2be74dd5e3c6a101f0e4f93bee.zip chromium_src-c883176fd86dee2be74dd5e3c6a101f0e4f93bee.tar.gz chromium_src-c883176fd86dee2be74dd5e3c6a101f0e4f93bee.tar.bz2 |
Allow repeated handler removal/addition with the TouchEventQueue
A valid scenario, observed in the wild, is a touchstart handler removing
itself and adding a touchmove handler. If those are the only touch
handlers on the document, the renderer will forward two distinct handler
existence notifications to the browser. This causes the TouchEventQueue
to effectively drop the remainder of the touch sequence, even though the
element targetted by the touchstart now has a touchmove.
Fix this scenario by making the TouchEventQueue effectively idempotent
to repeated addition and removal of touch handlers. In practice, this
means simplifying its statefulness and instead relying on the existing
pointer id state map to determine whether to forward touches for the
remainder of the sequence.
BUG=412723
Review URL: https://codereview.chromium.org/586553002
Cr-Commit-Position: refs/heads/master@{#296205}
Diffstat (limited to 'content/browser')
5 files changed, 260 insertions, 219 deletions
diff --git a/content/browser/renderer_host/input/input_router_impl_unittest.cc b/content/browser/renderer_host/input/input_router_impl_unittest.cc index b3a535f..1bed763 100644 --- a/content/browser/renderer_host/input/input_router_impl_unittest.cc +++ b/content/browser/renderer_host/input/input_router_impl_unittest.cc @@ -284,7 +284,7 @@ class InputRouterImplTest : public testing::Test { } bool TouchEventTimeoutEnabled() const { - return input_router()->touch_event_queue_.ack_timeout_enabled(); + return input_router()->touch_event_queue_.IsAckTimeoutEnabled(); } void Flush() const { @@ -1105,53 +1105,40 @@ TEST_F(InputRouterImplTest, // Start a touch sequence. PressTouchPoint(1, 1); SendTouchEvent(); + EXPECT_EQ(1U, GetSentMessageCountAndResetSink()); // TOUCH_ACTION_NONE should disable the timeout. OnSetTouchAction(TOUCH_ACTION_NONE); SendInputEventACK(WebInputEvent::TouchStart, INPUT_EVENT_ACK_STATE_CONSUMED); + EXPECT_EQ(1U, ack_handler_->GetAndResetAckCount()); EXPECT_FALSE(TouchEventTimeoutEnabled()); - // End the touch sequence. - ReleaseTouchPoint(0); + MoveTouchPoint(0, 1, 1); SendTouchEvent(); - SendInputEventACK(WebInputEvent::TouchEnd, INPUT_EVENT_ACK_STATE_CONSUMED); EXPECT_FALSE(TouchEventTimeoutEnabled()); - ack_handler_->GetAndResetAckCount(); - GetSentMessageCountAndResetSink(); - - // Start another touch sequence. While this does restore the touch timeout - // the timeout will not apply until the *next* touch sequence. This affords a - // touch-action response from the renderer without racing against the timeout. - PressTouchPoint(1, 1); - SendTouchEvent(); - EXPECT_TRUE(TouchEventTimeoutEnabled()); EXPECT_EQ(1U, GetSentMessageCountAndResetSink()); - // Delay the ack. The timeout should *not* fire. + // Delay the move ack. The timeout should not fire. RunTasksAndWait(base::TimeDelta::FromMilliseconds(timeout_ms + 1)); EXPECT_EQ(0U, ack_handler_->GetAndResetAckCount()); EXPECT_EQ(0U, GetSentMessageCountAndResetSink()); - - // Finally send the ack. The touch sequence should not have been cancelled. - SendInputEventACK(WebInputEvent::TouchStart, - INPUT_EVENT_ACK_STATE_NOT_CONSUMED); - EXPECT_TRUE(TouchEventTimeoutEnabled()); + SendInputEventACK(WebInputEvent::TouchEnd, INPUT_EVENT_ACK_STATE_CONSUMED); EXPECT_EQ(1U, ack_handler_->GetAndResetAckCount()); - EXPECT_EQ(0U, GetSentMessageCountAndResetSink()); - // End the sequence. + // End the touch sequence. ReleaseTouchPoint(0); SendTouchEvent(); SendInputEventACK(WebInputEvent::TouchEnd, INPUT_EVENT_ACK_STATE_CONSUMED); - EXPECT_EQ(1U, ack_handler_->GetAndResetAckCount()); - EXPECT_EQ(1U, GetSentMessageCountAndResetSink()); + EXPECT_FALSE(TouchEventTimeoutEnabled()); + ack_handler_->GetAndResetAckCount(); + GetSentMessageCountAndResetSink(); - // A new touch sequence should (finally) be subject to the timeout. + // Start another touch sequence. This should restore the touch timeout. PressTouchPoint(1, 1); SendTouchEvent(); EXPECT_TRUE(TouchEventTimeoutEnabled()); - EXPECT_EQ(0U, ack_handler_->GetAndResetAckCount()); EXPECT_EQ(1U, GetSentMessageCountAndResetSink()); + EXPECT_EQ(0U, ack_handler_->GetAndResetAckCount()); // Wait for the touch ack timeout to fire. RunTasksAndWait(base::TimeDelta::FromMilliseconds(timeout_ms + 1)); diff --git a/content/browser/renderer_host/input/touch_event_queue.cc b/content/browser/renderer_host/input/touch_event_queue.cc index 8b03c88..7edb363 100644 --- a/content/browser/renderer_host/input/touch_event_queue.cc +++ b/content/browser/renderer_host/input/touch_event_queue.cc @@ -69,15 +69,28 @@ class TouchEventQueue::TouchTimeoutHandler { timeout_delay_(timeout_delay), pending_ack_state_(PENDING_ACK_NONE), timeout_monitor_(base::Bind(&TouchTimeoutHandler::OnTimeOut, - base::Unretained(this))) { + base::Unretained(this))), + enabled_(true), + enabled_for_current_sequence_(false) { DCHECK(timeout_delay != base::TimeDelta()); } ~TouchTimeoutHandler() {} - void Start(const TouchEventWithLatencyInfo& event) { + void StartIfNecessary(const TouchEventWithLatencyInfo& event) { DCHECK_EQ(pending_ack_state_, PENDING_ACK_NONE); - DCHECK(ShouldTouchTriggerTimeout(event.event)); + if (!enabled_) + return; + + if (!ShouldTouchTriggerTimeout(event.event)) + return; + + if (WebTouchEventTraits::IsTouchSequenceStart(event.event)) + enabled_for_current_sequence_ = true; + + if (!enabled_for_current_sequence_) + return; + timeout_event_ = event; timeout_monitor_.Restart(timeout_delay_); } @@ -85,6 +98,8 @@ class TouchEventQueue::TouchTimeoutHandler { bool ConfirmTouchEvent(InputEventAckState ack_result) { switch (pending_ack_state_) { case PENDING_ACK_NONE: + if (ack_result == INPUT_EVENT_ACK_STATE_CONSUMED) + enabled_for_current_sequence_ = false; timeout_monitor_.Stop(); return false; case PENDING_ACK_ORIGINAL_EVENT: @@ -95,7 +110,8 @@ class TouchEventQueue::TouchTimeoutHandler { touch_queue_->SendTouchEventImmediately(cancel_event); } else { SetPendingAckState(PENDING_ACK_NONE); - touch_queue_->UpdateTouchAckStates(timeout_event_.event, ack_result); + touch_queue_->UpdateTouchConsumerStates(timeout_event_.event, + ack_result); } return true; case PENDING_ACK_CANCEL_EVENT: @@ -109,19 +125,29 @@ class TouchEventQueue::TouchTimeoutHandler { return HasTimeoutEvent(); } - bool IsTimeoutTimerRunning() const { - return timeout_monitor_.IsRunning(); - } + void SetEnabled(bool enabled) { + if (enabled_ == enabled) + return; - void Reset() { - pending_ack_state_ = PENDING_ACK_NONE; - timeout_monitor_.Stop(); - } + enabled_ = enabled; - void set_timeout_delay(base::TimeDelta timeout_delay) { - timeout_delay_ = timeout_delay; + if (enabled_) + return; + + enabled_for_current_sequence_ = false; + // Only reset the |timeout_handler_| if the timer is running and has not + // yet timed out. This ensures that an already timed out sequence is + // properly flushed by the handler. + if (IsTimeoutTimerRunning()) { + pending_ack_state_ = PENDING_ACK_NONE; + timeout_monitor_.Stop(); + } } + bool IsTimeoutTimerRunning() const { return timeout_monitor_.IsRunning(); } + + bool enabled() const { return enabled_; } + private: enum PendingAckState { PENDING_ACK_NONE, @@ -184,6 +210,9 @@ class TouchEventQueue::TouchTimeoutHandler { // Provides timeout-based callback behavior. TimeoutMonitor timeout_monitor_; + + bool enabled_; + bool enabled_for_current_sequence_; }; // Provides touchmove slop suppression for a single touch that remains within @@ -344,8 +373,7 @@ TouchEventQueue::TouchEventQueue(TouchEventQueueClient* client, : client_(client), dispatching_touch_ack_(NULL), dispatching_touch_(false), - touch_filtering_state_(TOUCH_FILTERING_STATE_DEFAULT), - ack_timeout_enabled_(config.touch_ack_timeout_supported), + has_handlers_(true), touchmove_slop_suppressor_(new TouchMoveSlopSuppressor( config.touchmove_slop_suppression_length_dips)), send_touch_events_async_(false), @@ -353,7 +381,7 @@ TouchEventQueue::TouchEventQueue(TouchEventQueueClient* client, last_sent_touch_timestamp_sec_(0), touch_scrolling_mode_(config.touch_scrolling_mode) { DCHECK(client); - if (ack_timeout_enabled_) { + if (config.touch_ack_timeout_supported) { timeout_handler_.reset( new TouchTimeoutHandler(this, config.touch_ack_timeout_delay)); } @@ -413,11 +441,6 @@ void TouchEventQueue::ProcessTouchAck(InputEventAckState ack_result, if (touch_queue_.empty()) return; - if (ack_result == INPUT_EVENT_ACK_STATE_CONSUMED && - touch_filtering_state_ == FORWARD_TOUCHES_UNTIL_TIMEOUT) { - touch_filtering_state_ = FORWARD_ALL_TOUCHES; - } - PopTouchEventToClient(ack_result, latency_info); TryForwardNextEventToRenderer(); } @@ -448,20 +471,8 @@ void TouchEventQueue::ForwardNextEventToRenderer() { DCHECK(!empty()); DCHECK(!dispatching_touch_); - DCHECK_NE(touch_filtering_state_, DROP_ALL_TOUCHES); TouchEventWithLatencyInfo touch = touch_queue_.front()->coalesced_event(); - if (WebTouchEventTraits::IsTouchSequenceStart(touch.event)) { - touch_filtering_state_ = - ack_timeout_enabled_ ? FORWARD_TOUCHES_UNTIL_TIMEOUT - : FORWARD_ALL_TOUCHES; - touch_ack_states_.clear(); - send_touch_events_async_ = false; - pending_async_touchmove_.reset(); - touch_sequence_start_position_ = - gfx::PointF(touch.event.touches[0].position); - } - if (send_touch_events_async_ && touch.event.type == WebInputEvent::TouchMove) { // Throttling touchmove's in a continuous touchmove stream while scrolling @@ -527,36 +538,27 @@ void TouchEventQueue::ForwardNextEventToRenderer() { // the touch timeout should not be started. base::AutoReset<bool> dispatching_touch(&dispatching_touch_, true); SendTouchEventImmediately(touch); - if (dispatching_touch_ && - touch_filtering_state_ == FORWARD_TOUCHES_UNTIL_TIMEOUT && - ShouldTouchTriggerTimeout(touch.event)) { - DCHECK(timeout_handler_); - timeout_handler_->Start(touch); - } + if (dispatching_touch_ && timeout_handler_) + timeout_handler_->StartIfNecessary(touch); } void TouchEventQueue::OnGestureScrollEvent( const GestureEventWithLatencyInfo& gesture_event) { if (gesture_event.event.type == blink::WebInputEvent::GestureScrollBegin) { - if (touch_filtering_state_ != DROP_ALL_TOUCHES && - touch_filtering_state_ != DROP_TOUCHES_IN_SEQUENCE) { + if (!touch_consumer_states_.is_empty() && + !drop_remaining_touches_in_sequence_) { DCHECK(!touchmove_slop_suppressor_->suppressing_touchmoves()) - << "The renderer should be offered a touchmove before scrolling " - "begins"; + << "A touch handler should be offered a touchmove before scrolling."; } - if (touch_scrolling_mode_ == TOUCH_SCROLLING_MODE_ASYNC_TOUCHMOVE && - touch_filtering_state_ != DROP_ALL_TOUCHES && - touch_filtering_state_ != DROP_TOUCHES_IN_SEQUENCE && - (touch_ack_states_.empty() || - AllTouchAckStatesHaveState( - INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS))) { + !drop_remaining_touches_in_sequence_ && + touch_consumer_states_.is_empty()) { // If no touch points have a consumer, prevent all subsequent touch events // received during the scroll from reaching the renderer. This ensures // that the first touchstart the renderer sees in any given sequence can // always be preventDefault'ed (cancelable == true). // TODO(jdduke): Revisit if touchstarts during scroll are made cancelable. - touch_filtering_state_ = DROP_TOUCHES_IN_SEQUENCE; + drop_remaining_touches_in_sequence_ = true; } if (touch_scrolling_mode_ == TOUCH_SCROLLING_MODE_ASYNC_TOUCHMOVE) { @@ -583,10 +585,10 @@ void TouchEventQueue::OnGestureScrollEvent( if (!dispatching_touch_ack_) return; - if (touch_filtering_state_ == DROP_TOUCHES_IN_SEQUENCE) + if (drop_remaining_touches_in_sequence_) return; - touch_filtering_state_ = DROP_TOUCHES_IN_SEQUENCE; + drop_remaining_touches_in_sequence_ = true; // Fake a TouchCancel to cancel the touch points of the touch event // that is currently being acked. @@ -622,22 +624,7 @@ void TouchEventQueue::OnGestureEventAck( void TouchEventQueue::OnHasTouchEventHandlers(bool has_handlers) { DCHECK(!dispatching_touch_ack_); DCHECK(!dispatching_touch_); - - if (has_handlers) { - if (touch_filtering_state_ == DROP_ALL_TOUCHES) { - // If no touch handler was previously registered, ensure that we don't - // send a partial touch sequence to the renderer. - touch_filtering_state_ = DROP_TOUCHES_IN_SEQUENCE; - } - } else { - // TODO(jdduke): Synthesize a TouchCancel if necessary to update Blink touch - // state tracking and/or touch-action filtering (e.g., if the touch handler - // was removed mid-sequence), crbug.com/375940. - touch_filtering_state_ = DROP_ALL_TOUCHES; - pending_async_touchmove_.reset(); - if (timeout_handler_) - timeout_handler_->Reset(); - } + has_handlers_ = has_handlers; } bool TouchEventQueue::IsPendingAckTouchStart() const { @@ -651,25 +638,12 @@ bool TouchEventQueue::IsPendingAckTouchStart() const { } void TouchEventQueue::SetAckTimeoutEnabled(bool enabled) { - // The timeout handler is valid only if explicitly supported in the config. - if (!timeout_handler_) - return; - - if (ack_timeout_enabled_ == enabled) - return; - - ack_timeout_enabled_ = enabled; - - if (enabled) - return; + if (timeout_handler_) + timeout_handler_->SetEnabled(enabled); +} - if (touch_filtering_state_ == FORWARD_TOUCHES_UNTIL_TIMEOUT) - touch_filtering_state_ = FORWARD_ALL_TOUCHES; - // Only reset the |timeout_handler_| if the timer is running and has not yet - // timed out. This ensures that an already timed out sequence is properly - // flushed by the handler. - if (timeout_handler_ && timeout_handler_->IsTimeoutTimerRunning()) - timeout_handler_->Reset(); +bool TouchEventQueue::IsAckTimeoutEnabled() const { + return timeout_handler_ && timeout_handler_->enabled(); } bool TouchEventQueue::HasPendingAsyncTouchMoveForTesting() const { @@ -689,8 +663,7 @@ void TouchEventQueue::FlushQueue() { DCHECK(!dispatching_touch_ack_); DCHECK(!dispatching_touch_); pending_async_touchmove_.reset(); - if (touch_filtering_state_ != DROP_ALL_TOUCHES) - touch_filtering_state_ = DROP_TOUCHES_IN_SEQUENCE; + drop_remaining_touches_in_sequence_ = true; while (!touch_queue_.empty()) PopTouchEventToClient(INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS); } @@ -711,7 +684,7 @@ void TouchEventQueue::AckTouchEventToClient( const ui::LatencyInfo* optional_latency_info) { DCHECK(acked_event); DCHECK(!dispatching_touch_ack_); - UpdateTouchAckStates(acked_event->coalesced_event().event, ack_result); + UpdateTouchConsumerStates(acked_event->coalesced_event().event, ack_result); // Note that acking the touch-event may result in multiple gestures being sent // to the renderer, or touch-events being queued. @@ -750,19 +723,25 @@ TouchEventQueue::FilterBeforeForwarding(const WebTouchEvent& event) { if (touchmove_slop_suppressor_->FilterEvent(event)) return ACK_WITH_NOT_CONSUMED; - if (touch_filtering_state_ == DROP_ALL_TOUCHES) - return ACK_WITH_NO_CONSUMER_EXISTS; + if (WebTouchEventTraits::IsTouchSequenceStart(event)) { + touch_consumer_states_.clear(); + send_touch_events_async_ = false; + pending_async_touchmove_.reset(); + touch_sequence_start_position_ = gfx::PointF(event.touches[0].position); + drop_remaining_touches_in_sequence_ = false; + if (!has_handlers_) { + drop_remaining_touches_in_sequence_ = true; + return ACK_WITH_NO_CONSUMER_EXISTS; + } + } - if (touch_filtering_state_ == DROP_TOUCHES_IN_SEQUENCE && + if (drop_remaining_touches_in_sequence_ && event.type != WebInputEvent::TouchCancel) { - if (WebTouchEventTraits::IsTouchSequenceStart(event)) - return FORWARD_TO_RENDERER; return ACK_WITH_NO_CONSUMER_EXISTS; } - // Touch press events should always be forwarded to the renderer. if (event.type == WebInputEvent::TouchStart) - return FORWARD_TO_RENDERER; + return has_handlers_ ? FORWARD_TO_RENDERER : ACK_WITH_NO_CONSUMER_EXISTS; for (unsigned int i = 0; i < event.touchesLength; ++i) { const WebTouchPoint& point = event.touches[i]; @@ -770,22 +749,15 @@ TouchEventQueue::FilterBeforeForwarding(const WebTouchEvent& event) { if (point.state == WebTouchPoint::StateStationary) continue; - if (touch_ack_states_.count(point.id) > 0) { - if (touch_ack_states_.find(point.id)->second != - INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS) - return FORWARD_TO_RENDERER; - } else { - // If the ACK status of a point is unknown, then the event should be - // forwarded to the renderer. + if (touch_consumer_states_.has_bit(point.id)) return FORWARD_TO_RENDERER; - } } return ACK_WITH_NO_CONSUMER_EXISTS; } -void TouchEventQueue::UpdateTouchAckStates(const WebTouchEvent& event, - InputEventAckState ack_result) { +void TouchEventQueue::UpdateTouchConsumerStates(const WebTouchEvent& event, + InputEventAckState ack_result) { // Update the ACK status for each touch point in the ACKed event. if (event.type == WebInputEvent::TouchEnd || event.type == WebInputEvent::TouchCancel) { @@ -794,31 +766,19 @@ void TouchEventQueue::UpdateTouchAckStates(const WebTouchEvent& event, const WebTouchPoint& point = event.touches[i]; if (point.state == WebTouchPoint::StateReleased || point.state == WebTouchPoint::StateCancelled) - touch_ack_states_.erase(point.id); + touch_consumer_states_.clear_bit(point.id); } } else if (event.type == WebInputEvent::TouchStart) { for (unsigned i = 0; i < event.touchesLength; ++i) { const WebTouchPoint& point = event.touches[i]; - if (point.state == WebTouchPoint::StatePressed) - touch_ack_states_[point.id] = ack_result; + if (point.state == WebTouchPoint::StatePressed) { + if (ack_result != INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS) + touch_consumer_states_.mark_bit(point.id); + else + touch_consumer_states_.clear_bit(point.id); + } } } } -bool TouchEventQueue::AllTouchAckStatesHaveState( - InputEventAckState ack_state) const { - if (touch_ack_states_.empty()) - return false; - - for (TouchPointAckStates::const_iterator iter = touch_ack_states_.begin(), - end = touch_ack_states_.end(); - iter != end; - ++iter) { - if (iter->second != ack_state) - return false; - } - - return true; -} - } // namespace content diff --git a/content/browser/renderer_host/input/touch_event_queue.h b/content/browser/renderer_host/input/touch_event_queue.h index a45e60f..3bdab90 100644 --- a/content/browser/renderer_host/input/touch_event_queue.h +++ b/content/browser/renderer_host/input/touch_event_queue.h @@ -14,6 +14,7 @@ #include "content/common/content_export.h" #include "content/common/input/input_event_ack_state.h" #include "third_party/WebKit/public/web/WebInputEvent.h" +#include "ui/events/gesture_detection/bitset_32.h" #include "ui/gfx/geometry/point_f.h" namespace content { @@ -114,6 +115,10 @@ class CONTENT_EXPORT TouchEventQueue { // it will take effect only for the following touch sequence. void SetAckTimeoutEnabled(bool enabled); + bool IsAckTimeoutEnabled() const; + + bool IsForwardingTouches(); + bool empty() const WARN_UNUSED_RESULT { return touch_queue_.empty(); } @@ -122,13 +127,7 @@ class CONTENT_EXPORT TouchEventQueue { return touch_queue_.size(); } - bool ack_timeout_enabled() const { - return ack_timeout_enabled_; - } - - bool has_handlers() const { - return touch_filtering_state_ != DROP_ALL_TOUCHES; - } + bool has_handlers() const { return has_handlers_; } private: class TouchTimeoutHandler; @@ -181,10 +180,8 @@ class CONTENT_EXPORT TouchEventQueue { // has no touch handler. PreFilterResult FilterBeforeForwarding(const blink::WebTouchEvent& event); void ForwardToRenderer(const TouchEventWithLatencyInfo& event); - void UpdateTouchAckStates(const blink::WebTouchEvent& event, - InputEventAckState ack_result); - bool AllTouchAckStatesHaveState(InputEventAckState ack_state) const; - + void UpdateTouchConsumerStates(const blink::WebTouchEvent& event, + InputEventAckState ack_result); // Handles touch event forwarding and ack'ed event dispatch. TouchEventQueueClient* client_; @@ -192,9 +189,11 @@ class CONTENT_EXPORT TouchEventQueue { typedef std::deque<CoalescedWebTouchEvent*> TouchQueue; TouchQueue touch_queue_; - // Maintain the ACK status for each touch point. - typedef std::map<int, InputEventAckState> TouchPointAckStates; - TouchPointAckStates touch_ack_states_; + // Maps whether each active pointer has a consumer (i.e., a touch point has a + // valid consumer iff |touch_consumer_states[pointer.id]| is true.). + // TODO(jdduke): Consider simply tracking whether *any* touchstart had a + // consumer, crbug.com/416497. + ui::BitSet32 touch_consumer_states_; // Position of the first touch in the most recent sequence forwarded to the // client. @@ -209,18 +208,18 @@ class CONTENT_EXPORT TouchEventQueue { // ack after forwarding a touch event to the client. bool dispatching_touch_; - enum TouchFilteringState { - FORWARD_ALL_TOUCHES, // Don't filter at all - the default. - FORWARD_TOUCHES_UNTIL_TIMEOUT, // Don't filter unless we get an ACK timeout. - DROP_TOUCHES_IN_SEQUENCE, // Filter all events until a new touch - // sequence is received. - DROP_ALL_TOUCHES, // Filter all events, e.g., no touch handler. - TOUCH_FILTERING_STATE_DEFAULT = FORWARD_ALL_TOUCHES, - }; - TouchFilteringState touch_filtering_state_; + // Whether the renderer has at least one touch handler. + bool has_handlers_; + + // Whether to allow any remaining touches for the current sequence. Note that + // this is a stricter condition than an empty |touch_consumer_states_|, as it + // also prevents forwarding of touchstart events for new pointers in the + // current sequence. This is only used when the event is synthetically + // cancelled after a touch timeout, or after a scroll event when the + // mode is TOUCH_SCROLLING_MODE_TOUCHCANCEL. + bool drop_remaining_touches_in_sequence_; // Optional handler for timed-out touch event acks. - bool ack_timeout_enabled_; scoped_ptr<TouchTimeoutHandler> timeout_handler_; // Suppression of TouchMove's within a slop region when a sequence has not yet diff --git a/content/browser/renderer_host/input/touch_event_queue_unittest.cc b/content/browser/renderer_host/input/touch_event_queue_unittest.cc index ed4f869..90e922b 100644 --- a/content/browser/renderer_host/input/touch_event_queue_unittest.cc +++ b/content/browser/renderer_host/input/touch_event_queue_unittest.cc @@ -196,8 +196,6 @@ class TouchEventQueueTest : public testing::Test, void SetAckTimeoutDisabled() { queue_->SetAckTimeoutEnabled(false); } - bool IsTimeoutEnabled() const { return queue_->ack_timeout_enabled(); } - bool IsTimeoutRunning() const { return queue_->IsTimeoutRunningForTesting(); } bool HasPendingAsyncTouchMove() const { @@ -285,9 +283,55 @@ TEST_F(TouchEventQueueTest, Basic) { EXPECT_TRUE(acked_event().cancelable); } -// 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) { +// Tests that touch-events with multiple points are queued properly. +TEST_F(TouchEventQueueTest, BasicMultiTouch) { + const size_t kPointerCount = 10; + for (size_t i = 0; i < kPointerCount; ++i) + PressTouchPoint(i, i); + + EXPECT_EQ(1U, GetAndResetSentEventCount()); + EXPECT_EQ(0U, GetAndResetAckedEventCount()); + EXPECT_EQ(kPointerCount, queued_event_count()); + + for (size_t i = 0; i < kPointerCount; ++i) + MoveTouchPoint(i, 1.f + i, 2.f + i); + + EXPECT_EQ(0U, GetAndResetSentEventCount()); + EXPECT_EQ(0U, GetAndResetAckedEventCount()); + // All moves should coalesce. + EXPECT_EQ(kPointerCount + 1, queued_event_count()); + + for (size_t i = 0; i < kPointerCount; ++i) + ReleaseTouchPoint(kPointerCount - 1 - i); + + EXPECT_EQ(0U, GetAndResetSentEventCount()); + EXPECT_EQ(0U, GetAndResetAckedEventCount()); + EXPECT_EQ(kPointerCount * 2 + 1, queued_event_count()); + + // Ack all presses. + for (size_t i = 0; i < kPointerCount; ++i) + SendTouchEventAck(INPUT_EVENT_ACK_STATE_CONSUMED); + + EXPECT_EQ(kPointerCount, GetAndResetAckedEventCount()); + EXPECT_EQ(kPointerCount, GetAndResetSentEventCount()); + + // Ack the coalesced move. + SendTouchEventAck(INPUT_EVENT_ACK_STATE_CONSUMED); + EXPECT_EQ(kPointerCount, GetAndResetAckedEventCount()); + EXPECT_EQ(1U, GetAndResetSentEventCount()); + + // Ack all releases. + for (size_t i = 0; i < kPointerCount; ++i) + SendTouchEventAck(INPUT_EVENT_ACK_STATE_CONSUMED); + + EXPECT_EQ(kPointerCount, GetAndResetAckedEventCount()); + EXPECT_EQ(kPointerCount - 1, GetAndResetSentEventCount()); +} + +// Tests that the touch-queue continues delivering events for an active pointer +// after all handlers are removed, but acks new pointers immediately as having +// no consumer. +TEST_F(TouchEventQueueTest, NoNewTouchesForwardedAfterHandlersRemoved) { OnHasTouchEventHandlers(true); EXPECT_EQ(0U, queued_event_count()); EXPECT_EQ(0U, GetAndResetSentEventCount()); @@ -309,37 +353,37 @@ TEST_F(TouchEventQueueTest, QueueFlushedOnAckAfterHandlersRemoved) { EXPECT_EQ(0U, queued_event_count()); EXPECT_EQ(INPUT_EVENT_ACK_STATE_CONSUMED, acked_event_state()); - // The release should not be forwarded. - ReleaseTouchPoint(0); + // Try forwarding a new pointer. It should be rejected immediately. + PressTouchPoint(2, 2); EXPECT_EQ(1U, GetAndResetAckedEventCount()); EXPECT_EQ(0U, queued_event_count()); EXPECT_EQ(INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS, acked_event_state()); - OnHasTouchEventHandlers(true); + // Further events for the pointer without a handler should not be forwarded. + MoveTouchPoint(1, 3, 3); + ReleaseTouchPoint(1); + EXPECT_EQ(2U, GetAndResetAckedEventCount()); + EXPECT_EQ(0U, queued_event_count()); + EXPECT_EQ(INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS, acked_event_state()); - // Events will be queued until the first sent event is ack'ed. - for (int i = 0; i < 10; ++i) { - PressTouchPoint(1, 1); - MoveTouchPoint(0, i, i); - ReleaseTouchPoint(0); - } - EXPECT_EQ(30U, queued_event_count()); + // Events for the first pointer, that had a handler, should be forwarded, even + // if the renderer reports that no handlers exist. + MoveTouchPoint(0, 4, 4); + ReleaseTouchPoint(0); EXPECT_EQ(1U, GetAndResetSentEventCount()); + EXPECT_EQ(2U, queued_event_count()); - // 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()); + SendTouchEventAck(INPUT_EVENT_ACK_STATE_CONSUMED); + EXPECT_EQ(1U, GetAndResetAckedEventCount()); + EXPECT_EQ(1U, GetAndResetSentEventCount()); + EXPECT_EQ(1U, queued_event_count()); + EXPECT_EQ(INPUT_EVENT_ACK_STATE_CONSUMED, acked_event_state()); - // 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(1U, GetAndResetAckedEventCount()); EXPECT_EQ(0U, GetAndResetSentEventCount()); - EXPECT_EQ(30U, GetAndResetAckedEventCount()); - EXPECT_EQ(INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS, acked_event_state()); + EXPECT_EQ(0U, queued_event_count()); + EXPECT_EQ(INPUT_EVENT_ACK_STATE_CONSUMED, acked_event_state()); } // Tests that addition of a touch handler during a touch sequence will not cause @@ -423,6 +467,34 @@ TEST_F(TouchEventQueueTest, ActiveSequenceDroppedWhenHandlersRemoved) { EXPECT_EQ(1U, GetAndResetSentEventCount()); } +// Tests that removal/addition of a touch handler without any intervening +// touch activity has no affect on touch forwarding. +TEST_F(TouchEventQueueTest, + ActiveSequenceUnaffectedByRepeatedHandlerRemovalAndAddition) { + // Send a touch-press event. + PressTouchPoint(1, 1); + EXPECT_EQ(1U, GetAndResetSentEventCount()); + EXPECT_EQ(1U, queued_event_count()); + + // Simulate the case where the touchstart handler removes itself, and adds a + // touchmove handler. + OnHasTouchEventHandlers(false); + OnHasTouchEventHandlers(true); + + // Queue a touch-move event. + MoveTouchPoint(0, 5, 5); + EXPECT_EQ(2U, queued_event_count()); + EXPECT_EQ(0U, GetAndResetAckedEventCount()); + EXPECT_EQ(0U, GetAndResetSentEventCount()); + + // The ack should trigger forwarding of the touchmove, as if no touch + // handler registration changes have occurred. + SendTouchEventAck(INPUT_EVENT_ACK_STATE_NOT_CONSUMED); + EXPECT_EQ(1U, GetAndResetAckedEventCount()); + EXPECT_EQ(1U, GetAndResetSentEventCount()); + EXPECT_EQ(1U, queued_event_count()); +} + // Tests that touch-events are coalesced properly in the queue. TEST_F(TouchEventQueueTest, Coalesce) { // Send a touch-press event. @@ -733,10 +805,10 @@ TEST_F(TouchEventQueueTest, AckWithFollowupEvents) { // Create a touch event that will be queued synchronously by a touch ack. // Note, this will be triggered by all subsequent touch acks. WebTouchEvent followup_event; - followup_event.type = WebInputEvent::TouchStart; + followup_event.type = WebInputEvent::TouchMove; followup_event.touchesLength = 1; - followup_event.touches[0].id = 1; - followup_event.touches[0].state = WebTouchPoint::StatePressed; + followup_event.touches[0].id = 0; + followup_event.touches[0].state = WebTouchPoint::StateMoved; SetFollowupEvent(followup_event); // Receive an ACK for the press. This should cause the followup touch-move to @@ -1175,14 +1247,12 @@ TEST_F(TouchEventQueueTest, NoTouchTimeoutIfDisabledAfterTouchStart) { // Send the ack immediately. The timeout should not have fired. SendTouchEventAck(INPUT_EVENT_ACK_STATE_NOT_CONSUMED); EXPECT_FALSE(IsTimeoutRunning()); - EXPECT_TRUE(IsTimeoutEnabled()); EXPECT_EQ(1U, GetAndResetSentEventCount()); EXPECT_EQ(1U, GetAndResetAckedEventCount()); // Now explicitly disable the timeout. SetAckTimeoutDisabled(); EXPECT_FALSE(IsTimeoutRunning()); - EXPECT_FALSE(IsTimeoutEnabled()); // A TouchMove should not start or trigger the timeout. MoveTouchPoint(0, 5, 5); @@ -1203,19 +1273,6 @@ TEST_F(TouchEventQueueTest, NoTouchTimeoutIfAckIsSynchronous) { EXPECT_FALSE(IsTimeoutRunning()); } -// Tests that the timeout is disabled if the touch handler disappears. -TEST_F(TouchEventQueueTest, NoTouchTimeoutIfTouchHandlerRemoved) { - SetUpForTimeoutTesting(DefaultTouchTimeoutDelay()); - - // Queue a TouchStart. - PressTouchPoint(0, 1); - ASSERT_TRUE(IsTimeoutRunning()); - - // Unload the touch handler. - OnHasTouchEventHandlers(false); - EXPECT_FALSE(IsTimeoutRunning()); -} - // Tests that the timeout does not fire if explicitly disabled while an event // is in-flight. TEST_F(TouchEventQueueTest, NoTouchTimeoutIfDisabledWhileTimerIsActive) { @@ -2131,4 +2188,42 @@ TEST_F(TouchEventQueueTest, TouchAbsorptionWithConsumedFirstMove) { EXPECT_EQ(0U, GetAndResetSentEventCount()); } +TEST_F(TouchEventQueueTest, UnseenTouchPointerIdsNotForwarded) { + SyntheticWebTouchEvent event; + event.PressPoint(0, 0); + SendTouchEvent(event); + EXPECT_EQ(1U, GetAndResetSentEventCount()); + SendTouchEventAck(INPUT_EVENT_ACK_STATE_CONSUMED); + EXPECT_EQ(1U, GetAndResetAckedEventCount()); + + // Give the touchmove a previously unseen pointer id; it should not be sent. + int press_id = event.touches[0].id; + event.MovePoint(0, 1, 1); + event.touches[0].id = 7; + SendTouchEvent(event); + EXPECT_EQ(0U, GetAndResetSentEventCount()); + EXPECT_EQ(1U, GetAndResetAckedEventCount()); + + // Give the touchmove a valid id; it should be sent. + event.touches[0].id = press_id; + SendTouchEvent(event); + EXPECT_EQ(1U, GetAndResetSentEventCount()); + SendTouchEventAck(INPUT_EVENT_ACK_STATE_CONSUMED); + EXPECT_EQ(1U, GetAndResetAckedEventCount()); + + // Do the same for release. + event.ReleasePoint(0); + event.touches[0].id = 11; + SendTouchEvent(event); + EXPECT_EQ(0U, GetAndResetSentEventCount()); + EXPECT_EQ(1U, GetAndResetAckedEventCount()); + + // Give the touchmove a valid id; it should be sent. + event.touches[0].id = press_id; + SendTouchEvent(event); + EXPECT_EQ(1U, GetAndResetSentEventCount()); + SendTouchEventAck(INPUT_EVENT_ACK_STATE_CONSUMED); + EXPECT_EQ(1U, GetAndResetAckedEventCount()); +} + } // namespace content diff --git a/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc b/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc index ac98178..63bdc87 100644 --- a/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc +++ b/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc @@ -919,7 +919,7 @@ TEST_F(RenderWidgetHostViewAuraTest, TouchEventState) { // 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; + ack.state = INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS; widget_host_->OnMessageReceived(InputHostMsg_HandleInputEvent_ACK(0, ack)); EXPECT_FALSE(widget_host_->ShouldForwardTouchEvent()); |