From 8c3603d7aa9361b89caf434fc5220e2ef12b0c4a Mon Sep 17 00:00:00 2001 From: "erikchen@chromium.org" Date: Sat, 4 Jan 2014 04:15:27 +0000 Subject: [PATCH] mac: Fix a crash in the history swiper for magic mouse momentum events. The 'trackSwipeWithEvent:' api throws an exception if the event passed in is a momentum scroll event. We started using momentum scroll events in our logic for magic trackpads, since we're working directly with hardware touches. We failed to filter out momentum events when falling back to the 'trackSwipeWithEvent:' api for magic mouse events. BUG=331506 Review URL: https://codereview.chromium.org/124313002 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@243003 0039d316-1c4b-4281-b951-d872f2087c98 --- ...me_render_widget_host_view_mac_history_swiper.h | 5 ++ ...e_render_widget_host_view_mac_history_swiper.mm | 18 ++++++- ...idget_host_view_mac_history_swiper_unit_test.mm | 55 +++++++++++++++++++++- 3 files changed, 75 insertions(+), 3 deletions(-) diff --git a/chrome/browser/renderer_host/chrome_render_widget_host_view_mac_history_swiper.h b/chrome/browser/renderer_host/chrome_render_widget_host_view_mac_history_swiper.h index e1888a8f760e..1ec39054e024 100644 --- a/chrome/browser/renderer_host/chrome_render_widget_host_view_mac_history_swiper.h +++ b/chrome/browser/renderer_host/chrome_render_widget_host_view_mac_history_swiper.h @@ -115,4 +115,9 @@ enum NavigationDirection { @end +// Exposed only for unit testing, do not call directly. +@interface HistorySwiper (PrivateExposedForTesting) ++ (void)resetMagicMouseState; +@end + #endif // CHROME_BROWSER_RENDERER_HOST_CHROME_RENDER_WIDGET_HOST_VIEW_MAC_HISTORY_SWIPER_ diff --git a/chrome/browser/renderer_host/chrome_render_widget_host_view_mac_history_swiper.mm b/chrome/browser/renderer_host/chrome_render_widget_host_view_mac_history_swiper.mm index a3bbf3a8b3d7..f37cc6f74fc9 100644 --- a/chrome/browser/renderer_host/chrome_render_widget_host_view_mac_history_swiper.mm +++ b/chrome/browser/renderer_host/chrome_render_widget_host_view_mac_history_swiper.mm @@ -295,6 +295,10 @@ static BOOL forceMagicMouse = NO; // We use an entirely different set of logic for magic mouse swipe events, // since we do not get NSTouch callbacks. - (BOOL)maybeHandleMagicMouseHistorySwiping:(NSEvent*)theEvent { + // The 'trackSwipeEventWithOptions:' api doesn't handle momentum events. + if ([theEvent phase] == NSEventPhaseNone) + return NO; + mouseScrollDelta_.width += [theEvent scrollingDeltaX]; mouseScrollDelta_.height += [theEvent scrollingDeltaY]; @@ -319,6 +323,12 @@ static BOOL forceMagicMouse = NO; return NO; } + [self initiateMagicMouseHistorySwipe:isRightScroll event:theEvent]; + return YES; +} + +- (void)initiateMagicMouseHistorySwipe:(BOOL)isRightScroll + event:(NSEvent*)event { // Released by the tracking handler once the gesture is complete. HistoryOverlayController* historyOverlay = [[HistoryOverlayController alloc] initForMode:isRightScroll ? kHistoryOverlayModeForward @@ -348,7 +358,7 @@ static BOOL forceMagicMouse = NO; // direction after the initial swipe doesn't cause the shield to move // in the wrong direction. forceMagicMouse = YES; - [theEvent trackSwipeEventWithOptions:NSEventSwipeTrackingLockDirection + [event trackSwipeEventWithOptions:NSEventSwipeTrackingLockDirection dampenAmountThresholdMin:-1 max:1 usingHandler:^(CGFloat gestureAmount, @@ -386,7 +396,6 @@ static BOOL forceMagicMouse = NO; if (isComplete) [historyOverlay release]; }]; - return YES; } // Checks if |theEvent| should trigger history swiping, and if so, does @@ -505,5 +514,10 @@ static BOOL forceMagicMouse = NO; [self beginHistorySwipeInDirection:direction event:theEvent]; return YES; } +@end +@implementation HistorySwiper (PrivateExposedForTesting) ++ (void)resetMagicMouseState { + forceMagicMouse = NO; +} @end diff --git a/chrome/browser/renderer_host/chrome_render_widget_host_view_mac_history_swiper_unit_test.mm b/chrome/browser/renderer_host/chrome_render_widget_host_view_mac_history_swiper_unit_test.mm index 8b61251e4ca8..58fac22e45c1 100644 --- a/chrome/browser/renderer_host/chrome_render_widget_host_view_mac_history_swiper_unit_test.mm +++ b/chrome/browser/renderer_host/chrome_render_widget_host_view_mac_history_swiper_unit_test.mm @@ -22,6 +22,8 @@ (history_swiper::NavigationDirection)goForward event:(NSEvent*)event; - (void)navigateBrowserInDirection:(history_swiper::NavigationDirection)forward; +- (void)initiateMagicMouseHistorySwipe:(BOOL)isRightScroll + event:(NSEvent*)event; @end class MacHistorySwiperTest : public CocoaTest { @@ -29,6 +31,8 @@ class MacHistorySwiperTest : public CocoaTest { virtual void SetUp() OVERRIDE { CocoaTest::SetUp(); + [HistorySwiper resetMagicMouseState]; + view_ = [[NSView alloc] init]; id mockDelegate = [OCMockObject mockForProtocol:@protocol(HistorySwiperDelegate)]; @@ -69,12 +73,21 @@ class MacHistorySwiperTest : public CocoaTest { [[[mockHistorySwiper stub] andDo:^(NSInvocation* invocation) { navigated_left_ = true; }] navigateBrowserInDirection:history_swiper::kBackwards]; + + [[[mockHistorySwiper stub] andDo:^(NSInvocation* invocation) { + magic_mouse_history_swipe_ = true; + }] initiateMagicMouseHistorySwipe:YES event:[OCMArg any]]; + [[[mockHistorySwiper stub] andDo:^(NSInvocation* invocation) { + magic_mouse_history_swipe_ = true; + }] initiateMagicMouseHistorySwipe:NO event:[OCMArg any]]; + historySwiper_ = [mockHistorySwiper retain]; begin_count_ = 0; end_count_ = 0; navigated_right_ = false; navigated_left_ = false; + magic_mouse_history_swipe_ = false; } virtual void TearDown() OVERRIDE { @@ -95,6 +108,7 @@ class MacHistorySwiperTest : public CocoaTest { int end_count_; bool navigated_right_; bool navigated_left_; + bool magic_mouse_history_swipe_; }; NSPoint makePoint(CGFloat x, CGFloat y) { @@ -117,15 +131,27 @@ id mockEventWithPoint(NSPoint point, NSEventType type) { return mockEvent; } -id scrollWheelEventWithPhase(NSEventPhase phase, NSEventPhase momentumPhase) { +id scrollWheelEventWithPhase(NSEventPhase phase, + NSEventPhase momentumPhase, + CGFloat scrollingDeltaX, + CGFloat scrollingDeltaY) { // The point isn't used, so we pass in bogus data. id event = mockEventWithPoint(makePoint(0,0), NSScrollWheel); [(NSEvent*)[[event stub] andReturnValue:OCMOCK_VALUE(phase)] phase]; [(NSEvent*) [[event stub] andReturnValue:OCMOCK_VALUE(momentumPhase)] momentumPhase]; + [(NSEvent*)[[event stub] + andReturnValue:OCMOCK_VALUE(scrollingDeltaX)] scrollingDeltaX]; + [(NSEvent*)[[event stub] + andReturnValue:OCMOCK_VALUE(scrollingDeltaY)] scrollingDeltaY]; return event; } +id scrollWheelEventWithPhase(NSEventPhase phase, + NSEventPhase momentumPhase) { + return scrollWheelEventWithPhase(phase, momentumPhase, 0, 0); +} + id scrollWheelEventWithPhase(NSEventPhase phase) { return scrollWheelEventWithPhase(phase, NSEventPhaseNone); } @@ -313,6 +339,33 @@ TEST_F(MacHistorySwiperTest, MomentumSwipeLeft) { EXPECT_TRUE(navigated_left_); } +// Momentum scroll events for magic mouse should not attempt to trigger the +// `trackSwipeEventWithOptions:` api, as that throws an exception. +TEST_F(MacHistorySwiperTest, MagicMouseMomentumSwipe) { + // These tests require 10.7+ APIs. + if (![NSEvent + respondsToSelector:@selector(isSwipeTrackingFromScrollEventsEnabled)]) + return; + + // Magic mouse events don't generate 'touches*' callbacks. + NSEvent* event = mockEventWithPoint(makePoint(0.5, 0.5), NSEventTypeGesture); + [historySwiper_ beginGestureWithEvent:event]; + NSEvent* scrollEvent = scrollWheelEventWithPhase(NSEventPhaseBegan); + [historySwiper_ handleEvent:scrollEvent]; + + // Callbacks from blink to set the relevant state for history swiping. + [historySwiper_ gotUnhandledWheelEvent]; + [historySwiper_ scrollOffsetPinnedToLeft:YES toRight:YES]; + [historySwiper_ setHasHorizontalScrollbar:NO]; + + // Send a momentum move gesture. + scrollEvent = + scrollWheelEventWithPhase(NSEventPhaseNone, NSEventPhaseChanged, 5.0, 0); + [historySwiper_ handleEvent:scrollEvent]; + + EXPECT_FALSE(magic_mouse_history_swipe_); +} + // User starts a swipe but doesn't move. TEST_F(MacHistorySwiperTest, NoSwipe) { // These tests require 10.7+ APIs. -- 2.11.4.GIT