From 908e43770ab94c7d5bcc97f0cbbea9e676a9ba6c Mon Sep 17 00:00:00 2001 From: ccameron Date: Wed, 29 Apr 2015 16:40:03 -0700 Subject: [PATCH] Mac: Make pinch less sensitive Require a cumulative 40% zoom before forwarding a pinch to the renderer. For the discrete browser zoom pre-M42, we required 60% (this code was removed in https://codereview.chromium.org/899283004). If a zoom gesture (which crossed the 40% line) has occurred in the last second, do not require the 40% threshold (since this is likely an effort to re-zoom in). Of note is that these percentages are additive (the sum of the magnification factors), not multiplicative (the product of 1 plus the magnification factors). This is to be consistent with the previous zoom threshold code. Also limit the maximum pinch-zoom level to 3x, since the full 4x is rarely useful. BUG=478981 Review URL: https://codereview.chromium.org/1110253002 Cr-Commit-Position: refs/heads/master@{#327619} --- content/DEPS | 1 + .../renderer_host/render_widget_host_view_mac.h | 8 ++ .../renderer_host/render_widget_host_view_mac.mm | 17 +++ .../render_widget_host_view_mac_unittest.mm | 138 +++++++++++++++++++++ content/content_tests.gypi | 5 + content/public/common/web_preferences.cc | 3 + 6 files changed, 172 insertions(+) diff --git a/content/DEPS b/content/DEPS index fb2bf9d417d6..91328cc3bfe1 100644 --- a/content/DEPS +++ b/content/DEPS @@ -65,6 +65,7 @@ include_rules = [ "+third_party/mojo/src/mojo/public", "+third_party/mozilla", "+third_party/npapi/bindings", + "+third_party/ocmock", "+third_party/re2", "+third_party/skia", "+third_party/sqlite", diff --git a/content/browser/renderer_host/render_widget_host_view_mac.h b/content/browser/renderer_host/render_widget_host_view_mac.h index 2465dd12ff02..499026922fd9 100644 --- a/content/browser/renderer_host/render_widget_host_view_mac.h +++ b/content/browser/renderer_host/render_widget_host_view_mac.h @@ -158,6 +158,14 @@ class Layer; // create a specific gesture begin event later. scoped_ptr gestureBeginEvent_; + // To avoid accidental pinches, require that a certain zoom threshold be + // reached before forwarding it to the browser. Use |unusedPinchAmount_| to + // hold this value. If the user reaches this value, don't re-require the + // threshold be reached until a certain time has passed since the last + // zoom. Store the time of the last zoom in |lastUsedPinchEventTimestamp_|. + float unusedPinchAmount_; + NSTimeInterval lastUsedPinchEventTimestamp_; + // This is set if a GesturePinchBegin event has been sent in the lifetime of // |gestureBeginEvent_|. If set, a GesturePinchEnd will be sent when the // gesture ends. 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 021f42146e40..481213f86999 100644 --- a/content/browser/renderer_host/render_widget_host_view_mac.mm +++ b/content/browser/renderer_host/render_widget_host_view_mac.mm @@ -1752,6 +1752,7 @@ void RenderWidgetHostViewMac::OnDisplayMetricsChanged( canBeKeyView_ = YES; opaque_ = YES; focusedPluginIdentifier_ = -1; + lastUsedPinchEventTimestamp_ = 0; // OpenGL support: if ([self respondsToSelector: @@ -2283,6 +2284,7 @@ void RenderWidgetHostViewMac::OnDisplayMetricsChanged( [responderDelegate_ beginGestureWithEvent:event]; gestureBeginEvent_.reset( new WebGestureEvent(WebInputEventFactory::gestureEvent(event, self))); + unusedPinchAmount_ = 0; } - (void)endGestureWithEvent:(NSEvent*)event { @@ -2297,6 +2299,7 @@ void RenderWidgetHostViewMac::OnDisplayMetricsChanged( endEvent.type = WebInputEvent::GesturePinchEnd; renderWidgetHostView_->render_widget_host_->ForwardGestureEvent(endEvent); gestureBeginPinchSent_ = NO; + lastUsedPinchEventTimestamp_ = [event timestamp]; } } @@ -2411,6 +2414,20 @@ void RenderWidgetHostViewMac::OnDisplayMetricsChanged( // Send a GesturePinchBegin event if none has been sent yet. if (!gestureBeginPinchSent_) { + // If less than 1 second has passed since an intentional pinch zoom + // was done, don't threshold zooms, because subsequent zooms are likely + // intentional. + const NSTimeInterval kSecondsUntilZoomThresholdReEnabled = 1; + if ([event timestamp] - lastUsedPinchEventTimestamp_ > + kSecondsUntilZoomThresholdReEnabled) { + // Require that a 40% zoom be hit before actually zooming the page, + // to avoid accidental zooms. + // http://crbug.com/478981 + unusedPinchAmount_ += [event magnification]; + if (unusedPinchAmount_ > -0.4 && unusedPinchAmount_ < 0.4) + return; + } + WebGestureEvent beginEvent(*gestureBeginEvent_); beginEvent.type = WebInputEvent::GesturePinchBegin; renderWidgetHostView_->render_widget_host_->ForwardGestureEvent(beginEvent); diff --git a/content/browser/renderer_host/render_widget_host_view_mac_unittest.mm b/content/browser/renderer_host/render_widget_host_view_mac_unittest.mm index dc48cc1e99e6..6fb79d187f3e 100644 --- a/content/browser/renderer_host/render_widget_host_view_mac_unittest.mm +++ b/content/browser/renderer_host/render_widget_host_view_mac_unittest.mm @@ -4,6 +4,8 @@ #include "content/browser/renderer_host/render_widget_host_view_mac.h" +#include + #include "base/mac/mac_util.h" #include "base/mac/scoped_nsautorelease_pool.h" #include "base/mac/sdk_forward_declarations.h" @@ -24,6 +26,8 @@ #include "content/test/test_render_view_host.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" +#import "third_party/ocmock/OCMock/OCMock.h" +#import "third_party/ocmock/ocmock_extensions.h" #include "ui/events/test/cocoa_test_event_utils.h" #import "ui/gfx/test/ui_cocoa_test_helper.h" @@ -88,6 +92,26 @@ namespace content { namespace { +id MockGestureEvent( + NSEventType type, NSTimeInterval timestamp, double magnification) { + id event = [OCMockObject mockForClass:[NSEvent class]]; + NSPoint locationInWindow = NSMakePoint(0, 0); + CGFloat deltaX = 0; + CGFloat deltaY = 0; + NSUInteger modifierFlags = 0; + [(NSEvent*)[[event stub] andReturnValue:OCMOCK_VALUE(type)] type]; + [(NSEvent*)[[event stub] + andReturnValue:OCMOCK_VALUE(locationInWindow)] locationInWindow]; + [(NSEvent*)[[event stub] andReturnValue:OCMOCK_VALUE(deltaX)] deltaX]; + [(NSEvent*)[[event stub] andReturnValue:OCMOCK_VALUE(deltaY)] deltaY]; + [(NSEvent*)[[event stub] andReturnValue:OCMOCK_VALUE(timestamp)] timestamp]; + [(NSEvent*)[[event stub] + andReturnValue:OCMOCK_VALUE(modifierFlags)] modifierFlags]; + [(NSEvent*)[[event stub] + andReturnValue:OCMOCK_VALUE(magnification)] magnification]; + return event; +} + class MockRenderWidgetHostDelegate : public RenderWidgetHostDelegate { public: MockRenderWidgetHostDelegate() {} @@ -842,4 +866,118 @@ TEST_F(RenderWidgetHostViewMacTest, Background) { host->Shutdown(); } +TEST_F(RenderWidgetHostViewMacTest, PinchThresholding) { + // This tests Lion+ functionality, so don't run the test pre-Lion. + if (!base::mac::IsOSLionOrLater()) + return; + + // Initialize the view associated with a MockRenderWidgetHostImpl, rather than + // the MockRenderProcessHost that is set up by the test harness which mocks + // out |OnMessageReceived()|. + TestBrowserContext browser_context; + MockRenderProcessHost* process_host = + new MockRenderProcessHost(&browser_context); + MockRenderWidgetHostDelegate delegate; + MockRenderWidgetHostImpl* host = new MockRenderWidgetHostImpl( + &delegate, process_host, MSG_ROUTING_NONE); + RenderWidgetHostViewMac* view = new RenderWidgetHostViewMac(host, false); + + // We'll use this IPC message to ack events. + InputHostMsg_HandleInputEvent_ACK_Params ack; + ack.type = blink::WebInputEvent::GesturePinchUpdate; + ack.state = INPUT_EVENT_ACK_STATE_CONSUMED; + scoped_ptr response( + new InputHostMsg_HandleInputEvent_ACK(0, ack)); + + // Do a gesture that crosses the threshold. + { + NSEvent* pinchBeginEvent = + MockGestureEvent(NSEventTypeBeginGesture, 100.1, 0); + NSEvent* pinchUpdateEvents[3] = { + MockGestureEvent(NSEventTypeMagnify, 100.2, 0.25), + MockGestureEvent(NSEventTypeMagnify, 100.3, 0.25), + MockGestureEvent(NSEventTypeMagnify, 100.4, 0.25), + }; + NSEvent* pinchEndEvent = + MockGestureEvent(NSEventTypeEndGesture, 100.5, 0); + + // No messages are sent for the pinch begin and the first update event. + [view->cocoa_view() beginGestureWithEvent:pinchBeginEvent]; + [view->cocoa_view() magnifyWithEvent:pinchUpdateEvents[0]]; + ASSERT_EQ(0U, process_host->sink().message_count()); + + // The second update event crosses the threshold of 0.4, and so a begin + // and update are sent. + [view->cocoa_view() magnifyWithEvent:pinchUpdateEvents[1]]; + ASSERT_EQ(2U, process_host->sink().message_count()); + host->OnMessageReceived(*response); + + // The third update only causes one event to be sent. + [view->cocoa_view() magnifyWithEvent:pinchUpdateEvents[2]]; + ASSERT_EQ(3U, process_host->sink().message_count()); + host->OnMessageReceived(*response); + + // As does the end. + [view->cocoa_view() endGestureWithEvent:pinchEndEvent]; + ASSERT_EQ(4U, process_host->sink().message_count()); + + process_host->sink().ClearMessages(); + } + + // Do a gesture that doesn't cross the threshold, but happens within 1 second, + // so it should be sent to the renderer. + { + NSEvent* pinchBeginEvent = + MockGestureEvent(NSEventTypeBeginGesture, 101.0, 0); + NSEvent* pinchUpdateEvent = + MockGestureEvent(NSEventTypeMagnify, 101.1, 0.25); + NSEvent* pinchEndEvent = + MockGestureEvent(NSEventTypeEndGesture, 101.2, 0); + + // No message comes for the begin event. + [view->cocoa_view() beginGestureWithEvent:pinchBeginEvent]; + ASSERT_EQ(0U, process_host->sink().message_count()); + + // Two messages come for the first update event. + [view->cocoa_view() magnifyWithEvent:pinchUpdateEvent]; + ASSERT_EQ(2U, process_host->sink().message_count()); + host->OnMessageReceived(*response); + + // The end event sends one message. + [view->cocoa_view() endGestureWithEvent:pinchEndEvent]; + ASSERT_EQ(3U, process_host->sink().message_count()); + + process_host->sink().ClearMessages(); + } + + // Do a gesture that doesn't cross the threshold and happens more than one + // second later. + { + NSEvent* pinchBeginEvent = + MockGestureEvent(NSEventTypeBeginGesture, 103.0, 0); + NSEvent* pinchUpdateEvent = + MockGestureEvent(NSEventTypeMagnify, 103.1, 0.25); + NSEvent* pinchEndEvent = + MockGestureEvent(NSEventTypeEndGesture, 103.2, 0); + + // No message comes for the begin event. + [view->cocoa_view() beginGestureWithEvent:pinchBeginEvent]; + ASSERT_EQ(0U, process_host->sink().message_count()); + + // Two messages come for the first update event. + [view->cocoa_view() magnifyWithEvent:pinchUpdateEvent]; + ASSERT_EQ(0U, process_host->sink().message_count()); + + // As does the end. + [view->cocoa_view() endGestureWithEvent:pinchEndEvent]; + ASSERT_EQ(0U, process_host->sink().message_count()); + + process_host->sink().ClearMessages(); + } + + // Clean up. + host->Shutdown(); +} + + } // namespace content diff --git a/content/content_tests.gypi b/content/content_tests.gypi index 1e2390a026f5..a9560cf99ef4 100644 --- a/content/content_tests.gypi +++ b/content/content_tests.gypi @@ -1039,6 +1039,11 @@ 'browser/file_descriptor_info_impl_unittest.cc', ], }], + ['OS == "mac"', { + 'dependencies': [ + '../third_party/ocmock/ocmock.gyp:ocmock', + ], + }], ['enable_plugins==1', { 'sources': [ '<@(content_unittests_plugins_sources)' ], }], diff --git a/content/public/common/web_preferences.cc b/content/public/common/web_preferences.cc index b61e1dc83906..039a5ff0e4f9 100644 --- a/content/public/common/web_preferences.cc +++ b/content/public/common/web_preferences.cc @@ -218,6 +218,9 @@ WebPreferences::WebPreferences() #if defined(OS_ANDROID) default_minimum_page_scale_factor(0.25f), default_maximum_page_scale_factor(5.f) +#elif defined(OS_MACOSX) + default_minimum_page_scale_factor(1.f), + default_maximum_page_scale_factor(3.f) #else default_minimum_page_scale_factor(1.f), default_maximum_page_scale_factor(4.f) -- 2.11.4.GIT