mac: Only let two-finger-scrolling trigger history if web doesn't swallow gesture

chrome/browser/renderer_host/render_widget_host_view_mac.mm

 // Copyright (c) 2011 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 <QuartzCore/QuartzCore.h>
 
 #include "chrome/browser/renderer_host/render_widget_host_view_mac.h"
 
 #include "base/debug/trace_event.h"
 #include "base/logging.h"
 #include "base/mac/mac_util.h"
 #include "base/mac/scoped_cftyperef.h"
 #import "base/mac/scoped_nsautorelease_pool.h"
 #include "base/metrics/histogram.h"
 #import "base/memory/scoped_nsobject.h"
 #include "base/string_util.h"
 #include "base/sys_info.h"
 #include "base/sys_string_conversions.h"
 #include "chrome/browser/browser_trial.h"
 #import "chrome/browser/renderer_host/accelerated_plugin_view_mac.h"
 #import "chrome/browser/renderer_host/text_input_client_mac.h"
 #include "chrome/browser/spellchecker/spellchecker_platform_engine.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_list.h"
 #import "chrome/browser/ui/cocoa/rwhvm_editcommand_helper.h"
 #import "chrome/browser/ui/cocoa/view_id_util.h"
 #include "chrome/common/render_messages.h"
 #include "chrome/common/spellcheck_messages.h"
 #import "content/browser/accessibility/browser_accessibility_cocoa.h"
 #include "content/browser/browser_thread.h"
 #include "content/browser/gpu/gpu_process_host.h"
 #include "content/browser/gpu/gpu_process_host_ui_shim.h"
 #include "content/browser/plugin_process_host.h"
 #include "content/browser/renderer_host/backing_store_mac.h"
@@ skipping 16 matching lines @@
 #include "ui/gfx/surface/io_surface_support_mac.h"
 #include "webkit/glue/webaccessibility.h"
 #include "webkit/plugins/npapi/webplugin.h"
 
 using WebKit::WebInputEvent;
 using WebKit::WebInputEventFactory;
 using WebKit::WebMouseEvent;
 using WebKit::WebMouseWheelEvent;
 using WebKit::WebGestureEvent;
 
+// Forward-declare symbols that are part of the 10.7 SDK.
+#if !defined(MAC_OS_X_VERSION_10_7) || \
+    MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7
+
+enum {
+    NSEventPhaseNone        = 0, // event not associated with a phase.
+    NSEventPhaseBegan       = 0x1 << 0,
+    NSEventPhaseStationary  = 0x1 << 1,
+    NSEventPhaseChanged     = 0x1 << 2,
+    NSEventPhaseEnded       = 0x1 << 3,
+    NSEventPhaseCancelled   = 0x1 << 4,
+};
+typedef NSUInteger NSEventPhase;
+
+enum {
+     NSEventSwipeTrackingLockDirection = 0x1 << 0,
+     NSEventSwipeTrackingClampGestureAmount = 0x1 << 1
+};
+typedef NSUInteger NSEventSwipeTrackingOptions;
+
+
+@interface NSEvent(LionAPI)

[Robert Sesek, 2011/08/15 14:46:27]

