Give blink a chance to consume ctrl+wheel events before zooming

Today chromium consumes all mouse wheel events when the control key is pressed for the purposes of browser zoom (except on MacOS).  With this change we give the web page a chance to get and consume the wheel event first, in case it wants to override the behavior.  This makes Chrome consistent with IE and Firefox on Windows.  See http://crbug.com/289887 and the linked chromium-dev thread for discussion.

Apparently there were no tests at all for this functionality.  I've added new unit tests at RenderWidgetHost and WebContents layers.

Depends on blink change http://src.chromium.org/viewvc/blink?view=rev&rev=168088

BUG=289887

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

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

9 files changed, 178 insertions, 20 deletions

diff --git a/content/browser/renderer_host/render_widget_host_delegate.cc b/content/browser/renderer_host/render_widget_host_delegate.cc
index 09f4967..a3b66e2 100644
--- a/content/browser/renderer_host/render_widget_host_delegate.cc
+++ b/content/browser/renderer_host/render_widget_host_delegate.cc
@@ -12,7 +12,7 @@ bool RenderWidgetHostDelegate::PreHandleKeyboardEvent(
   return false;
 }
 
-bool RenderWidgetHostDelegate::PreHandleWheelEvent(
+bool RenderWidgetHostDelegate::HandleWheelEvent(
     const blink::WebMouseWheelEvent& event) {
   return false;
 }
diff --git a/content/browser/renderer_host/render_widget_host_delegate.h b/content/browser/renderer_host/render_widget_host_delegate.h
index aac41d8..e31975c 100644
--- a/content/browser/renderer_host/render_widget_host_delegate.h
+++ b/content/browser/renderer_host/render_widget_host_delegate.h
@@ -42,10 +42,10 @@ class CONTENT_EXPORT RenderWidgetHostDelegate {
   // event (used for keyboard shortcuts).
   virtual void HandleKeyboardEvent(const NativeWebKeyboardEvent& event) {}
 
-  // Callback to give the browser a chance to handle the specified mouse wheel
-  // event before sending it to the renderer.
-  // Returns true if the |event| was handled.
-  virtual bool PreHandleWheelEvent(const blink::WebMouseWheelEvent& event);
+  // Callback to inform the browser that the renderer did not process the
+  // specified mouse wheel event.  Returns true if the browser has handled
+  // the event itself.
+  virtual bool HandleWheelEvent(const blink::WebMouseWheelEvent& event);
 
   // Callback to give the browser a chance to handle the specified gesture
   // event before sending it to the renderer.
diff --git a/content/browser/renderer_host/render_widget_host_impl.cc b/content/browser/renderer_host/render_widget_host_impl.cc
index b827c14..c715fca 100644
--- a/content/browser/renderer_host/render_widget_host_impl.cc
+++ b/content/browser/renderer_host/render_widget_host_impl.cc
@@ -973,9 +973,6 @@ void RenderWidgetHostImpl::ForwardWheelEventWithLatencyInfo(
   if (IgnoreInputEvents())
     return;
 
-  if (delegate_->PreHandleWheelEvent(wheel_event))
-    return;
-
   input_router_->SendWheelEvent(MouseWheelEventWithLatencyInfo(wheel_event,
                                                                latency_info));
 }
@@ -2062,8 +2059,10 @@ void RenderWidgetHostImpl::OnWheelEventAck(
   }
 
   const bool processed = (INPUT_EVENT_ACK_STATE_CONSUMED == ack_result);
-  if (!processed && !is_hidden() && view_)
-    view_->UnhandledWheelEvent(wheel_event.event);
+  if (!processed && !is_hidden() && view_) {
+    if (!delegate_->HandleWheelEvent(wheel_event.event))
+      view_->UnhandledWheelEvent(wheel_event.event);
+  }
 }
 
 void RenderWidgetHostImpl::OnGestureEventAck(
diff --git a/content/browser/renderer_host/render_widget_host_unittest.cc b/content/browser/renderer_host/render_widget_host_unittest.cc
index d597581..3e6253a 100644
--- a/content/browser/renderer_host/render_widget_host_unittest.cc
+++ b/content/browser/renderer_host/render_widget_host_unittest.cc
@@ -429,6 +429,7 @@ class TestView : public TestRenderWidgetHostView {
  public:
   explicit TestView(RenderWidgetHostImpl* rwh)
       : TestRenderWidgetHostView(rwh),
+        unhandled_wheel_event_count_(0),
         acked_event_count_(0),
         gesture_event_type_(-1),
         use_fake_physical_backing_size_(false),
@@ -450,6 +451,9 @@ class TestView : public TestRenderWidgetHostView {
   const WebMouseWheelEvent& unhandled_wheel_event() const {
     return unhandled_wheel_event_;
   }
+  int unhandled_wheel_event_count() const {
+    return unhandled_wheel_event_count_;
+  }
   int gesture_event_type() const { return gesture_event_type_; }
   InputEventAckState ack_result() const { return ack_result_; }
 
@@ -471,6 +475,7 @@ class TestView : public TestRenderWidgetHostView {
     ++acked_event_count_;
   }
   virtual void UnhandledWheelEvent(const WebMouseWheelEvent& event) OVERRIDE {
+    unhandled_wheel_event_count_++;
     unhandled_wheel_event_ = event;
   }
   virtual void GestureEventAck(const WebGestureEvent& event,
@@ -486,6 +491,7 @@ class TestView : public TestRenderWidgetHostView {
 
  protected:
   WebMouseWheelEvent unhandled_wheel_event_;
+  int unhandled_wheel_event_count_;
   WebTouchEvent acked_event_;
   int acked_event_count_;
   int gesture_event_type_;
@@ -506,7 +512,9 @@ class MockRenderWidgetHostDelegate : public RenderWidgetHostDelegate {
         prehandle_keyboard_event_called_(false),
         prehandle_keyboard_event_type_(WebInputEvent::Undefined),
         unhandled_keyboard_event_called_(false),
-        unhandled_keyboard_event_type_(WebInputEvent::Undefined) {
+        unhandled_keyboard_event_type_(WebInputEvent::Undefined),
+        handle_wheel_event_(false),
+        handle_wheel_event_called_(false) {
   }
   virtual ~MockRenderWidgetHostDelegate() {}
 
@@ -532,6 +540,14 @@ class MockRenderWidgetHostDelegate : public RenderWidgetHostDelegate {
     prehandle_keyboard_event_ = handle;
   }
 
+  void set_handle_wheel_event(bool handle) {
+    handle_wheel_event_ = handle;
+  }
+
+  bool handle_wheel_event_called() {
+    return handle_wheel_event_called_;
+  }
+
  protected:
   virtual bool PreHandleKeyboardEvent(const NativeWebKeyboardEvent& event,
                                       bool* is_keyboard_shortcut) OVERRIDE {
@@ -546,6 +562,12 @@ class MockRenderWidgetHostDelegate : public RenderWidgetHostDelegate {
     unhandled_keyboard_event_called_ = true;
   }
 
+  virtual bool HandleWheelEvent(
+      const blink::WebMouseWheelEvent& event) OVERRIDE {
+    handle_wheel_event_called_ = true;
+    return handle_wheel_event_;
+  }
+
  private:
   bool prehandle_keyboard_event_;
   bool prehandle_keyboard_event_called_;
@@ -553,6 +575,9 @@ class MockRenderWidgetHostDelegate : public RenderWidgetHostDelegate {
 
   bool unhandled_keyboard_event_called_;
   WebInputEvent::Type unhandled_keyboard_event_type_;
+
+  bool handle_wheel_event_;
+  bool handle_wheel_event_called_;
 };
 
 // RenderWidgetHostTest --------------------------------------------------------
@@ -1150,9 +1175,33 @@ TEST_F(RenderWidgetHostTest, UnhandledWheelEvent) {
   // Send the simulated response from the renderer back.
   SendInputEventACK(WebInputEvent::MouseWheel,
                     INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
+  EXPECT_TRUE(delegate_->handle_wheel_event_called());
+  EXPECT_EQ(1, view_->unhandled_wheel_event_count());
   EXPECT_EQ(-5, view_->unhandled_wheel_event().deltaX);
 }
 
+TEST_F(RenderWidgetHostTest, HandleWheelEvent) {
+  // Indicate that we're going to handle this wheel event
+  delegate_->set_handle_wheel_event(true);
+
+  SimulateWheelEvent(-5, 0, 0, true);
+
+  // Make sure we sent the input event to the renderer.
+  EXPECT_TRUE(process_->sink().GetUniqueMessageMatching(
+                  InputMsg_HandleInputEvent::ID));
+  process_->sink().ClearMessages();
+
+  // Send the simulated response from the renderer back.
+  SendInputEventACK(WebInputEvent::MouseWheel,
+                    INPUT_EVENT_ACK_STATE_NOT_CONSUMED);
+
+  // ensure the wheel event handler was invoked
+  EXPECT_TRUE(delegate_->handle_wheel_event_called());
+
+  // and that it suppressed the unhandled wheel event handler.
+  EXPECT_EQ(0, view_->unhandled_wheel_event_count());
+}
+
 TEST_F(RenderWidgetHostTest, UnhandledGestureEvent) {
   SimulateGestureEvent(WebInputEvent::GestureTwoFingerTap,
                        WebGestureEvent::Touchscreen);
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index b573d67..6d3609f7 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -1164,7 +1164,7 @@ void WebContentsImpl::HandleKeyboardEvent(const NativeWebKeyboardEvent& event) {
     delegate_->HandleKeyboardEvent(this, event);
 }
 
-bool WebContentsImpl::PreHandleWheelEvent(
+bool WebContentsImpl::HandleWheelEvent(
     const blink::WebMouseWheelEvent& event) {
 #if !defined(OS_MACOSX)
   // On platforms other than Mac, control+mousewheel changes zoom. On Mac, this
@@ -1180,7 +1180,6 @@ bool WebContentsImpl::PreHandleWheelEvent(
     return true;
   }
 #endif
-
   return false;
 }
 
diff --git a/content/browser/web_contents/web_contents_impl.h b/content/browser/web_contents/web_contents_impl.h
index af604bc..ff8e899 100644
--- a/content/browser/web_contents/web_contents_impl.h
+++ b/content/browser/web_contents/web_contents_impl.h
@@ -500,7 +500,7 @@ class CONTENT_EXPORT WebContentsImpl
       bool* is_keyboard_shortcut) OVERRIDE;
   virtual void HandleKeyboardEvent(
       const NativeWebKeyboardEvent& event) OVERRIDE;
-  virtual bool PreHandleWheelEvent(
+  virtual bool HandleWheelEvent(
       const blink::WebMouseWheelEvent& event) OVERRIDE;
   virtual bool PreHandleGestureEvent(
       const blink::WebGestureEvent& event) OVERRIDE;
diff --git a/content/browser/web_contents/web_contents_impl_unittest.cc b/content/browser/web_contents/web_contents_impl_unittest.cc
index 635543d..a116820 100644
--- a/content/browser/web_contents/web_contents_impl_unittest.cc
+++ b/content/browser/web_contents/web_contents_impl_unittest.cc
@@ -11,6 +11,7 @@
 #include "content/browser/site_instance_impl.h"
 #include "content/browser/webui/web_ui_controller_factory_registry.h"
 #include "content/common/frame_messages.h"
+#include "content/common/input/synthetic_web_input_event_builders.h"
 #include "content/common/view_messages.h"
 #include "content/public/browser/global_request_id.h"
 #include "content/public/browser/interstitial_page_delegate.h"
@@ -18,6 +19,7 @@
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/render_widget_host_view.h"
+#include "content/public/browser/web_contents_delegate.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/browser/web_ui_controller.h"
 #include "content/public/common/bindings_policy.h"
@@ -2232,4 +2234,93 @@ TEST_F(WebContentsImplTest, GetLastActiveTime) {
   EXPECT_FALSE(contents()->GetLastActiveTime().is_null());
 }
 
+class ContentsZoomChangedDelegate : public WebContentsDelegate {
+ public:
+  ContentsZoomChangedDelegate() :
+    contents_zoom_changed_call_count_(0),
+    last_zoom_in_(false) {
+  }
+
+  int GetAndResetContentsZoomChangedCallCount() {
+    int count = contents_zoom_changed_call_count_;
+    contents_zoom_changed_call_count_ = 0;
+    return count;
+  }
+
+  bool last_zoom_in() const {
+    return last_zoom_in_;
+  }
+
+  // WebContentsDelegate:
+  virtual void ContentsZoomChange(bool zoom_in) OVERRIDE {
+    contents_zoom_changed_call_count_++;
+    last_zoom_in_ = zoom_in;
+  }
+
+ private:
+  int contents_zoom_changed_call_count_;
+  bool last_zoom_in_;
+
+  DISALLOW_COPY_AND_ASSIGN(ContentsZoomChangedDelegate);
+};
+
+// Tests that some mouseehweel events get turned into browser zoom requests.
+TEST_F(WebContentsImplTest, HandleWheelEvent) {
+  using blink::WebInputEvent;
+
+  scoped_ptr<ContentsZoomChangedDelegate> delegate(
+      new ContentsZoomChangedDelegate());
+  contents()->SetDelegate(delegate.get());
+
+  int modifiers = 0;
+  float dy = 1;
+  // Verify that normal mouse wheel events do nothing to change the zoom level.
+  blink::WebMouseWheelEvent event =
+      SyntheticWebMouseWheelEventBuilder::Build(0, dy, modifiers, false);
+  EXPECT_FALSE(contents()->HandleWheelEvent(event));
+  EXPECT_EQ(0, delegate->GetAndResetContentsZoomChangedCallCount());
+
+  modifiers = WebInputEvent::ShiftKey | WebInputEvent::AltKey;
+  event = SyntheticWebMouseWheelEventBuilder::Build(0, dy, modifiers, false);
+  EXPECT_FALSE(contents()->HandleWheelEvent(event));
+  EXPECT_EQ(0, delegate->GetAndResetContentsZoomChangedCallCount());
+
+  // But whenever the ctrl modifier is applied, they can increase/decrease zoom.
+  // Except on MacOS where we never want to adjust zoom with mousewheel.
+  modifiers = WebInputEvent::ControlKey;
+  event = SyntheticWebMouseWheelEventBuilder::Build(0, dy, modifiers, false);
+  bool handled = contents()->HandleWheelEvent(event);
+#if defined(OS_MACOSX)
+  EXPECT_FALSE(handled);
+  EXPECT_EQ(0, delegate->GetAndResetContentsZoomChangedCallCount());
+#else
+  EXPECT_TRUE(handled);
+  EXPECT_EQ(1, delegate->GetAndResetContentsZoomChangedCallCount());
+  EXPECT_TRUE(delegate->last_zoom_in());
+#endif
+
+  modifiers = WebInputEvent::ControlKey | WebInputEvent::ShiftKey |
+      WebInputEvent::AltKey;
+  dy = -5;
+  event = SyntheticWebMouseWheelEventBuilder::Build(2, dy, modifiers, false);
+  handled = contents()->HandleWheelEvent(event);
+#if defined(OS_MACOSX)
+  EXPECT_FALSE(handled);
+  EXPECT_EQ(0, delegate->GetAndResetContentsZoomChangedCallCount());
+#else
+  EXPECT_TRUE(handled);
+  EXPECT_EQ(1, delegate->GetAndResetContentsZoomChangedCallCount());
+  EXPECT_FALSE(delegate->last_zoom_in());
+#endif
+
+  // Unless there is no vertical movement.
+  dy = 0;
+  event = SyntheticWebMouseWheelEventBuilder::Build(2, dy, modifiers, false);
+  EXPECT_FALSE(contents()->HandleWheelEvent(event));
+  EXPECT_EQ(0, delegate->GetAndResetContentsZoomChangedCallCount());
+
+  // Ensure pointers to the delegate aren't kept beyond it's lifetime.
+  contents()->SetDelegate(NULL);
+}
+
 }  // namespace content
diff --git a/content/renderer/input/input_handler_proxy.cc b/content/renderer/input/input_handler_proxy.cc
index 789cb58..95809a8 100644
--- a/content/renderer/input/input_handler_proxy.cc
+++ b/content/renderer/input/input_handler_proxy.cc
@@ -115,6 +115,11 @@ InputHandlerProxy::EventDisposition InputHandlerProxy::HandleInputEvent(
       // thread, so punt it to the main thread. http://crbug.com/236639
       return DID_NOT_HANDLE;
     }
+    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;
+    }
     cc::InputHandler::ScrollStatus scroll_status = input_handler_->ScrollBegin(
         gfx::Point(wheel_event.x, wheel_event.y), cc::InputHandler::Wheel);
     switch (scroll_status) {
diff --git a/content/renderer/input/input_handler_proxy_unittest.cc b/content/renderer/input/input_handler_proxy_unittest.cc
index 4a63128..94d49a6 100644
--- a/content/renderer/input/input_handler_proxy_unittest.cc
+++ b/content/renderer/input/input_handler_proxy_unittest.cc
@@ -190,6 +190,16 @@ TEST_F(InputHandlerProxyTest, MouseWheelByPageMainThread) {
   testing::Mock::VerifyAndClearExpectations(&mock_client_);
 }
 
+TEST_F(InputHandlerProxyTest, MouseWheelWithCtrl) {
+  expected_disposition_ = InputHandlerProxy::DID_NOT_HANDLE;
+  WebMouseWheelEvent wheel;
+  wheel.type = WebInputEvent::MouseWheel;
+  wheel.modifiers = WebInputEvent::ControlKey;
+
+  EXPECT_EQ(expected_disposition_, input_handler_->HandleInputEvent(wheel));
+  testing::Mock::VerifyAndClearExpectations(&mock_client_);
+}
+
 TEST_F(InputHandlerProxyTest, GestureScrollStarted) {
   // We shouldn't send any events to the widget for this gesture.
   expected_disposition_ = InputHandlerProxy::DID_HANDLE;
@@ -468,7 +478,9 @@ TEST_F(InputHandlerProxyTest, GestureFlingAnimatesTouchpad) {
   WebFloatPoint fling_delta = WebFloatPoint(1000, 0);
   WebPoint fling_point = WebPoint(7, 13);
   WebPoint fling_global_point = WebPoint(17, 23);
-  int modifiers = 7;
+  // Note that for trackpad, wheel events with the Control modifier are
+  // special (reserved for zoom), so don't set that here.
+  int modifiers = WebInputEvent::ShiftKey | WebInputEvent::AltKey;
   gesture_.data.flingStart.velocityX = fling_delta.x;
   gesture_.data.flingStart.velocityY = fling_delta.y;
   gesture_.sourceDevice = WebGestureEvent::Touchpad;
@@ -576,7 +588,9 @@ TEST_F(InputHandlerProxyTest, GestureFlingTransferResetsTouchpad) {
   WebFloatPoint fling_delta = WebFloatPoint(1000, 0);
   WebPoint fling_point = WebPoint(7, 13);
   WebPoint fling_global_point = WebPoint(17, 23);
-  int modifiers = 1;
+  // Note that for trackpad, wheel events with the Control modifier are
+  // special (reserved for zoom), so don't set that here.
+  int modifiers = WebInputEvent::ShiftKey | WebInputEvent::AltKey;
   gesture_.data.flingStart.velocityX = fling_delta.x;
   gesture_.data.flingStart.velocityY = fling_delta.y;
   gesture_.sourceDevice = WebGestureEvent::Touchpad;
@@ -682,7 +696,7 @@ TEST_F(InputHandlerProxyTest, GestureFlingTransferResetsTouchpad) {
   fling_delta = WebFloatPoint(0, -1000);
   fling_point = WebPoint(95, 87);
   fling_global_point = WebPoint(32, 71);
-  modifiers = 2;
+  modifiers = WebInputEvent::AltKey;
   gesture_.data.flingStart.velocityX = fling_delta.x;
   gesture_.data.flingStart.velocityY = fling_delta.y;
   gesture_.sourceDevice = WebGestureEvent::Touchpad;
@@ -861,7 +875,8 @@ TEST_F(InputHandlerProxyTest, GestureFlingAnimatesTouchscreen) {
   WebFloatPoint fling_delta = WebFloatPoint(1000, 0);
   WebPoint fling_point = WebPoint(7, 13);
   WebPoint fling_global_point = WebPoint(17, 23);
-  int modifiers = 7;
+  // Note that for touchscreen the control modifier is not special.
+  int modifiers = WebInputEvent::ControlKey;
   gesture_.data.flingStart.velocityX = fling_delta.x;
   gesture_.data.flingStart.velocityY = fling_delta.y;
   gesture_.sourceDevice = WebGestureEvent::Touchscreen;
@@ -924,7 +939,7 @@ TEST_F(InputHandlerProxyTest, GestureFlingWithValidTimestamp) {
   WebFloatPoint fling_delta = WebFloatPoint(1000, 0);
   WebPoint fling_point = WebPoint(7, 13);
   WebPoint fling_global_point = WebPoint(17, 23);
-  int modifiers = 7;
+  int modifiers = WebInputEvent::ControlKey;
   gesture_.timeStampSeconds = startTimeOffset.InSecondsF();
   gesture_.data.flingStart.velocityX = fling_delta.x;
   gesture_.data.flingStart.velocityY = fling_delta.y;
@@ -985,7 +1000,7 @@ TEST_F(InputHandlerProxyTest,
   WebFloatPoint fling_delta = WebFloatPoint(1000, 0);
   WebPoint fling_point = WebPoint(7, 13);
   WebPoint fling_global_point = WebPoint(17, 23);
-  int modifiers = 7;
+  int modifiers = WebInputEvent::ControlKey | WebInputEvent::AltKey;
   gesture_.data.flingStart.velocityX = fling_delta.x;
   gesture_.data.flingStart.velocityY = fling_delta.y;
   gesture_.sourceDevice = WebGestureEvent::Touchscreen;