Make Mac pinch thresholding apply only to zoom (Chromium side)

content/browser/renderer_host/render_widget_host_view_mac.mm

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/renderer_host/render_widget_host_view_mac.h"
 
 #import <objc/runtime.h>
 #include <OpenGL/gl.h>
 #include <QuartzCore/QuartzCore.h>
 
@@ skipping 1738 matching lines @@
   self = [super initWithFrame:NSZeroRect];
   if (self) {
     self.acceptsTouchEvents = YES;
     editCommand_helper_.reset(new RenderWidgetHostViewMacEditCommandHelper);
     editCommand_helper_->AddEditingSelectorsToClass([self class]);
 
     renderWidgetHostView_.reset(r);
     canBeKeyView_ = YES;
     opaque_ = YES;
     focusedPluginIdentifier_ = -1;
-    lastUsedPinchEventTimestamp_ = 0;
+    pinchLastGestureTimestamp_ = 0;
+    pinchHasReachedZoomThreshold_ = false;
 
     // OpenGL support:
     if ([self respondsToSelector:
         @selector(setWantsBestResolutionOpenGLSurface:)]) {
       [self setWantsBestResolutionOpenGLSurface:YES];
     }
     [[NSNotificationCenter defaultCenter]
         addObserver:self
            selector:@selector(didChangeScreenParameters:)
                name:NSApplicationDidChangeScreenParametersNotification
@@ skipping 511 matching lines @@
   if (endWheelMonitor_) {
     [NSEvent removeMonitor:endWheelMonitor_];
     endWheelMonitor_ = nil;
   }
 }
 
 - (void)beginGestureWithEvent:(NSEvent*)event {
   [responderDelegate_ beginGestureWithEvent:event];
   gestureBeginEvent_.reset(
       new WebGestureEvent(WebInputEventFactory::gestureEvent(event, self)));
-  unusedPinchAmount_ = 0;
+
+  // If more than 1 second has passed since the last pinch gesture ended, reset

[aelias_OOO_until_Jul13, 2015/05/06 23:20:24]

I don't like timeouts as a rule.  Why not reset the threshold immediately on
pinchEnd?

[ccameron, 2015/05/07 17:09:05]

If someone is doing several pinches in a row, to zoom to the maximum (or a
higher) level, they will re-start the gesture once their fingers are spread too
far apart. It feels really awkward to have each begin/end block re-require
thresholding.

[aelias_OOO_until_Jul13, 2015/05/07 18:16:16]

I see.  How about checking if page_scale_factor == minimum_scale from the
CompositorFrameMetadata instead of using the timeout?  It seems like you want to
threshold when not currently zoomed in, but if you're already zoomed in then you
want it to be easier to pinch.

+  // the zoom threshold levels.
+  const NSTimeInterval kSecondsUntilZoomThresholdReEnabled = 1;
+  if ([event timestamp] - pinchLastGestureTimestamp_ >
+      kSecondsUntilZoomThresholdReEnabled) {
+    pinchHasReachedZoomThreshold_ = false;
+    pinchUnusedAmount_ = 0;
+  }
 }
 
 - (void)endGestureWithEvent:(NSEvent*)event {
   [responderDelegate_ endGestureWithEvent:event];
   gestureBeginEvent_.reset();
 
   if (!renderWidgetHostView_->render_widget_host_)
     return;
 
   if (gestureBeginPinchSent_) {
     WebGestureEvent endEvent(WebInputEventFactory::gestureEvent(event, self));
     endEvent.type = WebInputEvent::GesturePinchEnd;
     renderWidgetHostView_->render_widget_host_->ForwardGestureEvent(endEvent);
     gestureBeginPinchSent_ = NO;
-    lastUsedPinchEventTimestamp_ = [event timestamp];
+    if (pinchHasReachedZoomThreshold_)
+      pinchLastGestureTimestamp_ = [event timestamp];
   }
 }
 
 - (void)touchesMovedWithEvent:(NSEvent*)event {
   [responderDelegate_ touchesMovedWithEvent:event];
 }
 
 - (void)touchesBeganWithEvent:(NSEvent*)event {
   [responderDelegate_ touchesBeganWithEvent:event];
 }
@@ skipping 92 matching lines @@
 - (void)magnifyWithEvent:(NSEvent*)event {
   if (!renderWidgetHostView_->render_widget_host_)
     return;
 
   // If, due to nesting of multiple gestures (e.g, from multiple touch
   // devices), the beginning of the gesture has been lost, skip the remainder
   // of the gesture.
   if (!gestureBeginEvent_)
     return;
 
+  if (!pinchHasReachedZoomThreshold_) {
+      pinchUnusedAmount_ += [event magnification];

[aelias_OOO_until_Jul13, 2015/05/06 23:20:24]

Please make this multiplicative.  Otherwise this will behave differently
depending on the speed of the gesture and how exactly it was divided into
events.  Making it multiplicative will consistently threshold at the same
overall magnitude.

[ccameron, 2015/05/07 17:09:06]

Done. I had made it additive to match the earlier thresholding math.
Empirically, the 40% additive matches a 50% multiplicative.

+      if (pinchUnusedAmount_ < -0.4 || pinchUnusedAmount_ > 0.4)
+          pinchHasReachedZoomThreshold_ = true;
+  }
+
   // 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);
     gestureBeginPinchSent_ = YES;
   }
 
   // Send a GesturePinchUpdate event.
-  const WebGestureEvent& updateEvent =
+  WebGestureEvent updateEvent =
       WebInputEventFactory::gestureEvent(event, self);
+  updateEvent.data.pinchUpdate.zoomDisabled = !pinchHasReachedZoomThreshold_;
   renderWidgetHostView_->render_widget_host_->ForwardGestureEvent(updateEvent);
 }
 
 - (void)viewWillMoveToWindow:(NSWindow*)newWindow {
   NSWindow* oldWindow = [self window];
 
   NSNotificationCenter* notificationCenter =
       [NSNotificationCenter defaultCenter];
 
   // Backing property notifications crash on 10.6 when building with the 10.7
@@ skipping 1036 matching lines @@
 
 // "-webkit-app-region: drag | no-drag" is implemented on Mac by excluding
 // regions that are not draggable. (See ControlRegionView in
 // native_app_window_cocoa.mm). This requires the render host view to be
 // draggable by default.
 - (BOOL)mouseDownCanMoveWindow {
   return YES;
 }
 
 @end