Bug 1016035 - Refactor the code that decides whether we want to start a swipe. r?kats draft
authorMarkus Stange <mstange@themasta.com>
Thu, 23 Jul 2015 13:57:02 -0400
changeset 288163 d13afd01221ac1edc945cef28e2cdc05987c453f
parent 288162 461492165dc7a1d9c48e1b50ba438585ccc7fa49
child 288164 40a6723d8216f90d02cde197cc2dff1e5c2370eb
push id4815
push usermstange@themasta.com
push dateThu, 27 Aug 2015 05:31:12 +0000
reviewerskats
bugs1016035
milestone43.0a1
Bug 1016035 - Refactor the code that decides whether we want to start a swipe. r?kats
widget/cocoa/nsChildView.mm
--- a/widget/cocoa/nsChildView.mm
+++ b/widget/cocoa/nsChildView.mm
@@ -191,16 +191,18 @@ static uint32_t gNumberOfWidgetsNeedingE
 #endif
 
 - (nsIntPoint)convertWindowCoordinates:(NSPoint)aPoint;
 - (APZCTreeManager*)apzctm;
 
 - (BOOL)inactiveWindowAcceptsMouseEvent:(NSEvent*)aEvent;
 - (void)updateWindowDraggableState;
 
+- (bool)shouldConsiderStartingSwipeFromEvent:(NSEvent*)aEvent;
+
 @end
 
 @interface EventThreadRunner : NSObject
 {
   NSThread* mThread;
 }
 - (id)init;
 
@@ -4250,103 +4252,91 @@ NSEvent* gLastDragMouseDownEvent = nil;
     uint32_t allowedDirectionsCopy = aAllowedDirections;
     [self sendSwipeEvent:anEvent
                 withKind:NS_SIMPLE_GESTURE_SWIPE_END
        allowedDirections:&allowedDirectionsCopy
                direction:0
                    delta:0.0];
 }
 
