input: Make the OverscrollController less intrusive, and some code cleanup.

* Make OverscrollController less intrusive to input-event dispatching. With this
  change, the OverscrollController consumes an event on its way to dispatch only
  if the event contributes to the overscroll.
* When an overscroll is in progress, and a gesture event comes in, this event is
  not dispatched to the GestureEventFilter from RenderWidgetHostImpl. So, if the
  event should be dispatched to the renderer, make sure it always goes through
  the host from the OverscrollController.
* Remove HasQueuedGestureEvents() from InputRouter interface, and forward all
  gesture events through the RenderWidgetHost from the overscroll-controller.

BUG=none
R=aelias@chromium.org, jdduke@chromium.org

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

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

11 files changed, 42 insertions, 85 deletions

diff --git a/content/browser/renderer_host/input/buffered_input_router.cc b/content/browser/renderer_host/input/buffered_input_router.cc
index 3f0fb23..97b5653 100644
--- a/content/browser/renderer_host/input/buffered_input_router.cc
+++ b/content/browser/renderer_host/input/buffered_input_router.cc
@@ -31,7 +31,6 @@ BufferedInputRouter::BufferedInputRouter(IPC::Sender* sender,
       ack_handler_(ack_handler),
       sender_(sender),
       routing_id_(routing_id),
-      queued_gesture_count_(0),
       has_touch_handler_(false),
       queued_touch_count_(0),
       input_queue_override_(NULL),
@@ -89,8 +88,7 @@ void BufferedInputRouter::SendGestureEvent(
     const GestureEventWithLatencyInfo& gesture_event) {
   if (!client_->OnSendGestureEvent(gesture_event))
     return;
-  if (QueueWebEvent(gesture_event.event, gesture_event.latency, false))
-    ++queued_gesture_count_;
+  QueueWebEvent(gesture_event.event, gesture_event.latency, false);
 }
 
 void BufferedInputRouter::SendTouchEvent(
@@ -207,10 +205,6 @@ bool BufferedInputRouter::ShouldForwardGestureEvent(
   return true;
 }
 
-bool BufferedInputRouter::HasQueuedGestureEvents() const {
-  return queued_gesture_count_ > 0;
-}
-
 void BufferedInputRouter::OnWebInputEventAck(
     int64 event_id,
     const WebKit::WebInputEvent& web_event,
@@ -245,10 +239,6 @@ void BufferedInputRouter::OnWebInputEventAck(
                                   latency_info),
         acked_result);
   } else if (WebInputEvent::isGestureEventType(web_event.type)) {
-    if (ack_from_input_queue) {
-      DCHECK_GT(queued_gesture_count_, 0);
-      --queued_gesture_count_;
-    }
     ack_handler_->OnGestureEventAck(
         static_cast<const WebGestureEvent&>(web_event), acked_result);
   } else
diff --git a/content/browser/renderer_host/input/buffered_input_router.h b/content/browser/renderer_host/input/buffered_input_router.h
index e1652f5..a9802a6 100644
--- a/content/browser/renderer_host/input/buffered_input_router.h
+++ b/content/browser/renderer_host/input/buffered_input_router.h
@@ -67,7 +67,6 @@ class CONTENT_EXPORT BufferedInputRouter
   virtual bool ShouldForwardTouchEvent() const OVERRIDE;
   virtual bool ShouldForwardGestureEvent(
       const GestureEventWithLatencyInfo& gesture_event) const OVERRIDE;
-  virtual bool HasQueuedGestureEvents() const OVERRIDE;
 
   // InputQueueClient
   virtual void Deliver(const EventPacket& packet) OVERRIDE;
@@ -124,9 +123,6 @@ class CONTENT_EXPORT BufferedInputRouter
   typedef std::map<int64, NativeWebKeyboardEvent> KeyMap;
   KeyMap queued_key_map_;
 
-  // Necessary for |HasQueuedGestureEvents()|.
-  int queued_gesture_count_;
-
   // Necessary for |ShouldForwardTouchEvent()|.
   bool has_touch_handler_;
   int queued_touch_count_;
diff --git a/content/browser/renderer_host/input/buffered_input_router_unittest.cc b/content/browser/renderer_host/input/buffered_input_router_unittest.cc
index e34cf60..4d63444 100644
--- a/content/browser/renderer_host/input/buffered_input_router_unittest.cc
+++ b/content/browser/renderer_host/input/buffered_input_router_unittest.cc
@@ -267,21 +267,6 @@ TEST_F(BufferedInputRouterTest, FlushRequestedOnQueue) {
   SCOPED_EXPECT(client_->ExpectNeedsFlushCalled(false), "SetNeedsFlushCalled");
 }
 
-TEST_F(BufferedInputRouterTest, HasQueuedGestureEvents) {
-  EXPECT_FALSE(input_router_->HasQueuedGestureEvents());
-  SimulateGestureEvent(WebInputEvent::GestureScrollBegin,
-                       WebGestureEvent::Touchpad);
-  EXPECT_TRUE(input_router_->HasQueuedGestureEvents());
-
-  // Only an ack'ed gesture should clear it from the queue.
-  input_router_->Flush();
-  ASSERT_TRUE(FinishFlush(INPUT_EVENT_COULD_NOT_DELIVER));
-  EXPECT_TRUE(input_router_->HasQueuedGestureEvents());
-
-  ASSERT_TRUE(FinishFlush(INPUT_EVENT_IMPL_THREAD_CONSUMED));
-  EXPECT_FALSE(input_router_->HasQueuedGestureEvents());
-}
-
 TEST_F(BufferedInputRouterTest, GetLastKeyboardEvent) {
   EXPECT_EQ(NULL, input_router_->GetLastKeyboardEvent());
 
diff --git a/content/browser/renderer_host/input/immediate_input_router.cc b/content/browser/renderer_host/input/immediate_input_router.cc
index 3e34bbe..5613b42 100644
--- a/content/browser/renderer_host/input/immediate_input_router.cc
+++ b/content/browser/renderer_host/input/immediate_input_router.cc
@@ -287,10 +287,6 @@ bool ImmediateInputRouter::ShouldForwardGestureEvent(
   return gesture_event_filter_->ShouldForward(touch_event);
 }
 
-bool ImmediateInputRouter::HasQueuedGestureEvents() const {
-  return gesture_event_filter_->HasQueuedGestureEvents();
-}
-
 bool ImmediateInputRouter::OnMessageReceived(const IPC::Message& message) {
   bool handled = true;
   bool message_is_ok = true;
diff --git a/content/browser/renderer_host/input/immediate_input_router.h b/content/browser/renderer_host/input/immediate_input_router.h
index f5aa167..1d1a0c0 100644
--- a/content/browser/renderer_host/input/immediate_input_router.h
+++ b/content/browser/renderer_host/input/immediate_input_router.h
@@ -62,7 +62,6 @@ class CONTENT_EXPORT ImmediateInputRouter
   virtual bool ShouldForwardTouchEvent() const OVERRIDE;
   virtual bool ShouldForwardGestureEvent(
       const GestureEventWithLatencyInfo& gesture_event) const OVERRIDE;
-  virtual bool HasQueuedGestureEvents() const OVERRIDE;
 
   // IPC::Listener
   virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
diff --git a/content/browser/renderer_host/input/input_router.h b/content/browser/renderer_host/input/input_router.h
index 93b7b3f..5e4533f 100644
--- a/content/browser/renderer_host/input/input_router.h
+++ b/content/browser/renderer_host/input/input_router.h
@@ -64,9 +64,6 @@ class InputRouter : public IPC::Listener {
   // |gesture_event| to the router.
   virtual bool ShouldForwardGestureEvent(
       const GestureEventWithLatencyInfo& gesture_event) const = 0;
-
-  // Returns |true| if the router has any queued or in-flight gesture events.
-  virtual bool HasQueuedGestureEvents() const = 0;
 };
 
 }  // namespace content
diff --git a/content/browser/renderer_host/overscroll_controller.cc b/content/browser/renderer_host/overscroll_controller.cc
index 51ff7df..f07efab 100644
--- a/content/browser/renderer_host/overscroll_controller.cc
+++ b/content/browser/renderer_host/overscroll_controller.cc
@@ -64,23 +64,25 @@ bool OverscrollController::WillDispatchEvent(
     // touch-scrolls maintain state in the renderer side (in the compositor, for
     // example), and the event that completes this action needs to be sent to
     // the renderer so that those states can be updated/reset appropriately.
-    // Send the event through the host when appropriate.
-    if (ShouldForwardToHost(event)) {
+    if (WebKit::WebInputEvent::isGestureEventType(event.type)) {
+      // A gesture-event isn't sent to the GestureEventFilter when overscroll is
+      // in progress. So dispatch the event through the RenderWidgetHost so that
+      // it can reach the GestureEventFilter.
       const WebKit::WebGestureEvent& gevent =
           static_cast<const WebKit::WebGestureEvent&>(event);
       return render_widget_host_->ShouldForwardGestureEvent(
           GestureEventWithLatencyInfo(gevent, latency_info));
     }
 
-    return false;
+    return true;
   }
 
   if (overscroll_mode_ != OVERSCROLL_NONE && DispatchEventResetsState(event)) {
     SetOverscrollMode(OVERSCROLL_NONE);
-    // The overscroll gesture status is being reset. If the event is a
-    // gesture event (from either touchscreen or trackpad), then make sure the
-    // host gets the event first (if it didn't already process it).
-    if (ShouldForwardToHost(event)) {
+    if (WebKit::WebInputEvent::isGestureEventType(event.type)) {
+      // A gesture-event isn't sent to the GestureEventFilter when overscroll is
+      // in progress. So dispatch the event through the RenderWidgetHost so that
+      // it can reach the GestureEventFilter.
       const WebKit::WebGestureEvent& gevent =
           static_cast<const WebKit::WebGestureEvent&>(event);
       return render_widget_host_->ShouldForwardGestureEvent(
@@ -92,16 +94,9 @@ bool OverscrollController::WillDispatchEvent(
   }
 
   if (overscroll_mode_ != OVERSCROLL_NONE) {
-    // Consume the event and update overscroll state when in the middle of the
-    // overscroll gesture.
-    ProcessEventForOverscroll(event);
-
-    if (event.type == WebKit::WebInputEvent::TouchEnd ||
-        event.type == WebKit::WebInputEvent::TouchCancel ||
-        event.type == WebKit::WebInputEvent::TouchMove) {
-      return true;
-    }
-    return false;
+    // Consume the event only if it updates the overscroll state.
+    if (ProcessEventForOverscroll(event))
+      return false;
   }
 
   return true;
@@ -226,18 +221,20 @@ bool OverscrollController::DispatchEventResetsState(
   }
 }
 
-void OverscrollController::ProcessEventForOverscroll(
+bool OverscrollController::ProcessEventForOverscroll(
     const WebKit::WebInputEvent& event) {
+  bool event_processed = false;
   switch (event.type) {
     case WebKit::WebInputEvent::MouseWheel: {
       const WebKit::WebMouseWheelEvent& wheel =
           static_cast<const WebKit::WebMouseWheelEvent&>(event);
       if (!wheel.hasPreciseScrollingDeltas)
-        return;
+        break;
 
       ProcessOverscroll(wheel.deltaX * wheel.accelerationRatioX,
                         wheel.deltaY * wheel.accelerationRatioY,
                         wheel.type);
+      event_processed = true;
       break;
     }
     case WebKit::WebInputEvent::GestureScrollUpdate: {
@@ -246,6 +243,7 @@ void OverscrollController::ProcessEventForOverscroll(
       ProcessOverscroll(gesture.data.scrollUpdate.deltaX,
                         gesture.data.scrollUpdate.deltaY,
                         gesture.type);
+      event_processed = true;
       break;
     }
     case WebKit::WebInputEvent::GestureFlingStart: {
@@ -258,12 +256,14 @@ void OverscrollController::ProcessEventForOverscroll(
         if ((overscroll_mode_ == OVERSCROLL_WEST && velocity_x < 0) ||
             (overscroll_mode_ == OVERSCROLL_EAST && velocity_x > 0)) {
           CompleteAction();
+          event_processed = true;
           break;
         }
       } else if (fabs(velocity_y) > kFlingVelocityThreshold) {
         if ((overscroll_mode_ == OVERSCROLL_NORTH && velocity_y < 0) ||
             (overscroll_mode_ == OVERSCROLL_SOUTH && velocity_y > 0)) {
           CompleteAction();
+          event_processed = true;
           break;
         }
       }
@@ -278,6 +278,7 @@ void OverscrollController::ProcessEventForOverscroll(
              WebKit::WebInputEvent::isTouchEventType(event.type))
           << "Received unexpected event: " << event.type;
   }
+  return event_processed;
 }
 
 void OverscrollController::ProcessOverscroll(float delta_x,
@@ -367,14 +368,4 @@ void OverscrollController::SetOverscrollMode(OverscrollMode mode) {
     delegate_->OnOverscrollModeChange(old_mode, overscroll_mode_);
 }
 
-bool OverscrollController::ShouldForwardToHost(
-    const WebKit::WebInputEvent& event) const {
-  if (!WebKit::WebInputEvent::isGestureEventType(event.type))
-    return false;
-
-  // If the RenderWidgetHost already processed this event, then the event must
-  // not be sent again.
-  return !render_widget_host_->HasQueuedGestureEvents();
-}
-
 }  // namespace content
diff --git a/content/browser/renderer_host/overscroll_controller.h b/content/browser/renderer_host/overscroll_controller.h
index 9c8282e..fab5319 100644
--- a/content/browser/renderer_host/overscroll_controller.h
+++ b/content/browser/renderer_host/overscroll_controller.h
@@ -89,8 +89,9 @@ class OverscrollController {
   // overscroll gesture status.
   bool DispatchEventResetsState(const WebKit::WebInputEvent& event) const;
 
-  // Processes an event and updates internal state for overscroll.
-  void ProcessEventForOverscroll(const WebKit::WebInputEvent& event);
+  // Processes an event to update the internal state for overscroll. Returns
+  // true if the state is updated, false otherwise.
+  bool ProcessEventForOverscroll(const WebKit::WebInputEvent& event);
 
   // Processes horizontal overscroll. This can update both the overscroll mode
   // and the over scroll amount (i.e. |overscroll_mode_|, |overscroll_delta_x_|
@@ -106,10 +107,6 @@ class OverscrollController {
   // appropriate).
   void SetOverscrollMode(OverscrollMode new_mode);
 
-  // Returns whether the input event should be forwarded to the
-  // RenderWidgetHost.
-  bool ShouldForwardToHost(const WebKit::WebInputEvent& event) const;
-
   // The RenderWidgetHost that owns this overscroll controller.
   RenderWidgetHostImpl* render_widget_host_;
 
diff --git a/content/browser/renderer_host/render_widget_host_impl.cc b/content/browser/renderer_host/render_widget_host_impl.cc
index 30b0ecb..1f81647 100644
--- a/content/browser/renderer_host/render_widget_host_impl.cc
+++ b/content/browser/renderer_host/render_widget_host_impl.cc
@@ -2235,10 +2235,6 @@ bool RenderWidgetHostImpl::ShouldForwardGestureEvent(
   return input_router_->ShouldForwardGestureEvent(gesture_event);
 }
 
-bool RenderWidgetHostImpl::HasQueuedGestureEvents() const {
-  return input_router_->HasQueuedGestureEvents();
-}
-
 void RenderWidgetHostImpl::StartUserGesture() {
   OnUserGesture();
 }
diff --git a/content/browser/renderer_host/render_widget_host_impl.h b/content/browser/renderer_host/render_widget_host_impl.h
index 431eb5c..c250404 100644
--- a/content/browser/renderer_host/render_widget_host_impl.h
+++ b/content/browser/renderer_host/render_widget_host_impl.h
@@ -370,7 +370,6 @@ class CONTENT_EXPORT RenderWidgetHostImpl : virtual public RenderWidgetHost,
   bool ShouldForwardTouchEvent() const;
   bool ShouldForwardGestureEvent(
       const GestureEventWithLatencyInfo& gesture_event) const;
-  bool HasQueuedGestureEvents() const;
 
   bool has_touch_handler() const { return has_touch_handler_; }
 
diff --git a/content/browser/renderer_host/render_widget_host_unittest.cc b/content/browser/renderer_host/render_widget_host_unittest.cc
index 95620f9..e7ab22d 100644
--- a/content/browser/renderer_host/render_widget_host_unittest.cc
+++ b/content/browser/renderer_host/render_widget_host_unittest.cc
@@ -171,7 +171,6 @@ class MockInputRouter : public InputRouter {
       const GestureEventWithLatencyInfo& gesture_event) const OVERRIDE {
     return true;
   }
-  virtual bool HasQueuedGestureEvents() const OVERRIDE { return true; }
 
   // IPC::Listener
   virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE {
@@ -1385,6 +1384,7 @@ TEST_F(RenderWidgetHostTest, WheelScrollEventOverscrolls) {
   SimulateMouseMove(5, 10, 0);
   EXPECT_EQ(OVERSCROLL_NONE, host_->overscroll_mode());
   EXPECT_EQ(OVERSCROLL_NONE, host_->overscroll_delegate()->current_mode());
+  EXPECT_EQ(1U, process_->sink().message_count());
 }
 
 // Tests that if some scroll events are consumed towards the start, then
@@ -1606,10 +1606,10 @@ TEST_F(RenderWidgetHostTest, ScrollEventsOverscrollWithZeroFling) {
   // Send a fling start, but with a small velocity, so that the overscroll is
   // aborted. The fling should proceed to the renderer, through the gesture
   // event filter.
-  SimulateGestureFlingStartEvent(0.f, 0.f, WebGestureEvent::Touchpad);
+  SimulateGestureFlingStartEvent(10.f, 0.f, WebGestureEvent::Touchpad);
   EXPECT_EQ(OVERSCROLL_NONE, host_->overscroll_mode());
-  EXPECT_EQ(0U, host_->GestureEventLastQueueEventSize());
-  EXPECT_EQ(0U, process_->sink().message_count());
+  EXPECT_EQ(1U, host_->GestureEventLastQueueEventSize());
+  EXPECT_EQ(1U, process_->sink().message_count());
 }
 
 // Tests that a fling in the opposite direction of the overscroll cancels the
@@ -1634,11 +1634,13 @@ TEST_F(RenderWidgetHostTest, ReverseFlingCancelsOverscroll) {
                       INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
     EXPECT_EQ(OVERSCROLL_EAST, host_->overscroll_mode());
     EXPECT_EQ(OVERSCROLL_EAST, host_->overscroll_delegate()->current_mode());
+    process_->sink().ClearMessages();
 
     SimulateGestureEvent(WebInputEvent::GestureScrollEnd,
                          WebGestureEvent::Touchscreen);
     EXPECT_EQ(OVERSCROLL_EAST, host_->overscroll_delegate()->completed_mode());
     EXPECT_EQ(OVERSCROLL_NONE, host_->overscroll_delegate()->current_mode());
+    EXPECT_EQ(1U, process_->sink().message_count());
     SendInputEventACK(WebInputEvent::GestureScrollEnd,
                       INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
   }
@@ -1658,9 +1660,12 @@ TEST_F(RenderWidgetHostTest, ReverseFlingCancelsOverscroll) {
                       INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
     EXPECT_EQ(OVERSCROLL_WEST, host_->overscroll_mode());
     EXPECT_EQ(OVERSCROLL_WEST, host_->overscroll_delegate()->current_mode());
+    process_->sink().ClearMessages();
+
     SimulateGestureFlingStartEvent(100, 0, WebGestureEvent::Touchscreen);
     EXPECT_EQ(OVERSCROLL_NONE, host_->overscroll_delegate()->completed_mode());
     EXPECT_EQ(OVERSCROLL_NONE, host_->overscroll_delegate()->current_mode());
+    EXPECT_EQ(1U, process_->sink().message_count());
   }
 }
 
@@ -1738,11 +1743,14 @@ TEST_F(RenderWidgetHostTest, GestureScrollConsumedHorizontal) {
                     INPUT_EVENT_ACK_STATE_CONSUMED);
   EXPECT_EQ(OVERSCROLL_NONE, host_->overscroll_mode());
   EXPECT_EQ(OVERSCROLL_NONE, host_->overscroll_delegate()->current_mode());
+  process_->sink().ClearMessages();
 
   // Send another gesture event and ACK as not being processed. This should
   // not initiate overscroll because the beginning of the scroll event did
-  // scroll some content on the page.
+  // scroll some content on the page. Since there was no overscroll, the event
+  // should reach the renderer.
   SimulateGestureScrollUpdateEvent(55, 0, 0);
+  EXPECT_EQ(1U, process_->sink().message_count());
   SendInputEventACK(WebInputEvent::GestureScrollUpdate,
                     INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
   EXPECT_EQ(OVERSCROLL_NONE, host_->overscroll_mode());
@@ -2289,12 +2297,15 @@ TEST_F(RenderWidgetHostTest, OverscrollMouseMoveCompletion) {
 
   // Overscroll gesture is in progress. Send a mouse-move now. This should
   // complete the gesture (because the amount overscrolled is above the
-  // threshold), and consume the event.
+  // threshold).
   SimulateMouseMove(5, 10, 0);
   EXPECT_EQ(OVERSCROLL_EAST, host_->overscroll_delegate()->completed_mode());
   EXPECT_EQ(OVERSCROLL_NONE, host_->overscroll_mode());
   EXPECT_EQ(OVERSCROLL_NONE, host_->overscroll_delegate()->current_mode());
-  EXPECT_EQ(0U, process_->sink().message_count());
+  EXPECT_EQ(1U, process_->sink().message_count());
+  process_->sink().ClearMessages();
+  SendInputEventACK(WebInputEvent::MouseMove,
+                    INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
 
   SimulateGestureEvent(WebInputEvent::GestureScrollEnd,
                        WebGestureEvent::Touchscreen);