nit: space before (

[Nico, 2011/08/15 16:39:46]

Done.

+// This is in the appkit 10.7 release notes, but not in

[Mark Mentovai, 2011/08/15 14:43:57]

If this comment were true, this declaration should come out of the #ifdef
because it would be needed with the 10.7 SDK too. However, I don’t believe this
comment.

mark@leo bash$ grep isSwipeTrackingFromScrollEventsEnabled
/Developer/SDKs/MacOSX10.7.sdk/System/Library/Frameworks/AppKit.framework/Headers/NSEvent.h
+ (BOOL)isSwipeTrackingFromScrollEventsEnabled NS_AVAILABLE_MAC(10_7);
mark@leo bash$ grep isSwipeTrackingFromScrollEventsEnabled
/Xcode4.2/SDKs/MacOSX10.7.sdk/System/Library/Frameworks/AppKit.framework/Headers/NSEvent.h
+ (BOOL)isSwipeTrackingFromScrollEventsEnabled NS_AVAILABLE_MAC(10_7);

/Developer is Xcode 4.1 4B110 and /Xcode4.2 is Xcode 4.2 4D75. Are you using a
different version? Can you recheck your headers? Based on what I have on my
system, I think this comment should be removed.

[Nico, 2011/08/15 16:39:46]

You're right. It's not documented at
http://developer.apple.com/library/mac/#documentation/cocoa/reference/Applica...
though. Removed comment.

+// the public nsevent headers?
++ (BOOL)isSwipeTrackingFromScrollEventsEnabled;
+
+- (NSEventPhase)phase;
+- (CGFloat)scrollingDeltaX;
+- (CGFloat)scrollingDeltaY;
+- (void)trackSwipeEventWithOptions:(NSEventSwipeTrackingOptions)options
+          dampenAmountThresholdMin:(CGFloat)minDampenThreshold
+                               max:(CGFloat)maxDampenThreshold
+                      usingHandler:(void (^)(CGFloat gestureAmount,

[Mark Mentovai, 2011/08/15 14:43:57]

You must #include "chrome/browser/mac/closure_blocks_leopard_compat.h" to ensure
that references to the block symbols are marked for weak import.

(Actually, the point at which this #include is required is the point of the use
of a ^block, not at this declaration, but you use a ^block later in this file,
and even if you had stuck the declaration in a header, it’d be a good idea to
#include the compatibility header there to ensure that anyone adhering to the
API got the right behavior.)

[Nico, 2011/08/15 16:39:46]

Good news: I was aware of that and thought I had done it :-P

+                                             NSEventPhase phase,
+                                             BOOL isComplete,
+                                             BOOL *stop))trackingHandler;
+@end
+
+#endif  // 10.7
+
 static inline int ToWebKitModifiers(NSUInteger flags) {
   int modifiers = 0;
   if (flags & NSControlKeyMask) modifiers |= WebInputEvent::ControlKey;
   if (flags & NSShiftKeyMask) modifiers |= WebInputEvent::ShiftKey;
   if (flags & NSAlternateKeyMask) modifiers |= WebInputEvent::AltKey;
   if (flags & NSCommandKeyMask) modifiers |= WebInputEvent::MetaKey;
   return modifiers;
 }
 
 // Private methods:
@@ skipping 942 matching lines @@
 
 void RenderWidgetHostViewMac::ForceTextureReload() {
   plugin_container_manager_.ForceTextureReload();
 }
 
 void RenderWidgetHostViewMac::SetVisuallyDeemphasized(const SkColor* color,
                                                       bool animate) {
   // This is not used on mac.
 }
 
+void RenderWidgetHostViewMac::UnhandledWheelEvent(
+    const WebKit::WebMouseWheelEvent& event) {
+  [cocoa_view_ setGotUnhandledWheelEvent:YES];
+}
+
+void RenderWidgetHostViewMac::SetHasHorizontalScrollbar(
+    bool has_horizontal_scrollbar) {
+  [cocoa_view_ setHasHorizontalScrollbar:has_horizontal_scrollbar];
+}
+
+void RenderWidgetHostViewMac::SetScrollOffsetPinning(
+    bool is_pinned_to_left, bool is_pinned_to_right) {
+  [cocoa_view_ setPinnedLeft:is_pinned_to_left];
+  [cocoa_view_ setPinnedRight:is_pinned_to_right];
+}
+
 void RenderWidgetHostViewMac::ShutdownHost() {
   shutdown_factory_.RevokeAll();
   render_widget_host_->Shutdown();
   // Do not touch any members at this point, |this| has been deleted.
 }
 
 gfx::Rect RenderWidgetHostViewMac::GetViewCocoaBounds() const {
   return gfx::Rect(NSRectToCGRect([cocoa_view_ bounds]));
 }
 
@@ skipping 63 matching lines @@
       DisablePasswordInput();
   }
 }
 
 // RenderWidgetHostViewCocoa ---------------------------------------------------
 
 @implementation RenderWidgetHostViewCocoa
 
 @synthesize selectedRange = selectedRange_;
 @synthesize markedRange = markedRange_;
