Delegate touch handler registration logic to the TouchEventQueue

Teach the TouchEventQueue about touch handler registration, allowing it to make
more informed touch forwarding decisions.  This also prevents corner cases where
a partial touch sequence would be improperly forwarded to the renderer after a
touch handler is registered.

BUG=332418

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@244072 0039d316-1c4b-4281-b951-d872f2087c98

content/browser/renderer_host/input/input_router_impl.cc

@@ -100,7 +100,6 @@ InputRouterImpl::InputRouterImpl(IPC::Sender* sender,
       move_caret_pending_(false),
       mouse_move_pending_(false),
       mouse_wheel_pending_(false),
-      has_touch_handler_(false),
       touch_ack_timeout_enabled_(false),
       touch_ack_timeout_delay_ms_(std::numeric_limits<size_t>::max()),
       current_ack_source_(ACK_SOURCE_NONE),
@@ -259,11 +258,8 @@ const NativeWebKeyboardEvent* InputRouterImpl::GetLastKeyboardEvent() const {
 }
 
 bool InputRouterImpl::ShouldForwardTouchEvent() const {
-  // Always send a touch event if the renderer has a touch-event handler. It is
-  // possible that a renderer stops listening to touch-events while there are
-  // still events in the touch-queue. In such cases, the new events should still
-  // get into the queue.
-  return has_touch_handler_ || !touch_event_queue_->empty();
+  // Always send a touch event if the renderer has a touch-event handler.
+  return touch_event_queue_->has_handlers();
 }
 
 void InputRouterImpl::OnViewUpdated(int view_flags) {
@@ -502,11 +498,9 @@ void InputRouterImpl::OnSelectRangeAck() {
 }
 
 void InputRouterImpl::OnHasTouchEventHandlers(bool has_handlers) {
- if (has_touch_handler_ == has_handlers)
+  if (has_handlers == touch_event_queue_->has_handlers())
     return;
-  has_touch_handler_ = has_handlers;
-  if (!has_handlers)
-    touch_event_queue_->FlushQueue();
+  touch_event_queue_->OnHasTouchEventHandlers(has_handlers);
   client_->OnHasTouchEventHandlers(has_handlers);
 }
 

content/browser/renderer_host/input/input_router_impl.h

@@ -230,11 +230,6 @@ private:
   // back to whatever unhandled handler instead of the returned version.
   KeyQueue key_queue_;
 
-  // Keeps track of whether the webpage has any touch event handler. If it does,
-  // then touch events are sent to the renderer. Otherwise, the touch events are
-  // not sent to the renderer.
-  bool has_touch_handler_;
-
   // Whether touch ack timeout handling has been enabled via the command line.
   bool touch_ack_timeout_enabled_;
   size_t touch_ack_timeout_delay_ms_;

content/browser/renderer_host/input/input_router_impl_unittest.cc

@@ -269,6 +269,11 @@ class InputRouterImplTest : public testing::Test {
     return input_router()->touch_event_queue_->ack_timeout_enabled();
   }
 
+  void OnHasTouchEventHandlers(bool has_handlers) {
+    input_router_->OnMessageReceived(
+        ViewHostMsg_HasTouchEventHandlers(0, has_handlers));
+  }
+
   size_t GetSentMessageCountAndResetSink() {
     size_t count = process_->sink().message_count();
     process_->sink().ClearMessages();
@@ -529,6 +534,8 @@ TEST_F(InputRouterImplTest,
 
 // Tests that touch-events are queued properly.
 TEST_F(InputRouterImplTest, TouchEventQueue) {
+  OnHasTouchEventHandlers(true);
+
   PressTouchPoint(1, 1);
   SendTouchEvent();
   EXPECT_TRUE(client_->GetAndResetFilterEventCalled());
@@ -563,7 +570,7 @@ TEST_F(InputRouterImplTest, TouchEventQueue) {
 // Tests that the touch-queue is emptied if a page stops listening for touch
 // events.
 TEST_F(InputRouterImplTest, TouchEventQueueFlush) {
-  input_router_->OnMessageReceived(ViewHostMsg_HasTouchEventHandlers(0, true));
+  OnHasTouchEventHandlers(true);
   EXPECT_TRUE(client_->has_touch_handler());
   EXPECT_EQ(0U, GetSentMessageCountAndResetSink());
   EXPECT_TRUE(TouchEventQueueEmpty());
@@ -579,7 +586,7 @@ TEST_F(InputRouterImplTest, TouchEventQueueFlush) {
   // 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.
-  input_router_->OnMessageReceived(ViewHostMsg_HasTouchEventHandlers(0, false));
+  OnHasTouchEventHandlers(false);
   EXPECT_FALSE(client_->has_touch_handler());
   EXPECT_EQ(0U, GetSentMessageCountAndResetSink());
   EXPECT_TRUE(TouchEventQueueEmpty());
@@ -702,6 +709,8 @@ TEST_F(InputRouterImplTest, UnhandledWheelEvent) {
 }
 
 TEST_F(InputRouterImplTest, TouchTypesIgnoringAck) {
+  OnHasTouchEventHandlers(true);
+
   int start_type = static_cast<int>(WebInputEvent::TouchStart);
   int end_type = static_cast<int>(WebInputEvent::TouchCancel);
   ASSERT_LT(start_type, end_type);

content/browser/renderer_host/input/touch_event_queue.cc

@@ -103,6 +103,11 @@ class TouchEventQueue::TouchTimeoutHandler {
     return timeout_monitor_.IsRunning();
   }
 
+  void Reset() {
+    pending_ack_state_ = PENDING_ACK_NONE;
+    timeout_monitor_.Stop();
+  }
+
  private:
   enum PendingAckState {
     PENDING_ACK_NONE,
@@ -236,7 +241,8 @@ TouchEventQueue::TouchEventQueue(TouchEventQueueClient* client)
     : client_(client),
       dispatching_touch_ack_(NULL),
       dispatching_touch_(false),
-      no_touch_to_renderer_(false),
+      has_handlers_(false),
+      scroll_in_progress_(false),
       renderer_is_consuming_touch_gesture_(false),
       ack_timeout_enabled_(false) {
   DCHECK(client);
@@ -248,6 +254,14 @@ TouchEventQueue::~TouchEventQueue() {
 }
 
 void TouchEventQueue::QueueEvent(const TouchEventWithLatencyInfo& event) {
+  // Optimization of the case without touch handlers.  Removing this path
+  // yields identical results, but this avoids unnecessary allocations.
+  if (!has_handlers_) {
+    DCHECK(touch_queue_.empty());
+    client_->OnTouchEventAck(event, kDefaultNotForwardedAck);
+    return;
+  }
+
   // If the queueing of |event| was triggered by an ack dispatch, defer
   // processing the event until the dispatch has finished.
   if (touch_queue_.empty() && !dispatching_touch_ack_) {
@@ -296,8 +310,10 @@ void TouchEventQueue::TryForwardNextEventToRenderer() {
   while (!touch_queue_.empty()) {
     const TouchEventWithLatencyInfo& touch =
         touch_queue_.front()->coalesced_event();
-    if (IsNewTouchGesture(touch.event))
+    if (IsNewTouchGesture(touch.event)) {
+      touch_ack_states_.clear();
       renderer_is_consuming_touch_gesture_ = false;
+    }
     if (ShouldForwardToRenderer(touch.event)) {
       ForwardToRenderer(touch);
       break;
@@ -330,9 +346,9 @@ void TouchEventQueue::OnGestureScrollEvent(
     // dispatching a touch event ack, so that we can fake a cancel
     // event that has the correct touch ids as the touch event that
     // is being acked. If not, we don't do the touch-cancel optimization.
-    if (no_touch_to_renderer_ || !dispatching_touch_ack_)
+    if (scroll_in_progress_ || !dispatching_touch_ack_)
       return;
-    no_touch_to_renderer_ = true;
+    scroll_in_progress_ = true;
 
     // If we have a timeout event, a cancel has already been dispatched
     // for the current touch stream.
@@ -350,15 +366,34 @@ void TouchEventQueue::OnGestureScrollEvent(
             dispatching_touch_ack_->coalesced_event()), true));
   } else if (type == blink::WebInputEvent::GestureScrollEnd ||
              type == blink::WebInputEvent::GestureFlingStart) {
-    no_touch_to_renderer_ = false;
+    scroll_in_progress_ = false;
   }
 }
 
-void TouchEventQueue::FlushQueue() {
+void TouchEventQueue::OnHasTouchEventHandlers(bool has_handlers) {
   DCHECK(!dispatching_touch_ack_);
   DCHECK(!dispatching_touch_);
-  while (!touch_queue_.empty())
-    PopTouchEventToClient(kDefaultNotForwardedAck, ui::LatencyInfo());
+  if (has_handlers_ == has_handlers)
+    return;
+
+  has_handlers_ = has_handlers;
+
+  if (!has_handlers_) {
+    // TODO(jdduke): Synthesize a TouchCancel if necessary to update Blink touch
+    // state tracking.
+    if (timeout_handler_)
+      timeout_handler_->Reset();
+    if (!touch_queue_.empty())
+      ProcessTouchAck(kDefaultNotForwardedAck, ui::LatencyInfo());
+    // As there is no touch handler, ack'ing the event should flush the queue.
+    DCHECK(touch_queue_.empty());
+  } else {
+    DCHECK(touch_queue_.empty());
+    // Prevent a partial sequence from being sent to the renderer.
+    TouchPointAckStates::iterator ack_it = touch_ack_states_.begin();
+    for (; ack_it != touch_ack_states_.end(); ++ack_it)
+      ack_it->second = kDefaultNotForwardedAck;
+  }
 }
 
 bool TouchEventQueue::IsPendingAckTouchStart() const {
@@ -398,6 +433,13 @@ TouchEventQueue::GetLatestEventForTesting() const {
   return touch_queue_.back()->coalesced_event();
 }
 
+void TouchEventQueue::FlushQueue() {
+  DCHECK(!dispatching_touch_ack_);
+  DCHECK(!dispatching_touch_);
+  while (!touch_queue_.empty())
+    PopTouchEventToClient(kDefaultNotForwardedAck, ui::LatencyInfo());
+}
+
 void TouchEventQueue::PopTouchEventToClient(
     InputEventAckState ack_result,
     const ui::LatencyInfo& renderer_latency_info) {
@@ -428,7 +470,10 @@ bool TouchEventQueue::ShouldForwardToRenderer(
   if (HasTimeoutEvent())
     return false;
 
-  if (no_touch_to_renderer_ &&
+  if (!has_handlers_)
+    return false;
+
+  if (scroll_in_progress_ &&
       event.type != blink::WebInputEvent::TouchCancel)
     return false;
 

content/browser/renderer_host/input/touch_event_queue.h

@@ -58,9 +58,8 @@ class CONTENT_EXPORT TouchEventQueue {
   // resume the normal flow of sending touch events to the renderer.
   void OnGestureScrollEvent(const GestureEventWithLatencyInfo& gesture_event);
 
-  // Empties the queue of touch events. This may result in any number of gesture
-  // events being sent to the renderer.
-  void FlushQueue();
+  // Notifies the queue whether the renderer has at least one touch handler.
+  void OnHasTouchEventHandlers(bool has_handlers);
 
   // Returns whether the currently pending touch event (waiting ACK) is for
   // a touch start event.
@@ -82,6 +81,10 @@ class CONTENT_EXPORT TouchEventQueue {
     return ack_timeout_enabled_;
   }
 
+  bool has_handlers() const {
+    return has_handlers_;
+  }
+
  private:
   class TouchTimeoutHandler;
   friend class TouchTimeoutHandler;
@@ -91,6 +94,10 @@ class CONTENT_EXPORT TouchEventQueue {
   bool IsTimeoutRunningForTesting() const;
   const TouchEventWithLatencyInfo& GetLatestEventForTesting() const;
 
+  // Empties the queue of touch events. This may result in any number of gesture
+  // events being sent to the renderer.
+  void FlushQueue();
+
   // Walks the queue, checking each event for |ShouldForwardToRenderer()|.
   // If true, forwards the touch event and stops processing further events.
   // If false, acks the event with |INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS|.
@@ -126,8 +133,11 @@ class CONTENT_EXPORT TouchEventQueue {
   // ack after forwarding a touch event to the client.
   bool dispatching_touch_;
 
+  // Whether there are any registered touch handlers. Defaults to false.
+  bool has_handlers_;
+
   // Don't send touch events to the renderer while scrolling.
-  bool no_touch_to_renderer_;
+  bool scroll_in_progress_;
 
   // Whether an event in the current (multi)touch sequence was consumed by the
   // renderer.  The touch timeout will never be activated when this is true.

content/browser/renderer_host/input/touch_event_queue_unittest.cc

@@ -35,6 +35,7 @@ class TouchEventQueueTest : public testing::Test,
   // testing::Test
   virtual void SetUp() OVERRIDE {
     queue_.reset(new TouchEventQueue(this));
+    queue_->OnHasTouchEventHandlers(true);
   }
 
   virtual void TearDown() OVERRIDE {
@@ -145,16 +146,14 @@ class TouchEventQueueTest : public testing::Test,
     return queue_->IsPendingAckTouchStart();
   }
 
-  void Flush() {
-    queue_->FlushQueue();
-  }
-
-  void SetEnableTouchForwarding(bool enabled) {
-    queue_->no_touch_to_renderer_ = !enabled;
+  void OnHasTouchEventHandlers(bool has_handlers) {
+    queue_->OnHasTouchEventHandlers(has_handlers);
   }
 
   bool WillForwardTouchEvents() {
-    return !queue_->no_touch_to_renderer_ && !queue_->HasTimeoutEvent();
+    return queue_->has_handlers_ &&
+           !queue_->scroll_in_progress_ &&
+           !queue_->HasTimeoutEvent();
   }
 
   bool IsTimeoutRunning() {
@@ -181,14 +180,6 @@ class TouchEventQueueTest : public testing::Test,
     return last_acked_event_state_;
   }
 
-  void set_no_touch_to_renderer(bool no_touch) {
-    queue_->no_touch_to_renderer_ = no_touch;
-  }
-
-  bool no_touch_to_renderer() const {
-    return queue_->no_touch_to_renderer_;
-  }
-
  private:
   void SendTouchEvent() {
     SendTouchEvent(touch_event_);
@@ -237,8 +228,8 @@ TEST_F(TouchEventQueueTest, Basic) {
 
 // Tests that the touch-queue is emptied if a page stops listening for touch
 // events.
-TEST_F(TouchEventQueueTest, Flush) {
-  Flush();
+TEST_F(TouchEventQueueTest, QueueFlushedWhenHandlersRemoved) {
+  OnHasTouchEventHandlers(true);
   EXPECT_EQ(0U, queued_event_count());
   EXPECT_EQ(0U, GetAndResetSentEventCount());
 
@@ -267,12 +258,58 @@ TEST_F(TouchEventQueueTest, Flush) {
 
   // Flush the queue. The touch-event queue should now be emptied, but none of
   // the queued touch-events should be sent to the renderer.
-  Flush();
+  OnHasTouchEventHandlers(false);
   EXPECT_EQ(0U, queued_event_count());
   EXPECT_EQ(0U, GetAndResetSentEventCount());
   EXPECT_EQ(31U, GetAndResetAckedEventCount());
 }
 
+// Tests that removal of a touch handler during a touch sequence will prevent
+// the remaining sequence from being forwarded, even if another touch handler is
+// registered during the same touch sequence.
+TEST_F(TouchEventQueueTest, ActiveSequenceDroppedWhenHandlersRemoved) {
+  // Send a touch-press event.
+  PressTouchPoint(1, 1);
+  EXPECT_EQ(1U, GetAndResetSentEventCount());
+  EXPECT_EQ(1U, queued_event_count());
+
+  // Queue a touch-move event.
+  MoveTouchPoint(0, 5, 5);
+  EXPECT_EQ(2U, queued_event_count());
+  EXPECT_EQ(0U, GetAndResetAckedEventCount());
+  EXPECT_EQ(0U, GetAndResetSentEventCount());
+
+  // Touch handle deregistration should flush the queue.
+  OnHasTouchEventHandlers(false);
+  EXPECT_EQ(2U, GetAndResetAckedEventCount());
+  EXPECT_EQ(0U, queued_event_count());
+
+  // The ack should be ignored as the touch queue is now empty.
+  SendTouchEventACK(INPUT_EVENT_ACK_STATE_NO_CONSUMER_EXISTS);
+  EXPECT_EQ(0U, GetAndResetAckedEventCount());
+  EXPECT_EQ(0U, queued_event_count());
+
+  // Events should be dropped while there is no touch handler.
+  MoveTouchPoint(0, 10, 10);
+  EXPECT_EQ(0U, queued_event_count());
+  EXPECT_EQ(1U, GetAndResetAckedEventCount());
+  EXPECT_EQ(0U, GetAndResetSentEventCount());
+
+  // Simulate touch handler registration in the middle of a touch sequence.
+  OnHasTouchEventHandlers(true);
+
+  // The touch end for the interrupted sequence should be dropped.
+  ReleaseTouchPoint(0);
+  EXPECT_EQ(0U, queued_event_count());
+  EXPECT_EQ(1U, GetAndResetAckedEventCount());
+  EXPECT_EQ(0U, GetAndResetSentEventCount());
+
+  // A new touch sequence should be forwarded properly.
+  PressTouchPoint(1, 1);
+  EXPECT_EQ(1U, queued_event_count());
+  EXPECT_EQ(1U, GetAndResetSentEventCount());
+}
+
 // Tests that touch-events are coalesced properly in the queue.
 TEST_F(TouchEventQueueTest, Coalesce) {
   // Send a touch-press event.
@@ -393,7 +430,7 @@ TEST_F(TouchEventQueueTest, AckAfterQueueFlushed) {
   EXPECT_EQ(1U, GetAndResetSentEventCount());
   EXPECT_EQ(1U, queued_event_count());
 
-  Flush();
+  OnHasTouchEventHandlers(false);
   EXPECT_EQ(0U, GetAndResetSentEventCount());
   EXPECT_EQ(0U, queued_event_count());
 
@@ -685,7 +722,7 @@ TEST_F(TouchEventQueueTest, ImmediateAckWithFollowupEvents) {
 // Tests basic TouchEvent forwarding suppression.
 TEST_F(TouchEventQueueTest, NoTouchBasic) {
   // Disable TouchEvent forwarding.
-  SetEnableTouchForwarding(false);
+  OnHasTouchEventHandlers(false);
   MoveTouchPoint(0, 30, 5);
   EXPECT_EQ(0U, GetAndResetSentEventCount());
   EXPECT_EQ(1U, GetAndResetAckedEventCount());
@@ -706,7 +743,7 @@ TEST_F(TouchEventQueueTest, NoTouchBasic) {
   EXPECT_EQ(1U, GetAndResetAckedEventCount());
 
   // Enable TouchEvent forwarding.
-  SetEnableTouchForwarding(true);
+  OnHasTouchEventHandlers(true);
 
   PressTouchPoint(80, 10);
   EXPECT_EQ(1U, GetAndResetSentEventCount());
@@ -977,6 +1014,19 @@ TEST_F(TouchEventQueueTest, NoTouchTimeoutIfAckIsSynchronous) {
   EXPECT_FALSE(IsTimeoutRunning());
 }
 
+// Tests that the timeout is disabled if the touch handler disappears.
+TEST_F(TouchEventQueueTest, TouchTimeoutStoppedIfTouchHandlerRemoved) {
+  SetUpForTimeoutTesting(kDefaultTouchTimeoutDelayMs);
+
+  // Queue a TouchStart.
+  PressTouchPoint(0, 1);
+  ASSERT_TRUE(IsTimeoutRunning());
+
+  // Unload the touch handler.
+  OnHasTouchEventHandlers(false);
+  EXPECT_FALSE(IsTimeoutRunning());
+}
+
 // Tests that a TouchCancel timeout plays nice when the timed out touch stream
 // turns into a scroll gesture sequence.
 TEST_F(TouchEventQueueTest, TouchTimeoutWithFollowupGesture) {