Handle mac trackpad zoom via GesturePinch events

Prior to this change, Mac handles pinch gestures directly in the UI layer (BrowserWindowController).  Here we instead have the RenderWidgetHost send GesturePinchUpdate events, and when those events go unhandled WebContentsImpl implements the browser zoom behavior (similar to how it does already for ctrl+mousewheel events on other platforms).

This lays the groundwork in the short term for giving the web page a chance to override the pinch behavior.  Longer term this will enable us to hook into the pinch-zoom codepath used for touchscreen so that we can get a similar effect (instead of just browser-zoom - which probably isn't really what the user wants when pinching).

The only observable behavior change in this CL is that pinching outside the content area (eg. in the tabstrip) no longer does anything.  It's not clear what the pinch origin should be in such a case, and it probably doesn't really make sense to try to handle it (pinching is supposed to zoom whatever is under the cursor).

This adds some basic unit test coverage for the pre-existing (but untested) logic for mapping pinch scales to zoom in/out actions.

Depends on blink CL: https://src.chromium.org/viewvc/blink?view=rev&revision=168482

BUG=289887

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

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

chrome/browser/ui/cocoa/browser_window_controller.h

@@ -108,11 +108,6 @@ class Command;
   // NO on growth.
   BOOL isShrinkingFromZoomed_;
 
-  // The raw accumulated zoom value and the actual zoom increments made for an
-  // an in-progress pinch gesture.
-  CGFloat totalMagnifyGestureAmount_;
-  NSInteger currentZoomStepDelta_;
-
   // The view controller that manages the incognito badge or the multi-profile
   // avatar button. Depending on whether the --new-profile-management flag is
   // used, the multi-profile button can either be the avatar's icon badge or a

chrome/browser/ui/cocoa/browser_window_controller.mm

@@ -1868,41 +1868,6 @@ enum {
   }
 }
 
