MacViews: Handle three-finger swipe gestures for navigation.

chrome/browser/ui/views/frame/browser_view.cc

 // Copyright 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 "chrome/browser/ui/views/frame/browser_view.h"
 
 #include <stdint.h>
 
 #include <algorithm>
 #include <memory>
@@ skipping 1962 matching lines @@
   // Do not handle native theme changes before the browser view is initialized.
   if (!initialized_)
     return;
   ClientView::OnNativeThemeChanged(theme);
   // Don't infinitely recurse.
   if (!handling_theme_changed_)
     UserChangedTheme();
   chrome::MaybeShowInvertBubbleView(this);
 }
 
+void BrowserView::OnGestureEvent(ui::GestureEvent* event) {
+#if defined(OS_MACOSX)
+  if (event->details().type() == ui::ET_GESTURE_SWIPE) {

[sky, 2017/02/10 19:08:33]

Can you structure this code to be a bit more readable, e.g.:

int command = NO_COMMAND (hopefully there is a define for it).
if (GetGestureCommand(*event, &command) && IsCommandE...)
  ExecuteCommand...

[themblsha, 2017/02/15 17:35:34]

base::Optional<int> should be a better way to do it rather than a NO_COMMAND
(and there seem to be no such thing in chrome_command_ids.h).

+    unsigned int command = 0;

[sky, 2017/02/10 19:08:33]

Aren't command ids ints?

[themblsha, 2017/02/15 17:35:34]

The code was adapted from the browser_window_controller.mm. Updated to an int.

+    if (event->details().swipe_left())

[tapted, 2017/02/13 11:26:24]

-[BrowserWindowController swipeWithEvent:] only triggers these commands when the
delta is over a threshold (i.e. 0.5). but GestureEvent sets these bools with any
strictly positive value - I don't think this will give a good UX.

[themblsha, 2017/02/15 17:35:34]

In my testing the -swipeWithEvent: is always called with a value of "+/-1"
without taking gesture speed into account. This matches Apple's documentation
that the delta should be simply taken as positive/negative.

Only a single event is send per three-finger swipe. The UX is exactly the same
as with the other three-finger swiping apps :-)

[tapted, 2017/02/15 22:25:52]

Can you comment about this where you construct the GestureEvent? - that's a
great breadcrumb to leave for a future reader.
oh weird - yeah even Safari has no feedback for this. (yuck)

[themblsha, 2017/02/16 15:27:49]

Done.
They're probably from the ancient times when the GestureRecognizers haven't been
invented yet. And some people probably enjoy the immediacy and relative lack of
feedback :-)

+      command = IDC_FORWARD;
+    else if (event->details().swipe_right())

[tapted, 2017/02/13 11:26:24]

I really think left/right around the right are around the wrong way here.

"left" should go back, "right" should go forward.

The problem is that GestureEventDetails does

      data_.swipe.left = delta_x < 0;
      data_.swipe.right = delta_x > 0;

but BrowserWindowController does

  if (deltaX > 0.5) {
    command = IDC_BACK;
  } else if (deltaX < -0.5) {
    command = IDC_FORWARD;
  }

I'd say it's the NSEvents that are "weird" so you'll need to put on a negative
sign before passing [event deltaX] to the GestureEvent constructor.

Please test deltaY manually to see what should be done with that (i.e. ensure
swipe_up()/down() actually correspond to whether you're swiping up or down

[themblsha, 2017/02/15 17:35:34]

I originally tested the behaviour to match Safari and Cocoa Chrome. Inverted the
deltaX / deltaY in the constructor call, thanks!

+      command = IDC_BACK;
+
+    if (chrome::IsCommandEnabled(browser(), command)) {
+      chrome::ExecuteCommandWithDisposition(
+          browser(), command, ui::DispositionFromEventFlags(event->flags()));
+    }
+    return;
+  }
+#endif  // OS_MACOSX
+
+  ClientView::OnGestureEvent(event);
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // BrowserView, ui::AcceleratorTarget overrides:
 
 bool BrowserView::AcceleratorPressed(const ui::Accelerator& accelerator) {
   std::map<ui::Accelerator, int>::const_iterator iter =
       accelerator_table_.find(accelerator);
   DCHECK(iter != accelerator_table_.end());
   int command_id = iter->second;
 
   if (accelerator.IsRepeat() && !IsCommandRepeatable(command_id))
@@ skipping 644 matching lines @@
 }
 
 extensions::ActiveTabPermissionGranter*
 BrowserView::GetActiveTabPermissionGranter() {
   content::WebContents* web_contents = GetActiveWebContents();
   if (!web_contents)
     return nullptr;
   return extensions::TabHelper::FromWebContents(web_contents)
       ->active_tab_permission_granter();
 }