+- (bool)shouldConsiderStartingSwipeFromEvent:(NSEvent*)anEvent
+{
+  if (!nsCocoaFeatures::OnLionOrLater()) {
+    return false;
+  }
+
+  // This method checks whether the AppleEnableSwipeNavigateWithScrolls global
+  // preference is set.  If it isn't, fluid swipe tracking is disabled, and a
+  // horizontal two-finger gesture is always a scroll (even in Safari).  This
+  // preference can't (currently) be set from the Preferences UI -- only using
+  // 'defaults write'.
+  if (![NSEvent isSwipeTrackingFromScrollEventsEnabled]) {
+    return false;
+  }
+
+  // Only initiate horizontal tracking for gestures that have just begun --
+  // otherwise a scroll to one side of the page can have a swipe tacked on
+  // to it.
+  NSEventPhase eventPhase = nsCocoaUtils::EventPhase(anEvent);
+  if ([anEvent type] != NSScrollWheel ||
+      eventPhase != NSEventPhaseBegan ||
+      ![anEvent hasPreciseScrollingDeltas]) {
+    return false;
+  }
+
+  // Only initiate horizontal tracking for events whose horizontal element is
+  // at least eight times larger than its vertical element. This minimizes
+  // performance problems with vertical scrolls (by minimizing the possibility
+  // that they'll be misinterpreted as horizontal swipes), while still
+  // tolerating a small vertical element to a true horizontal swipe.  The number
+  // '8' was arrived at by trial and error.
+  CGFloat deltaX = [anEvent scrollingDeltaX];
+  CGFloat deltaY = [anEvent scrollingDeltaY];
+  return std::abs(deltaX) > std::abs(deltaY) * 8;
+}
+
 // Support fluid swipe tracking on OS X 10.7 and higher. We must be careful
 // to only invoke this support on a two-finger gesture that really
 // is a swipe (and not a scroll) -- in other words, the app is responsible
 // for deciding which is which. But once the decision is made, the OS tracks
 // the swipe until it has finished, and decides whether or not it succeeded.
 // A horizontal swipe has the same functionality as the Back and Forward
 // buttons.
 // This method is partly based on Apple sample code available at
 // developer.apple.com/library/mac/#releasenotes/Cocoa/AppKitOlderNotes.html
 // (under Fluid Swipe Tracking API).
 - (void)maybeTrackScrollEventAsSwipe:(NSEvent *)anEvent
                      scrollOverflowX:(double)anOverflowX
                      scrollOverflowY:(double)anOverflowY
               viewPortIsOverscrolled:(BOOL)aViewPortIsOverscrolled
 {
-  if (!nsCocoaFeatures::OnLionOrLater()) {
-    return;
-  }
-
-  // This method checks whether the AppleEnableSwipeNavigateWithScrolls global
-  // preference is set.  If it isn't, fluid swipe tracking is disabled, and a
-  // horizontal two-finger gesture is always a scroll (even in Safari).  This
-  // preference can't (currently) be set from the Preferences UI -- only using
-  // 'defaults write'.
-  if (![NSEvent isSwipeTrackingFromScrollEventsEnabled]) {
-    return;
-  }
-
   // We should only track scroll events as swipe if the viewport is being
   // overscrolled.
   if (!aViewPortIsOverscrolled) {
     return;
   }
 
-  NSEventPhase eventPhase = nsCocoaUtils::EventPhase(anEvent);
-  // Verify that this is a scroll wheel event with proper phase to be tracked
-  // by the OS.
-  if ([anEvent type] != NSScrollWheel || eventPhase == NSEventPhaseNone) {
-    return;
-  }
-
   // Only initiate tracking if the user has tried to scroll past the edge of
   // the current page (as indicated by 'anOverflowX' or 'anOverflowY' being
   // non-zero). Gecko only sets WidgetMouseScrollEvent.scrollOverflow when it's
   // processing NS_MOUSE_PIXEL_SCROLL events (not NS_MOUSE_SCROLL events).
-  if (anOverflowX == 0.0 && anOverflowY == 0.0) {
-    return;
-  }
-
-  CGFloat deltaX, deltaY;
-  if ([anEvent hasPreciseScrollingDeltas]) {
-    deltaX = [anEvent scrollingDeltaX];
-    deltaY = [anEvent scrollingDeltaY];
-  } else {
+  if (anOverflowX == 0.0) {
     return;
   }
 
-  uint32_t direction = 0;
-
-  // Only initiate horizontal tracking for events whose horizontal element is
-  // at least eight times larger than its vertical element. This minimizes
-  // performance problems with vertical scrolls (by minimizing the possibility
-  // that they'll be misinterpreted as horizontal swipes), while still
-  // tolerating a small vertical element to a true horizontal swipe.  The number
-  // '8' was arrived at by trial and error.
-  if (anOverflowX != 0.0 && deltaX != 0.0 &&
-      std::abs(deltaX) > std::abs(deltaY) * 8) {
-    // Only initiate horizontal tracking for gestures that have just begun --
-    // otherwise a scroll to one side of the page can have a swipe tacked on
-    // to it.
-    if (eventPhase != NSEventPhaseBegan) {
-      return;
-    }
-
-    if (deltaX < 0.0) {
-      direction = (uint32_t)nsIDOMSimpleGestureEvent::DIRECTION_RIGHT;
-    } else {
-      direction = (uint32_t)nsIDOMSimpleGestureEvent::DIRECTION_LEFT;
-    }
-  } else {
-    return;
-  }
-
-  uint32_t allowedDirections = 0;
+  CGFloat deltaX = [anEvent scrollingDeltaX];
+
+  uint32_t direction = (deltaX < 0.0)
+    ? (uint32_t)nsIDOMSimpleGestureEvent::DIRECTION_RIGHT
+    : (uint32_t)nsIDOMSimpleGestureEvent::DIRECTION_LEFT;
+
   // We're ready to start the animation. Tell Gecko about it, and at the same
   // time ask it if it really wants to start an animation for this event.
   // This event also reports back the directions that we can swipe in.
+  uint32_t allowedDirections = 0;
   bool shouldStartSwipe = [self sendSwipeEvent:anEvent
                                       withKind:NS_SIMPLE_GESTURE_SWIPE_START
                              allowedDirections:&allowedDirections
                                      direction:direction
                                          delta:0.0];
 
   if (!shouldStartSwipe) {
     return;
@@ -4944,26 +4934,29 @@ PanGestureTypeForEvent(NSEvent* aEvent)
   }
 
   if (usePreciseDeltas && hasPhaseInformation) {
     PanGestureInput panEvent(PanGestureTypeForEvent(theEvent),
                              eventIntervalTime, eventTimeStamp,
                              position, preciseDelta, modifiers);
     panEvent.mLineOrPageDeltaX = lineOrPageDeltaX;
     panEvent.mLineOrPageDeltaY = lineOrPageDeltaY;
+
+    bool canTriggerSwipe = [self shouldConsiderStartingSwipeFromEvent:theEvent];
+
     widgetWheelEvent = mGeckoChild->DispatchAPZWheelInputEvent(panEvent);
 
     if (!mGeckoChild) {
       return;
     }
 
 #ifdef __LP64__
     // overflowDeltaX and overflowDeltaY tell us when the user has tried to
     // scroll past the edge of a page (in those cases it's non-zero).
-    if ((widgetWheelEvent.deltaX != 0.0 || widgetWheelEvent.deltaY != 0.0)) {
+    if (canTriggerSwipe) {
 
       [self maybeTrackScrollEventAsSwipe:theEvent
                          scrollOverflowX:widgetWheelEvent.overflowDeltaX
                          scrollOverflowY:widgetWheelEvent.overflowDeltaY
                   viewPortIsOverscrolled:widgetWheelEvent.mViewPortIsOverscrolled];
     }
 #endif // #ifdef __LP64__