Hook ScrollElasticityController to InputHandlerProxy

content/renderer/input/input_handler_proxy.cc

Index: content/renderer/input/input_handler_proxy.cc
diff --git a/content/renderer/input/input_handler_proxy.cc b/content/renderer/input/input_handler_proxy.cc
index fe6de51a38270a5dbda42c2d74a20d5bd976fa2e..26589795edab4f3042d988174184d0f3dacb538b 100644
--- a/content/renderer/input/input_handler_proxy.cc
+++ b/content/renderer/input/input_handler_proxy.cc
@@ -13,6 +13,7 @@
 #include "content/common/input/web_input_event_traits.h"
 #include "content/public/common/content_switches.h"
 #include "content/renderer/input/input_handler_proxy_client.h"
+#include "content/renderer/input/input_scroll_elasticity_controller.h"
 #include "third_party/WebKit/public/platform/Platform.h"
 #include "third_party/WebKit/public/web/WebInputEvent.h"
 #include "ui/events/latency_info.h"
@@ -161,6 +162,18 @@ InputHandlerProxy::InputHandlerProxy(cc::InputHandler* input_handler,
   input_handler_->BindToClient(this);
   smooth_scroll_enabled_ = CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kEnableSmoothScrolling);
+
+#if defined(OS_MACOSX)
+  if (CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableThreadedEventHandlingMac)) {
+    cc::ScrollElasticityControllerClient* scroll_elasticity_client =
+        input_handler_->GetScrollElasticityControllerClient();
+    scroll_elasticity_controller_.reset(
+        new InputScrollElasticityController(scroll_elasticity_client));
+    scroll_elasticity_client->BindToController(
+        scroll_elasticity_controller_.get());
+  }
+#endif
 }
 
 InputHandlerProxy::~InputHandlerProxy() {}