+@synthesize gotUnhandledWheelEvent = gotUnhandledWheelEvent_;
+@synthesize isPinnedLeft = isPinnedLeft_;
+@synthesize isPinnedRight = isPinnedRight_;
+@synthesize hasHorizontalScrollbar = hasHorizontalScrollbar_;
 
 - (id)initWithRenderWidgetHostViewMac:(RenderWidgetHostViewMac*)r {
   self = [super initWithFrame:NSZeroRect];
   if (self) {
     editCommand_helper_.reset(new RWHVMEditCommandHelper);
     editCommand_helper_->AddEditingSelectorsToClass([self class]);
 
     renderWidgetHostView_.reset(r);
     canBeKeyView_ = YES;
     focusedPluginIdentifier_ = -1;
@@ skipping 113 matching lines @@
 - (void)beginGestureWithEvent:(NSEvent*)event {
   if (base::mac::IsOSLionOrLater() &&
       [event subtype] == kIOHIDEventTypeScroll &&
       renderWidgetHostView_->render_widget_host_) {
     WebGestureEvent webEvent = WebInputEventFactory::gestureEvent(event, self);
     renderWidgetHostView_->render_widget_host_->ForwardGestureEvent(webEvent);
   }
 
   // Forward the gesture event to the next responder so that the browser window
   // controller has a chance to act on back/forward gestures.
   [[self nextResponder] beginGestureWithEvent:event];

[Alexei Svitkine (slow), 2011/08/15 15:58:42]

Since the browser controller now won't implement -beginGestureWithEvent:, can
you delete this?

[Nico, 2011/08/15 16:39:46]

Done.

 }
 
 - (void)endGestureWithEvent:(NSEvent*)event {
   if (base::mac::IsOSLionOrLater() &&
       [event subtype] == kIOHIDEventTypeScroll &&
       renderWidgetHostView_->render_widget_host_) {
     WebGestureEvent webEvent = WebInputEventFactory::gestureEvent(event, self);
     renderWidgetHostView_->render_widget_host_->ForwardGestureEvent(webEvent);
   }
 
   // Forward the gesture event to the next responder so that the browser window
   // controller has a chance to act on back/forward gestures.
   [[self nextResponder] endGestureWithEvent:event];

[Alexei Svitkine (slow), 2011/08/15 15:58:42]

Ditto.

[Nico, 2011/08/15 16:39:46]

Done.

 }
 
 - (BOOL)performKeyEquivalent:(NSEvent*)theEvent {
   // |performKeyEquivalent:| is sent to all views of a window, not only down the
   // responder chain (cf. "Handling Key Equivalents" in
   // http://developer.apple.com/mac/library/documentation/Cocoa/Conceptual/EventOverview/HandlingKeyEvents/HandlingKeyEvents.html
   // ). We only want to handle key equivalents if we're first responder.
   if ([[self window] firstResponder] != self)
     return NO;
 
@@ skipping 229 matching lines @@
       event.skip_in_browser = true;
       widgetHost->ForwardKeyboardEvent(event);
     }
   }
 
   // Possibly autohide the cursor.
   if ([RenderWidgetHostViewCocoa shouldAutohideCursorForEvent:theEvent])
     [NSCursor setHiddenUntilMouseMoves:YES];
 }
 