-// Called repeatedly during a pinch gesture, with incremental change values.
-- (void)magnifyWithEvent:(NSEvent*)event {
-  // The deltaZ difference necessary to trigger a zoom action. Derived from
-  // experimentation to find a value that feels reasonable.
-  const float kZoomStepValue = 0.6;
-
-  // Find the (absolute) thresholds on either side of the current zoom factor,
-  // then convert those to actual numbers to trigger a zoom in or out.
-  // This logic deliberately makes the range around the starting zoom value for
-  // the gesture twice as large as the other ranges (i.e., the notches are at
-  // ..., -3*step, -2*step, -step, step, 2*step, 3*step, ... but not at 0)
-  // so that it's easier to get back to your starting point than it is to
-  // overshoot.
-  float nextStep = (abs(currentZoomStepDelta_) + 1) * kZoomStepValue;
-  float backStep = abs(currentZoomStepDelta_) * kZoomStepValue;
-  float zoomInThreshold = (currentZoomStepDelta_ >= 0) ? nextStep : -backStep;
-  float zoomOutThreshold = (currentZoomStepDelta_ <= 0) ? -nextStep : backStep;
-
-  unsigned int command = 0;
-  totalMagnifyGestureAmount_ += [event magnification];
-  if (totalMagnifyGestureAmount_ > zoomInThreshold) {
-    command = IDC_ZOOM_PLUS;
-  } else if (totalMagnifyGestureAmount_ < zoomOutThreshold) {
-    command = IDC_ZOOM_MINUS;
-  }
-
-  if (command && chrome::IsCommandEnabled(browser_.get(), command)) {
-    currentZoomStepDelta_ += (command == IDC_ZOOM_PLUS) ? 1 : -1;
-    chrome::ExecuteCommandWithDisposition(
-        browser_.get(),
-        command,
-        ui::WindowOpenDispositionFromNSEvent(event));
-  }
-}
-
 // Delegate method called when window is resized.
 - (void)windowDidResize:(NSNotification*)notification {
   [self saveWindowPositionIfNeeded];

content/browser/renderer_host/input/gesture_event_queue.h

@@ -68,6 +68,8 @@ class CONTENT_EXPORT GestureEventQueue {
 
   // Returns |true| if the caller should immediately forward the provided
   // |GestureEventWithLatencyInfo| argument to the renderer.
+  // If this function returns false, then the event may be queued and forwared
+  // at a later point.
   bool ShouldForward(const GestureEventWithLatencyInfo&);
 
   // Indicates that the caller has received an acknowledgement from the renderer

content/browser/renderer_host/input/input_router_impl.cc

@@ -242,6 +242,7 @@ void InputRouterImpl::SendGestureEvent(
     const GestureEventWithLatencyInfo& original_gesture_event) {
   event_stream_validator_.OnEvent(original_gesture_event.event);
   GestureEventWithLatencyInfo gesture_event(original_gesture_event);
+
   if (touch_action_filter_.FilterGestureEvent(&gesture_event.event))
     return;
 
@@ -420,6 +421,18 @@ void InputRouterImpl::FilterAndSendWebInputEvent(
 void InputRouterImpl::OfferToHandlers(const WebInputEvent& input_event,
                                       const ui::LatencyInfo& latency_info,
                                       bool is_keyboard_shortcut) {
+  // Trackpad pinch gestures are not yet handled by the renderer.
+  // TODO(rbyers): Send mousewheel for trackpad pinch - crbug.com/289887.
+  if (input_event.type == WebInputEvent::GesturePinchUpdate &&
+      static_cast<const WebGestureEvent&>(input_event).sourceDevice ==
+          WebGestureEvent::Touchpad) {
+    ProcessInputEventAck(input_event.type,
+                         INPUT_EVENT_ACK_STATE_NOT_CONSUMED,
+                         latency_info,
+                         ACK_SOURCE_NONE);
+    return;
+  }
+
   if (OfferToOverscrollController(input_event, latency_info))
     return;
 

content/browser/renderer_host/input/input_router_impl_unittest.cc

@@ -965,6 +965,8 @@ TEST_F(InputRouterImplTest, GestureShowPressIsInOrder) {
   EXPECT_EQ(1U, ack_handler_->GetAndResetAckCount());
 
   // GesturePinchUpdate waits for an ack.
+  // This also verifies that GesturePinchUpdates for touchscreen are sent
+  // to the renderer (in contrast to the TrackpadPinchUpdate test).
   SimulateGestureEvent(WebInputEvent::GesturePinchUpdate,
                        WebGestureEvent::Touchscreen);
   EXPECT_EQ(1U, GetSentMessageCountAndResetSink());
@@ -1297,4 +1299,19 @@ TEST_F(InputRouterImplTest, DoubleTapGestureDependsOnFirstTap) {
   EXPECT_EQ(0, client_->in_flight_event_count());
 }
 
+// Test that GesturePinchUpdate is handled specially for trackpad
+TEST_F(InputRouterImplTest, TrackpadPinchUpdate) {
+  // For now Trackpad PinchUpdate events are just immediately ACKed
+  // as unconsumed without going to the renderer.
+  // TODO(rbyers): Update for wheel event behavior - crbug.com/289887.
+  // Note that the Touchscreen case is verified as NOT doing this as
+  // part of the ShowPressIsInOrder test.
+  SimulateGestureEvent(WebInputEvent::GesturePinchUpdate,
+                       WebGestureEvent::Touchpad);
+  ASSERT_EQ(0U, GetSentMessageCountAndResetSink());
+  EXPECT_EQ(1U, ack_handler_->GetAndResetAckCount());
+  EXPECT_EQ(INPUT_EVENT_ACK_STATE_NOT_CONSUMED, ack_handler_->ack_state());
+  EXPECT_EQ(0, client_->in_flight_event_count());
+}
+
 }  // namespace content

content/browser/renderer_host/render_widget_host_delegate.cc

@@ -22,6 +22,11 @@ bool RenderWidgetHostDelegate::PreHandleGestureEvent(
   return false;
 }
 
+bool RenderWidgetHostDelegate::HandleGestureEvent(
+    const blink::WebGestureEvent& event) {
+  return false;
+}
+
 #if defined(OS_WIN)
 gfx::NativeViewAccessible
 RenderWidgetHostDelegate::GetParentNativeViewAccessible() {

content/browser/renderer_host/render_widget_host_delegate.h

@@ -52,6 +52,10 @@ class CONTENT_EXPORT RenderWidgetHostDelegate {
   // Returns true if the |event| was handled.
   virtual bool PreHandleGestureEvent(const blink::WebGestureEvent& event);
 
+  // Callback to inform the browser that the renderer did not process the
+  // specified gesture event.  Returns true if the |event| was handled.
+  virtual bool HandleGestureEvent(const blink::WebGestureEvent& event);
+
   // Notifies that screen rects were sent to renderer process.
   virtual void DidSendScreenRects(RenderWidgetHostImpl* rwh) {}
 

content/browser/renderer_host/render_widget_host_impl.cc

@@ -2134,6 +2134,11 @@ void RenderWidgetHostImpl::OnGestureEventAck(
         ui::INPUT_EVENT_LATENCY_TERMINATED_GESTURE_COMPONENT, 0 ,0);
   }
 
+  if (ack_result != INPUT_EVENT_ACK_STATE_CONSUMED) {
+    if (delegate_->HandleGestureEvent(event.event))
+      ack_result = INPUT_EVENT_ACK_STATE_CONSUMED;
+  }
+
   if (view_)
     view_->GestureEventAck(event.event, ack_result);
 }

content/browser/renderer_host/render_widget_host_view_mac.mm

@@ -88,6 +88,7 @@ using blink::WebInputEvent;
 using blink::WebInputEventFactory;
 using blink::WebMouseEvent;
 using blink::WebMouseWheelEvent;
+using blink::WebGestureEvent;
 
 // These are not documented, so use only after checking -respondsToSelector:.
 @interface NSApplication (UndocumentedSpeechMethods)
@@ -2924,6 +2925,20 @@ SkBitmap::Config RenderWidgetHostViewMac::PreferredReadbackFormat() {
   }
 }
 
+// Called repeatedly during a pinch gesture, with incremental change values.
+- (void)magnifyWithEvent:(NSEvent*)event {
+  if (renderWidgetHostView_->render_widget_host_) {
+    // Send a GesturePinchUpdate event.
+    // Note that we don't attempt to bracket these by GesturePinchBegin/End (or
+    // GestureSrollBegin/End) as is done for touchscreen.  Keeping track of when
+    // a pinch is active would take a little more work here, and we don't need
+    // it for anything yet.
+    const WebGestureEvent& webEvent =
+        WebInputEventFactory::gestureEvent(event, self);
+    renderWidgetHostView_->render_widget_host_->ForwardGestureEvent(webEvent);
+  }
+}
+
 - (void)viewWillMoveToWindow:(NSWindow*)newWindow {
   NSWindow* oldWindow = [self window];
 

content/browser/web_contents/web_contents_impl.cc

@@ -332,6 +332,8 @@ WebContentsImpl::WebContentsImpl(
       minimum_zoom_percent_(static_cast<int>(kMinimumZoomFactor * 100)),
       maximum_zoom_percent_(static_cast<int>(kMaximumZoomFactor * 100)),
       temporary_zoom_settings_(false),
+      totalPinchGestureAmount_(0),
+      currentPinchZoomStepDelta_(0),
       color_chooser_identifier_(0),
       render_view_message_source_(NULL),
       fullscreen_widget_routing_id_(MSG_ROUTING_NONE),
@@ -1215,6 +1217,44 @@ bool WebContentsImpl::PreHandleGestureEvent(
   return delegate_ && delegate_->PreHandleGestureEvent(this, event);
 }
 
+bool WebContentsImpl::HandleGestureEvent(
+    const blink::WebGestureEvent& event) {
+  // Some platforms (eg. Mac) send GesturePinch events for trackpad pinch-zoom.
+  // Use them to implement browser zoom, as for HandleWheelEvent above.
+  if (event.type == blink::WebInputEvent::GesturePinchUpdate &&
+      event.sourceDevice == blink::WebGestureEvent::Touchpad) {
+    // The scale difference necessary to trigger a zoom action. Derived from
+    // experimentation to find a value that feels reasonable.
+    const float kZoomStepValue = 0.6f;
+
+    // Find the (absolute) thresholds on either side of the current zoom factor,
+    // then convert those to actual numbers to trigger a zoom in or out.
+    // This logic deliberately makes the range around the starting zoom value
+    // for the gesture twice as large as the other ranges (i.e., the notches are
+    // at ..., -3*step, -2*step, -step, step, 2*step, 3*step, ... but not at 0)
+    // so that it's easier to get back to your starting point than it is to
+    // overshoot.
+    float nextStep = (abs(currentPinchZoomStepDelta_) + 1) * kZoomStepValue;
+    float backStep = abs(currentPinchZoomStepDelta_) * kZoomStepValue;
+    float zoomInThreshold = (currentPinchZoomStepDelta_ >= 0) ? nextStep
+        : -backStep;
+    float zoomOutThreshold = (currentPinchZoomStepDelta_ <= 0) ? -nextStep
+        : backStep;
+
+    totalPinchGestureAmount_ += event.data.pinchUpdate.scale;
+    if (totalPinchGestureAmount_ > zoomInThreshold) {
+      currentPinchZoomStepDelta_++;
+      delegate_->ContentsZoomChange(true);
+    } else if (totalPinchGestureAmount_ < zoomOutThreshold) {
+      currentPinchZoomStepDelta_--;
+      delegate_->ContentsZoomChange(false);
+    }
+    return true;
+  }
+
+  return false;
+}
+
 #if defined(OS_WIN)
 gfx::NativeViewAccessible WebContentsImpl::GetParentNativeViewAccessible() {
   return accessible_parent_;

content/browser/web_contents/web_contents_impl.h

@@ -487,6 +487,8 @@ class CONTENT_EXPORT WebContentsImpl
       const blink::WebMouseWheelEvent& event) OVERRIDE;
   virtual bool PreHandleGestureEvent(
       const blink::WebGestureEvent& event) OVERRIDE;
+  virtual bool HandleGestureEvent(
+      const blink::WebGestureEvent& event) OVERRIDE;
   virtual void DidSendScreenRects(RenderWidgetHostImpl* rwh) OVERRIDE;
 #if defined(OS_WIN)
   virtual gfx::NativeViewAccessible GetParentNativeViewAccessible() OVERRIDE;
@@ -986,6 +988,11 @@ class CONTENT_EXPORT WebContentsImpl
   // remember it.
   bool temporary_zoom_settings_;
 
+  // The raw accumulated zoom value and the actual zoom increments made for an
+  // an in-progress pinch gesture.
+  float totalPinchGestureAmount_;
+  int currentPinchZoomStepDelta_;
+
   // The intrinsic size of the page.
   gfx::Size preferred_size_;
 

content/browser/web_contents/web_contents_impl_unittest.cc

@@ -2430,4 +2430,54 @@ TEST_F(WebContentsImplTest, HandleWheelEvent) {
   contents()->SetDelegate(NULL);
 }
 
+// Tests that trackpad GesturePinchUpdate events get turned into browser zoom.
+TEST_F(WebContentsImplTest, HandleGestureEvent) {
+  using blink::WebGestureEvent;
+  using blink::WebInputEvent;
+
+  scoped_ptr<ContentsZoomChangedDelegate> delegate(
+      new ContentsZoomChangedDelegate());
+  contents()->SetDelegate(delegate.get());
+
+  const float kZoomStepValue = 0.6f;
+  blink::WebGestureEvent event = SyntheticWebGestureEventBuilder::Build(
+      WebInputEvent::GesturePinchUpdate, WebGestureEvent::Touchpad);
+
+  // A pinch less than the step value doesn't change the zoom level.
+  event.data.pinchUpdate.scale = kZoomStepValue * 0.8f;
+  EXPECT_TRUE(contents()->HandleGestureEvent(event));
+  EXPECT_EQ(0, delegate->GetAndResetContentsZoomChangedCallCount());
+
+  // But repeating the event so the combined scale is greater does.
+  EXPECT_TRUE(contents()->HandleGestureEvent(event));
+  EXPECT_EQ(1, delegate->GetAndResetContentsZoomChangedCallCount());
+  EXPECT_TRUE(delegate->last_zoom_in());
+
+  // Pinching back out one step goes back to 100%.
+  event.data.pinchUpdate.scale = -kZoomStepValue;
+  EXPECT_TRUE(contents()->HandleGestureEvent(event));
+  EXPECT_EQ(1, delegate->GetAndResetContentsZoomChangedCallCount());
+  EXPECT_FALSE(delegate->last_zoom_in());
+
+  // Pinching out again doesn't zoom (step is twice as large around 100%).
+  EXPECT_TRUE(contents()->HandleGestureEvent(event));
+  EXPECT_EQ(0, delegate->GetAndResetContentsZoomChangedCallCount());
+
+  // And again now it zooms once per step.
+  EXPECT_TRUE(contents()->HandleGestureEvent(event));
+  EXPECT_EQ(1, delegate->GetAndResetContentsZoomChangedCallCount());
+  EXPECT_FALSE(delegate->last_zoom_in());
+
+  // No other type of gesture event is handled by WebContentsImpl (for example
+  // a touchscreen pinch gesture).
+  event = SyntheticWebGestureEventBuilder::Build(
+      WebInputEvent::GesturePinchUpdate, WebGestureEvent::Touchscreen);
+  event.data.pinchUpdate.scale = kZoomStepValue * 3;
+  EXPECT_FALSE(contents()->HandleGestureEvent(event));
+  EXPECT_EQ(0, delegate->GetAndResetContentsZoomChangedCallCount());
+
+  // Ensure pointers to the delegate aren't kept beyond it's lifetime.
+  contents()->SetDelegate(NULL);
+}
+
 }  // namespace content