summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorrbyers@chromium.org <rbyers@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-03-30 17:37:29 +0000
committerrbyers@chromium.org <rbyers@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-03-30 17:37:29 +0000
commitbab38afe71a47e18c441c5789a2d3eb2586cbce2 (patch)
tree80ceb1f12b0d5a5574beafda499c3546a2d69775
parent3c81a89a854294cc77f25813dc6fc6498546c172 (diff)
downloadchromium_src-bab38afe71a47e18c441c5789a2d3eb2586cbce2.zip
chromium_src-bab38afe71a47e18c441c5789a2d3eb2586cbce2.tar.gz
chromium_src-bab38afe71a47e18c441c5789a2d3eb2586cbce2.tar.bz2
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
-rw-r--r--chrome/browser/ui/cocoa/browser_window_controller.h5
-rw-r--r--chrome/browser/ui/cocoa/browser_window_controller.mm35
-rw-r--r--content/browser/renderer_host/input/gesture_event_queue.h2
-rw-r--r--content/browser/renderer_host/input/input_router_impl.cc13
-rw-r--r--content/browser/renderer_host/input/input_router_impl_unittest.cc17
-rw-r--r--content/browser/renderer_host/render_widget_host_delegate.cc5
-rw-r--r--content/browser/renderer_host/render_widget_host_delegate.h4
-rw-r--r--content/browser/renderer_host/render_widget_host_impl.cc5
-rw-r--r--content/browser/renderer_host/render_widget_host_view_mac.mm15
-rw-r--r--content/browser/web_contents/web_contents_impl.cc40
-rw-r--r--content/browser/web_contents/web_contents_impl.h7
-rw-r--r--content/browser/web_contents/web_contents_impl_unittest.cc50
12 files changed, 158 insertions, 40 deletions
diff --git a/chrome/browser/ui/cocoa/browser_window_controller.h b/chrome/browser/ui/cocoa/browser_window_controller.h
index a89f849..d16649f 100644
--- a/chrome/browser/ui/cocoa/browser_window_controller.h
+++ b/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
diff --git a/chrome/browser/ui/cocoa/browser_window_controller.mm b/chrome/browser/ui/cocoa/browser_window_controller.mm
index 65a7ac6..b7de006 100644
--- a/chrome/browser/ui/cocoa/browser_window_controller.mm
+++ b/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];
diff --git a/content/browser/renderer_host/input/gesture_event_queue.h b/content/browser/renderer_host/input/gesture_event_queue.h
index 8cbb253..4c34070 100644
--- a/content/browser/renderer_host/input/gesture_event_queue.h
+++ b/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
diff --git a/content/browser/renderer_host/input/input_router_impl.cc b/content/browser/renderer_host/input/input_router_impl.cc
index b7ee95d..1028668 100644
--- a/content/browser/renderer_host/input/input_router_impl.cc
+++ b/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;
diff --git a/content/browser/renderer_host/input/input_router_impl_unittest.cc b/content/browser/renderer_host/input/input_router_impl_unittest.cc
index 37fe462..9f6760b 100644
--- a/content/browser/renderer_host/input/input_router_impl_unittest.cc
+++ b/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
diff --git a/content/browser/renderer_host/render_widget_host_delegate.cc b/content/browser/renderer_host/render_widget_host_delegate.cc
index a3b66e2..23687d0 100644
--- a/content/browser/renderer_host/render_widget_host_delegate.cc
+++ b/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() {
diff --git a/content/browser/renderer_host/render_widget_host_delegate.h b/content/browser/renderer_host/render_widget_host_delegate.h
index e31975c..983f282 100644
--- a/content/browser/renderer_host/render_widget_host_delegate.h
+++ b/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) {}
diff --git a/content/browser/renderer_host/render_widget_host_impl.cc b/content/browser/renderer_host/render_widget_host_impl.cc
index 576f06a..b08dff8 100644
--- a/content/browser/renderer_host/render_widget_host_impl.cc
+++ b/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);
}
diff --git a/content/browser/renderer_host/render_widget_host_view_mac.mm b/content/browser/renderer_host/render_widget_host_view_mac.mm
index 776a426..0e913b5 100644
--- a/content/browser/renderer_host/render_widget_host_view_mac.mm
+++ b/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];
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index 79cfa06..005f768 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/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_;
diff --git a/content/browser/web_contents/web_contents_impl.h b/content/browser/web_contents/web_contents_impl.h
index df48e6d..c1587c1 100644
--- a/content/browser/web_contents/web_contents_impl.h
+++ b/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_;
diff --git a/content/browser/web_contents/web_contents_impl_unittest.cc b/content/browser/web_contents/web_contents_impl_unittest.cc
index 0c8f70c..a6a66bb 100644
--- a/content/browser/web_contents/web_contents_impl_unittest.cc
+++ b/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