MacViews: Handle three-finger swipe gestures for navigation.

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

Index: chrome/browser/ui/views/frame/browser_view.cc
diff --git a/chrome/browser/ui/views/frame/browser_view.cc b/chrome/browser/ui/views/frame/browser_view.cc
index f4dbf87a9b3440d7ce19808cf32bf6b2c92f3fe7..76eef5e3f426c307069403dc44488d0f62601250 100644
--- a/chrome/browser/ui/views/frame/browser_view.cc
+++ b/chrome/browser/ui/views/frame/browser_view.cc
@@ -1938,6 +1938,31 @@ void BrowserView::Layout() {
       IsToolbarVisible() ? FocusBehavior::ALWAYS : FocusBehavior::NEVER);
 }
 
+void BrowserView::OnGestureEvent(ui::GestureEvent* event) {
+#if defined(OS_MACOSX)
+  auto get_gesture_command = [](ui::GestureEvent* event,

[sky, 2017/02/22 00:37:17]

IMO using a closure for this is overkill. How about a normal function?

[themblsha, 2017/02/22 11:49:50]

Ok. I thought that this lambda serves only a very specific purpose on macOS so
it should be visually close to the OnGestureEvent call, and anonymous namespace
is pretty far away.

[sky, 2017/02/22 17:34:10]

You can always use an anonymous namespace above this function, which gives you
most of the scoping you are concerned about.

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

Ah, there seems to be no restriction on where the anonymous namespace should be
placed, but someone scolded me about it once before when I tried to add one
right before the test that needed it. A single anonymous namespace at the top of
the file is the Chromium convention, so it's safe :-)

+                                base::Optional<int>* command) {
+    if (event->details().type() == ui::ET_GESTURE_SWIPE) {
+      if (event->details().swipe_left())
+        *command = IDC_BACK;
+      else if (event->details().swipe_right())
+        *command = IDC_FORWARD;
+    }
+    return command->has_value();
+  };
+
+  base::Optional<int> command;

[sky, 2017/02/22 00:37:17]

Why bother with a base_optional instead of just the return value? Using an
optional make this less understandable.

[themblsha, 2017/02/22 11:49:50]

I thought that it would provide a nice additional level of safety: it would
DCHECK if you would try to access the value while there's none. And there's no
specific command id to signify that it's not initialized, that was its second
purpose.

[sky, 2017/02/22 17:34:10]

Like I said, I don't think it really buys you much in this case.

+  if (get_gesture_command(event, &command) &&
+      chrome::IsCommandEnabled(browser(), *command)) {
+    chrome::ExecuteCommandWithDisposition(
+        browser(), *command, ui::DispositionFromEventFlags(event->flags()));
+    return;
+  }
+#endif  // OS_MACOSX
+
+  ClientView::OnGestureEvent(event);
+}
+
 void BrowserView::ViewHierarchyChanged(
     const ViewHierarchyChangedDetails& details) {
   if (!initialized_ && details.is_add && details.child == this && GetWidget()) {