MacViews: Handle three-finger swipe gestures for navigation.

ui/views/view_unittest_mac.mm

+// Copyright 2017 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.
+
+#import <Cocoa/Cocoa.h>
+
+#include "base/mac/scoped_nsobject.h"

[tapted, 2017/02/20 23:14:21]

nit: import

[themblsha, 2017/02/21 13:00:51]

Done.

+#include "base/optional.h"
+#include "ui/events/gesture_event_details.h"
+#include "ui/views/test/widget_test.h"
+#include "ui/views/view.h"

[tapted, 2017/02/20 23:14:21]

nit: move this one all the way to the top (above Cocoa), with a blank line
under.

[themblsha, 2017/02/21 13:00:51]

Done.

+
+// We can't create NSEventTypeSwipe using normal means, and rely on duck typing
+// instead.
+@interface FakeSwipeEvent : NSEvent
+@property CGFloat deltaX;
+@property CGFloat deltaY;
+@property(assign) NSWindow* window;
+@property NSPoint locationInWindow;
+@property NSEventModifierFlags modifierFlags;
+@property NSTimeInterval timestamp;
+@end
+
+@implementation FakeSwipeEvent
+@synthesize deltaX;
+@synthesize deltaY;
+@synthesize window;
+@synthesize locationInWindow;
+@synthesize modifierFlags;
+@synthesize timestamp;
+
+- (NSEventType)type {
+  return NSEventTypeSwipe;
+}
+
+- (NSEventSubtype)subtype {
+  return static_cast<NSEventSubtype>(0);

[tapted, 2017/02/20 23:14:21]

0 is `NSEventSubtypeWindowExposed` - maybe `return NSEventSubtypeTouch` ?

[themblsha, 2017/02/21 13:00:51]

In my testing the Swipe events all had 0 as their subtype. Added comment.

[tapted, 2017/02/22 00:04:44]

Acknowledged.

+}
+@end
+
+namespace views {
+

[tapted, 2017/02/20 23:14:21]

add anonymous

namespace {

[themblsha, 2017/02/21 13:00:51]

Done.

+// Stores last received swipe gesture direction vector in
+// |last_swipe_gesture()|.
+class ThreeFingerSwipeView : public View {
+ public:
+  ThreeFingerSwipeView() {}
+
+  // views::View:

[tapted, 2017/02/20 23:14:21]

nit: remove `views::`

[themblsha, 2017/02/21 13:00:51]

Done.

+  void OnGestureEvent(ui::GestureEvent* event) override {
+    DCHECK_EQ(event->details().type(), ui::ET_GESTURE_SWIPE);

[tapted, 2017/02/20 23:14:21]

nit: EXPECT_EQ(ui::ET_GESTURE_SWIPE, event->details().type());

 - DCHECK crashes the test process, which can fail unrelated tests when the test
harness runs in a threaded mode
 - For EXPECT, `EXPECT_FOO(<expected>, <actual>);` gives more readable messages.

[themblsha, 2017/02/21 13:00:52]

Thanks! I fixed the EXPECTs at the bottom of the file before submitting but
missed these ones.

+
+    int dx = 0, dy = 0;
+    if (event->details().swipe_left())
+      dx = -1;
+
+    if (event->details().swipe_right()) {
+      DCHECK_EQ(dx, 0);

[tapted, 2017/02/20 23:14:21]

EXPECT_EQ(0, dx)

[themblsha, 2017/02/21 13:00:51]

Done.

+      dx = 1;
+    }
+
+    if (event->details().swipe_down())
+      dy = 1;
+
+    if (event->details().swipe_up()) {
+      DCHECK_EQ(dy, 0);

[tapted, 2017/02/20 23:14:21]

EXPECT_EQ(0, dy);

[themblsha, 2017/02/21 13:00:51]

Done.

+      dy = -1;
+    }
+
+    last_swipe_gesture_ = gfx::Point(dx, dy);
+  }
+
+  base::Optional<gfx::Point> last_swipe_gesture() const {
+    return last_swipe_gesture_;
+  }
+
+ private:
+  base::Optional<gfx::Point> last_swipe_gesture_;
+
+  DISALLOW_COPY_AND_ASSIGN(ThreeFingerSwipeView);
+};
+

[tapted, 2017/02/20 23:14:21]

conventionally, the

}  // namespace

would go here.

[themblsha, 2017/02/21 13:00:51]

Also works when placed at the end of the ViewMacTest, but conventions are to be
adhered to.

+class ViewMacTest : public test::WidgetTest {
+ public:
+  base::Optional<gfx::Point> SwipeGestureVector(int dx, int dy) {
+    base::scoped_nsobject<FakeSwipeEvent> swipe_event(
+        [[FakeSwipeEvent alloc] init]);
+    swipe_event.get().deltaX = dx;

[tapted, 2017/02/20 23:14:21]

[swipe_event setDeltaX:dx]

same for all the stuff below

(pretty sure that works. We don't use Objc2 dot notation for properties in
Chrome [yet]).

[themblsha, 2017/02/21 13:00:51]

Done.

+    swipe_event.get().deltaY = dy;
+    swipe_event.get().window = widget_->GetNativeWindow();
+    swipe_event.get().locationInWindow = NSMakePoint(50, 50);
+    swipe_event.get().timestamp = [[NSProcessInfo processInfo] systemUptime];
+
+    // BridgedContentView should create an appropriate ui::GestureEvent and pass
+    // it to the views::Widget.
+    [[widget_->GetNativeWindow() contentView] swipeWithEvent:swipe_event];
+    return view_->last_swipe_gesture();
+  }
+
+  // testing::Test:
+  void SetUp() override {
+    WidgetTest::SetUp();
+
+    widget_ = CreateTopLevelPlatformWidget();
+    const gfx::Rect bounds(0, 0, 100, 100);
+    widget_->SetBounds(bounds);
+    widget_->Show();
+
+    view_ = new ThreeFingerSwipeView;
+    view_->SetBoundsRect(bounds);

[tapted, 2017/02/20 23:14:21]

This will make the view bigger than the window's content area due to the
titlebar. I'd probably do:

widget_->SetBounds(gfx::Rect(0, 0, 100, 100));
..
view_->SetSize(widget_->GetClientAreaBoundsInScreen().size());



(CreateTopLevelFramelessPlatformWidget() above is another option)

[themblsha, 2017/02/21 13:00:51]

There seems to be a lot of tests which set up the widgets/views without regard
to the titlebar, but still work because they generally target the central
portion of the widget. So I thought it would be fine. But your suggestion is
quite simple as well and correct to boot. Thanks!

+    widget_->GetContentsView()->AddChildView(view_);
+  }
+
+  void TearDown() override {
+    widget_->CloseNow();
+    WidgetTest::TearDown();
+  }
+
+ private:
+  Widget* widget_ = nullptr;
+  ThreeFingerSwipeView* view_ = nullptr;
+};

[tapted, 2017/02/20 23:14:21]

nit: DISALLOW_COPY_AND_ASSIGN(..)  (you'll need to add a default constructor
too)

[themblsha, 2017/02/21 13:00:51]

Done.

+
+// Three-finger swipes send immediate events and they cannot be tracked.
+TEST_F(ViewMacTest, HandlesThreeFingerSwipeGestures) {
+  EXPECT_EQ(gfx::Point(1, 0), *SwipeGestureVector(-1, 0));

[tapted, 2017/02/20 23:14:21]

nit: a comment like "Note that positive delta is left and up for NSEvent, which
is the inverse of ui::GestureEventDetails."

[themblsha, 2017/02/21 13:00:51]

Done.

+  EXPECT_EQ(gfx::Point(-1, 0), *SwipeGestureVector(1, 0));
+  EXPECT_EQ(gfx::Point(0, 1), *SwipeGestureVector(0, -1));
+  EXPECT_EQ(gfx::Point(0, -1), *SwipeGestureVector(0, 1));
+}
+
+}  // namespace views