@@ -276,64 +289,81 @@ InputHandlerProxy::EventDisposition InputHandlerProxy::HandleInputEvent(
 
 InputHandlerProxy::EventDisposition InputHandlerProxy::HandleMouseWheel(
     const WebMouseWheelEvent& wheel_event) {
+  InputHandlerProxy::EventDisposition result = DID_NOT_HANDLE;
+  cc::InputHandlerScrollResult scroll_result;
+
   if (wheel_event.scrollByPage) {
     // TODO(jamesr): We don't properly handle scroll by page in the compositor
     // thread, so punt it to the main thread. http://crbug.com/236639
-    return DID_NOT_HANDLE;
-  }
-  if (wheel_event.modifiers & WebInputEvent::ControlKey) {
+    result = DID_NOT_HANDLE;
+  } else if (wheel_event.modifiers & WebInputEvent::ControlKey) {
     // Wheel events involving the control key never trigger scrolling, only
     // event handlers.  Forward to the main thread.
-    return DID_NOT_HANDLE;
-  }
-  if (smooth_scroll_enabled_) {
+    result = DID_NOT_HANDLE;
+  } else if (smooth_scroll_enabled_) {
     cc::InputHandler::ScrollStatus scroll_status =
         input_handler_->ScrollAnimated(
             gfx::Point(wheel_event.x, wheel_event.y),
             gfx::Vector2dF(-wheel_event.deltaX, -wheel_event.deltaY));
     switch (scroll_status) {
       case cc::InputHandler::ScrollStarted:
-        return DID_HANDLE;
+        result = DID_HANDLE;
+        break;
       case cc::InputHandler::ScrollIgnored:
-        return DROP_EVENT;
+        result = DROP_EVENT;
       default:
-        return DID_NOT_HANDLE;
+        result = DID_NOT_HANDLE;
+        break;
     }
-  }
-  cc::InputHandler::ScrollStatus scroll_status = input_handler_->ScrollBegin(
-      gfx::Point(wheel_event.x, wheel_event.y), cc::InputHandler::Wheel);
-  switch (scroll_status) {
-    case cc::InputHandler::ScrollStarted: {
-      TRACE_EVENT_INSTANT2(
-          "input",
-          "InputHandlerProxy::handle_input wheel scroll",
-          TRACE_EVENT_SCOPE_THREAD,
-          "deltaX",
-          -wheel_event.deltaX,
-          "deltaY",
-          -wheel_event.deltaY);
-      gfx::Point scroll_point(wheel_event.x, wheel_event.y);
-      gfx::Vector2dF scroll_delta(-wheel_event.deltaX, -wheel_event.deltaY);
-      cc::InputHandlerScrollResult scroll_result = input_handler_->ScrollBy(
-          scroll_point, scroll_delta);
-      HandleOverscroll(scroll_point, scroll_result);
-      input_handler_->ScrollEnd();
-      return scroll_result.did_scroll ? DID_HANDLE : DROP_EVENT;
+  } else {
+    cc::InputHandler::ScrollStatus scroll_status = input_handler_->ScrollBegin(
+        gfx::Point(wheel_event.x, wheel_event.y), cc::InputHandler::Wheel);
+    switch (scroll_status) {
+      case cc::InputHandler::ScrollStarted: {
+        TRACE_EVENT_INSTANT2(
+            "input", "InputHandlerProxy::handle_input wheel scroll",
+            TRACE_EVENT_SCOPE_THREAD, "deltaX", -wheel_event.deltaX, "deltaY",
+            -wheel_event.deltaY);
+        gfx::Point scroll_point(wheel_event.x, wheel_event.y);
+        gfx::Vector2dF scroll_delta(-wheel_event.deltaX, -wheel_event.deltaY);
+        scroll_result = input_handler_->ScrollBy(scroll_point, scroll_delta);
+        HandleOverscroll(scroll_point, scroll_result);
+        input_handler_->ScrollEnd();
+        result = scroll_result.did_scroll ? DID_HANDLE : DROP_EVENT;
+        break;
+      }
+      case cc::InputHandler::ScrollIgnored:
+        // TODO(jamesr): This should be DROP_EVENT, but in cases where we fail
+        // to properly sync scrollability it's safer to send the event to the
+        // main thread. Change back to DROP_EVENT once we have synchronization
+        // bugs sorted out.
+        result = DID_NOT_HANDLE;
+        break;
+      case cc::InputHandler::ScrollUnknown:
+      case cc::InputHandler::ScrollOnMainThread:
+        result = DID_NOT_HANDLE;
+        break;
+      case cc::InputHandler::ScrollStatusCount:
+        NOTREACHED();
+        break;
     }
-    case cc::InputHandler::ScrollIgnored:
-      // TODO(jamesr): This should be DROP_EVENT, but in cases where we fail
-      // to properly sync scrollability it's safer to send the event to the
-      // main thread. Change back to DROP_EVENT once we have synchronization
-      // bugs sorted out.
-      return DID_NOT_HANDLE;
-    case cc::InputHandler::ScrollUnknown:
-    case cc::InputHandler::ScrollOnMainThread:
-      return DID_NOT_HANDLE;
-    case cc::InputHandler::ScrollStatusCount:
-      NOTREACHED();
-      break;
   }
-  return DID_NOT_HANDLE;
+
+  // Send the event and its disposition to the elasticity controller to update
+  // the over-scroll animation. If the event is to be handled on the main
+  // thread, the event and its disposition will be sent after being handled

[jdduke (slow), 2014/11/12 02:32:24]

Maybe tweak this to say something like "... and its disposition will be sent to
the elasticity controller after being..." just to make it explicit where exactly
they will be forwarded?

[ccameron, 2014/11/12 08:15:29]

Good point -- changed to " event and its disposition will be sent to the
elasticity controller after being handled on the main thread."

+  // there.
+  if (scroll_elasticity_controller_ && result != DID_NOT_HANDLE) {
+    // Note that the call to the elasticity controller is made asynchronously,
+    // to minimize divergence between main thread and impl thread event
+    // handling paths.
+    base::MessageLoop::current()->PostTask(

[jdduke (slow), 2014/11/12 00:55:34]

Is this a speculative solution? Or have you observed severe divergence when this
call is synchronous? Wheel events are already async with respect to their source
(the browser), so I can't say that I completely understand how reposting the
task to the compositor thread will improve divergence from the main thread other
than introducing synthetic lag.

[ccameron, 2014/11/12 02:08:24]

This was aelias' suggestion (aelais: please verify that this is what you meant)
-- not so much to fix bugs, but to make sure that I wasn't relying on any
side-effects of doing the call synchronously. For instance, if calling
ObserveWheelEvent before any callbacks caused by InputHandler::ScrollBy hides
bugs, then we'd only see them when handling main-thread events. This will at
least make those sorts of things happen for both main- and impl-thread paths.

There is some discussion of bouncing this all the way back to the browser (so
that we can have the main and impl paths be identical), but I'm loathe to add
that plumbing until we see that it's needed in the context of the rest of the
work (namely, the main thread overscroll support).

[aelias_OOO_until_Jul13, 2014/11/12 02:27:41]

Yes, I suggested it based on my past experience that having the same code called
synchronously sometimes and async other times can have surprising consequences. 
Although I don't have any more specific reasons than that, for this use case.  I
agree it does look unnatural to do it this way and my preference would be to
always bounce the messages to the browser.

[jdduke (slow), 2014/11/12 02:32:24]

OK, thanks for the explanation, seems reasonable enough.

[ccameron, 2014/11/12 08:15:29]

To be clear, that may be the right thing to do, I just don't want to do it until
we have the other half (the main thread part) sorted out. When I get to that,
I'll probably write up the two different paths, and we can choose which of the
to CLs look better (I just lack the foresight to know which will be better ahead
of time).  

+        FROM_HERE,
+        base::Bind(&InputScrollElasticityController::ObserveWheelEventAndResult,
+                   scroll_elasticity_controller_->GetWeakPtr(), wheel_event,
+                   scroll_result));
+  }
+  return result;
 }
 
 InputHandlerProxy::EventDisposition InputHandlerProxy::HandleGestureScrollBegin(