Fix page navigation being incorrectly fired on Mac OSX.

chrome/browser/renderer_host/chrome_render_widget_host_view_mac_history_swiper.mm

Index: chrome/browser/renderer_host/chrome_render_widget_host_view_mac_history_swiper.mm
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 b161b84fa8abd0c8bafbab8f12e6d812fe2344fd..abd15fde7a0c72b13e0914d08b56bb6e60ea37c1 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
@@ -118,7 +118,25 @@ BOOL forceMagicMouse = NO;
                          consumed:(BOOL)consumed {
   if (event.phase != NSEventPhaseBegan)
     return;
-  beganEventUnconsumed_ = !consumed;
+  firstScrollUnconsumed_ = !consumed;
+}
+
+- (void)rendererHandledGestureScrollEvent:(const blink::WebGestureEvent&)event
+                                 consumed:(BOOL)consumed {
+  switch (event.type) {
+    case blink::WebInputEvent::GestureScrollBegin:
+      if (event.data.scrollBegin.synthetic || event.data.scrollBegin.inertial)
+        return;
+      nextScrollUpdatesUnconsumed_ = YES;
+      break;
+    case blink::WebInputEvent::GestureScrollUpdate:
+      if (nextScrollUpdatesUnconsumed_)
+        firstScrollUnconsumed_ = !consumed;

[erikchen, 2016/04/01 16:18:58]

I don't understand this logic.

Does there exist a mechanism by which the renderer tells the browser: I will
never consume any more scroll events?

The logic on line 565 should reflect that condition.

[dtapuska, 2016/04/01 17:22:25]

The condition matches the condition logic on line 119 (which uses PhaseBegan).
In that if the first scroll isn't handled by the page then the history arrow is
shown. Being that ScrollBegin isn't a blocking event we don't know if it was
handled or not. But the first ScrollUpdate will tell us.

[erikchen, 2016/04/01 17:57:34]

Based on the description you just gave, firstScrollUnconsumed_ should only be
set on this line.

Perhaps you meant:
If rendererHandledWheelEvent does not consume the event, then the renderer
guarantees that it will not need/consume any future events.

If rendererHandledWheelEvent consumes the first event, but the first
non-synthetic/intertial GestureScrollUpdate event does not consume the event,
then the renderer also guarantees that it will not need/consume any future
events.

If my description is accurate, then renaming nextScrollUpdatesUnconsumed_ to
waitingForFirstGestureScrollUpdate_ would make a lot more sense.

[dtapuska, 2016/04/01 18:08:45]

Ah I think I see the problem with your confusion.

There is a setting --enable-wheel-gestures (enabled by default); which will only
call rendererHandledGestureScrollEvent (and never rendererHandledWheelEvent).
And another setting --disable-wheel-gestures; which will cause it never to call
rendererHandledGestureScrollEvent and always rendererHandledWheelEvent..

See the relevant code and a TODO here:
https://codereview.chromium.org/1848063002/patch/1/10005

+      nextScrollUpdatesUnconsumed_ = NO;
+      break;
+    default:
+      break;
+  }
 }
 
 - (BOOL)canRubberbandLeft:(NSView*)view {
@@ -219,7 +237,8 @@ BOOL forceMagicMouse = NO;
   // Reset state pertaining to previous trackpad gestures.
   gestureStartPointValid_ = NO;
   gestureTotalY_ = 0;
-  beganEventUnconsumed_ = NO;
+  firstScrollUnconsumed_ = NO;
+  nextScrollUpdatesUnconsumed_ = NO;
   recognitionState_ = history_swiper::kPending;
 }
 
@@ -543,7 +562,7 @@ BOOL forceMagicMouse = NO;
 
   // Don't enable history swiping until the renderer has decided to not consume
   // the event with phase NSEventPhaseBegan.
-  if (!beganEventUnconsumed_)
+  if (!firstScrollUnconsumed_)
     return NO;
 
   // Magic mouse and touchpad swipe events are identical except magic mouse