-- (void)scrollWheel:(NSEvent *)theEvent {
+- (void)scrollWheel:(NSEvent*)theEvent {
   [self cancelChildPopups];
 
+  bool canUseLionApis = [theEvent respondsToSelector:@selector(phase)];

[Mark Mentovai, 2011/08/15 14:43:57]

Stick with a BOOL for this one because that’s the type that the thing you’re
calling returns.

You can leave the bools below because those are just logic and you don’t care
about matching return types.

[Nico, 2011/08/15 16:39:46]

Done.

+
+  // Scroll events always go to the web first, and can only trigger history
+  // swiping if they come back unhandled.
+  if (canUseLionApis && [theEvent phase] == NSEventPhaseBegan) {

[Mark Mentovai, 2011/08/15 14:43:57]

Is there any reason you don’t wrap this block and the next into a larger block
named “if (canUseLionApis)”? Because…

(continued on line 1574)

+    totalScrollDelta_ = NSZeroSize;
+    gotUnhandledWheelEvent_ = false;

[Mark Mentovai, 2011/08/15 14:43:57]

This property is a BOOL, so use NO.

+  }
+
+  if (canUseLionApis &&  // Must be the first condition!

[Mark Mentovai, 2011/08/15 14:43:57]

(continued from line 1569)

…it’d eliminate this tricky “must be the first condition!” thing. I don’t see
the sense in checking canUseLionApis twice when its value won’t change between
the two checks.

[Robert Sesek, 2011/08/15 14:46:27]

Is there any reason to not put this and the previous block in an outer |if
(canUseLionApis)|?

[Nico, 2011/08/15 16:39:46]

The reason was indentation. I moved the history tracking into its own function,
so Done.

+      gotUnhandledWheelEvent_ &&
+      [NSEvent isSwipeTrackingFromScrollEventsEnabled] &&

[Robert Sesek, 2011/08/15 14:46:27]

Doing this the proper way means that you can use this. Yay!

+      [theEvent phase] == NSEventPhaseChanged) {
+    totalScrollDelta_.width += [theEvent scrollingDeltaX];
+    totalScrollDelta_.height += [theEvent scrollingDeltaY];
+
+    bool isHorizontalGesture =

[Robert Sesek, 2011/08/15 14:46:27]

über/optional-nit: Cocoa BOOLs?

+      fabs(totalScrollDelta_.width) > fabs(totalScrollDelta_.height);

[Mark Mentovai, 2011/08/15 14:43:57]

#include <cmath> and use std::abs instead of fabs.

[Robert Sesek, 2011/08/15 14:46:27]

std::abs

[Nico, 2011/08/15 16:39:46]

Done.

+
+    bool isRightScroll = [theEvent scrollingDeltaX] < 0;
+    bool goForward = isRightScroll;
+
+    if (isHorizontalGesture &&
+        // If "forward" is inactive and the user rubber-bands to the right,
+        // "isPinnedLeft" will be false.  When the user then rubber-bands to
+        // the left in the same gesture, that should trigger history
+        // immediately if there's no scrollbar, hence the check for
+        // hasHorizontalScrollbar_.
+        // One would think we have to check canGoBack / canGoForward here, but
+        // that's actually done in the renderer
+        // (ChromeClientImpl::shouldRubberBand()), when it decides if it should
+        // rubberband or send back an event unhandled.
+        (((isPinnedLeft_ || !hasHorizontalScrollbar_) && !isRightScroll) ||

[Mark Mentovai, 2011/08/15 14:43:57]

I think you can simplify this entire thing by pulling !hasHorizontalScrollbar
out of this portion and listing it only once.

[Nico, 2011/08/15 16:39:46]

Done.

+            ((isPinnedRight_ || !hasHorizontalScrollbar_) && isRightScroll))) {
+      // The way this api works: gestureAmount is between -1 and 1 (float). If
+      // the user does the gesture for more than about 25% (i.e. < -0.25 or >
+      // 0.25) and then lets go, it is accepted, we get a NSEventPhaseEnded,
+      // and after that the block is called with amounts animating towards 1
+      // (or -1, depending on the direction).  If the user lets go below that
+      // threshold, we get NSEventPhaseCancelled, and the amount animates
+      // toward 0.
+      [theEvent trackSwipeEventWithOptions:0
+                 dampenAmountThresholdMin:-1

[Mark Mentovai, 2011/08/15 14:43:57]

Nit: align colons.

[Mark Mentovai, 2011/08/15 14:43:57]

Do you know how AppKit uses these values? I suppose that -1 and 1 is fine for
how we’re (ab)using this, but “real” values would make for a smoother experience
for things that put all scrolling through this, or animating CoverFlow, or that
sort of thing.

[Nico, 2011/08/15 16:39:46]

Done.

[Nico, 2011/08/15 16:39:46]

I don't really understand what these values do. I tried -1,1 and -500,500 and
couldn't feel a difference. Will play with this again once I have some UI.

+                                      max:1
+                             usingHandler:^(CGFloat gestureAmount,
+                                            NSEventPhase phase,
+                                            BOOL isComplete,

[Robert Sesek, 2011/08/15 14:46:27]

Why do you not reference isComplete in this? Are you saving that for the UI
portion?

[Nico, 2011/08/15 16:39:46]

Yes. The way this works is that as soon as the user lets go their fingers, we
get an NSEventPhaseEnded, and then the block is called with |gestureAmount|
animating towards 1, and only then isComplete is set to true. Doing the
navigation on isComplete is too late. (With UI, we'll do the navigation on
NSEventPhaseEnded, and then continue to show the gesture UI until isComplete
becomes true)

+                                            BOOL *stop) {
+          // |gestureAmount| obeys -[NSEvent isDirectionInvertedFromDevice]
+          // automatically.
+          // TODO(thakis): UI.
+          Browser* browser = BrowserList::GetLastActive();
+          if (phase == NSEventPhaseEnded && browser) {
+            if (goForward)
+              browser->GoForward(CURRENT_TAB);
+            else
+              browser->GoBack(CURRENT_TAB);
+          }
+        }];
+      return;
+    }
+  }
+
   const WebMouseWheelEvent& event =
       WebInputEventFactory::mouseWheelEvent(theEvent, self);
   if (renderWidgetHostView_->render_widget_host_)
     renderWidgetHostView_->render_widget_host_->ForwardWheelEvent(event);
 }
 
 // See the comment in RenderWidgetHostViewMac::Destroy() about cancellation
 // events. On the Mac we must kill popups on outside events, thus this lovely
 // case of filicide caused by events on parent views.
 - (void)cancelChildPopups {
@@ skipping 1248 matching lines @@
   if (!string) return NO;
 
   // If the user is currently using an IME, confirm the IME input,
   // and then insert the text from the service, the same as TextEdit and Safari.
   [self confirmComposition];
   [self insertText:string];
   return YES;
 }
 
 @end