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 1920 matching lines @@
   if (!initialized_ || in_process_fullscreen_)
     return;
 
   views::View::Layout();
 
   // TODO(jamescook): Why was this in the middle of layout code?
   toolbar_->location_bar()->omnibox_view()->SetFocusBehavior(
       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()) {
     InitViews();
     initialized_ = true;
   }
 }
 
 void BrowserView::ChildPreferredSizeChanged(View* child) {
   Layout();
@@ skipping 686 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();
 }