MacViews: Implement Tab Dragging

ui/base/test/ui_controls_mac.mm

Index: ui/base/test/ui_controls_mac.mm
diff --git a/ui/base/test/ui_controls_mac.mm b/ui/base/test/ui_controls_mac.mm
index b7615b6c2fd78eab1ddbb87a351cf33f47c4efd1..f6de2f5058668c28ff3d30b8b3adaabf0c4f5e40 100644
--- a/ui/base/test/ui_controls_mac.mm
+++ b/ui/base/test/ui_controls_mac.mm
@@ -9,10 +9,14 @@
 
 #include "base/bind.h"
 #include "base/callback.h"
+#import "base/mac/foundation_util.h"
+#import "base/mac/scoped_objc_class_swizzler.h"
 #include "base/message_loop/message_loop.h"
 #include "ui/base/cocoa/cocoa_base_utils.h"
 #include "ui/events/keycodes/keyboard_code_conversion_mac.h"
 #import "ui/events/test/cocoa_test_event_utils.h"
+#include "ui/gfx/geometry/point.h"
+#import "ui/gfx/mac/coordinate_conversion.h"
 
 // Implementation details: We use [NSApplication sendEvent:] instead
 // of [NSApplication postEvent:atStart:] so that the event gets sent
@@ -52,6 +56,10 @@ namespace {
 // when firing keyboard and mouse click events.
 NSPoint g_mouse_location = { 0, 0 };
 
+// Stores the current pressed mouse buttons. Indexed by
+// ui_controls::MouseButton.
+bool g_mouse_button_down[3] = {false, false, false};
+
 bool g_ui_controls_enabled = false;
 
 // Creates the proper sequence of autoreleased key events for a key down + up.
@@ -125,6 +133,9 @@ void SynthesizeKeyEventsSequence(NSWindow* window,
 // A helper function to watch for the event queue. The specific task will be
 // fired when there is no more event in the queue.
 void EventQueueWatcher(const base::Closure& task) {
+  DCHECK_EQ(dispatch_get_current_queue(), dispatch_get_main_queue())

[tapted, 2016/06/01 11:29:55]

This DCHECK can probably go since we're not using GCD any more

[themblsha, 2016/06/03 17:42:08]

Done.

+      << "It should be run on the UI thread, as otherwise it will always "
+         "report there are no pending events";
   NSEvent* event = [NSApp nextEventMatchingMask:NSAnyEventMask
                                       untilDate:nil
                                          inMode:NSDefaultRunLoopMode
@@ -173,6 +184,73 @@ NSWindow* WindowAtCurrentMouseLocation() {
 
 }  // namespace
 
+// Donates testing implementations of NSEvent methods.
+@interface FakeNSEventTestingDonor : NSObject
+@end
+
+@implementation FakeNSEventTestingDonor
++ (NSPoint)mouseLocation {
+  return g_mouse_location;
+}
+
++ (NSUInteger)pressedMouseButtons {
+  NSUInteger result = 0;
+  const int buttons[3] = {
+      ui_controls::LEFT, ui_controls::RIGHT, ui_controls::MIDDLE};
+  for (unsigned int i = 0; i < arraysize(buttons); ++i) {

[tapted, 2016/06/01 11:29:55]

nit: unsigned int -> size_t (or just `int`, but that probably warns)

[themblsha, 2016/06/03 17:42:08]

Done.

+    if (g_mouse_button_down[buttons[i]])
+      result |= (1 << i);
+  }
+  return result;
+}
+@end
+
+// Donates testing implementations of NSWindow methods.
+@interface FakeNSWindowTestingDonor : NSObject
+@end
+
+@implementation FakeNSWindowTestingDonor
+- (NSPoint)mouseLocationOutsideOfEventStream {

[tapted, 2016/06/01 11:29:55]

Is anything being tested that requires this? (remove?).

[themblsha, 2016/06/03 17:42:08]

It's widely used in Chromium, and without this swizzle the mouse position
returned there would be inconsistent with g_mouse_location, as we're not really
moving the mouse.

That being said, removing this doesn't make any of the
DetachToBrowserTabDragControllerTests fail.

[tapted, 2016/06/06 07:12:36]

`git grep mouseLocationOutsideOfEventStream` shows up a bunch of stuff in
c/b/ui/cocoa code, but nothing interesting under src/ui, which is pretty much
all we use for toolkit-views on Mac.

This adds complexity, so we should remove it until we know it's needed. At this
stage it's possible all the uses of mouseLocationOutsideOfEventStream get phased
out of Chrome.

[themblsha, 2016/06/06 17:20:27]

Done.

+  NSWindow* window = base::mac::ObjCCastStrict<NSWindow>(self);
+  return ui::ConvertPointFromWindowToScreen(window, g_mouse_location);
+}
+@end
+
+namespace {

[tapted, 2016/06/01 11:29:55]

nit: blank line after

[themblsha, 2016/06/03 17:42:08]

Done.

+class NSEventSwizzler {

[tapted, 2016/06/01 11:29:55]

needs a comment

[themblsha, 2016/06/03 17:42:08]

Done.

+ public:
+  static void Install() {
+    static NSEventSwizzler* swizzler = nullptr;

[tapted, 2016/06/01 11:29:55]

I'm unsure about this -- the swizzler is "Scoped" for a reason - but this breaks
that.

We should move this class declaration to the header and allow the test harness
to construct it and store it in a data member on
|DetachToBrowserTabDragControllerTest| in SetUpOnMainThread() where we bring the
browser to the front. ScopedObjCClassSwizzler will need to be forward declared.

And we need a better name than NSEventSwizzler. Maybe
ScopedMockNSEventClassMethods

[themblsha, 2016/06/03 17:42:08]

In tests the entire browser state gets reconstructed anew for each test, isn't
it? And even if it isn't, I think it's a benefit, as after installing the
NSEventSwizzler you're not supposed to interact with the tested browser directly
 without using ui_controls.h functions.

I feel that this swizzler could benefit tons of other tests as well, as global
mouse position is used pretty widely, both directly and through
display::Screen::GetCursorScreenPoint().

EventGeneratorDelegateMac::SetContext() also swizzles several calls, although
I'm not sure when it's destroyed.

[tapted, 2016/06/06 07:12:36]

Not always -- there is `--single-process-tests` which is currently broken for
MacViews -- See http://crbug.com/594033 Maybe EventGeneratorDelegateMac is the
culprit.
EventGeneratorDelegateMac is explicitly installed on ViewsTestHelperMac, with
`test::InitializeMacEventGeneratorDelegate();` so it's less likely that things
like sharding or randomizing the order that tests are run in will change the
behaviour.

Calling `Install()` from multiple places, and as a side-effect of sending the
first test event (for some subset of test event types) is not a robust approach.

There's also void ViewsTestHelperMac::SetUp() which installs some correctly
scoped swizzler methods for tests, some specifically for interactive_ui_tests,
so we need to be careful of conflicts here. In particular,
ViewsTestHelperMac::SetUp() swizzles some stuff on NSWindow, but after removing
`FakeNSWindowTestingDonor` from this CL we keep some separation if we only
swizzle NSEvent stuff.

How about calling ScopedMockNSEventClassMethods::Init() in
ui_controls::EnableUIControls? This avoids the downsides of having
platform-specific initialization in the tests that a fine-grained scoped
approach would need.

[themblsha, 2016/06/06 17:20:27]

If it's only installed in EnableUIControls() and never removed, I don't think it
should have a Scoped prefix. Renamed the class and moved the installation there,
that single place is definitely a better fit.

+    if (!swizzler) {
+      swizzler = new NSEventSwizzler();
+    }
+  }
+
+ protected:
+  NSEventSwizzler()
+      : mouse_location_swizzler_(new base::mac::ScopedObjCClassSwizzler(
+            [NSEvent class],
+            [FakeNSEventTestingDonor class],
+            @selector(mouseLocation))),
+        pressed_mouse_buttons_swizzler_(new base::mac::ScopedObjCClassSwizzler(
+            [NSEvent class],
+            [FakeNSEventTestingDonor class],
+            @selector(pressedMouseButtons))),
+        mouse_location_outside_of_event_stream_swizzler_(
+            new base::mac::ScopedObjCClassSwizzler(
+                [NSWindow class],
+                [FakeNSWindowTestingDonor class],
+                @selector(mouseLocationOutsideOfEventStream))) {}
+
+ private:
+  std::unique_ptr<base::mac::ScopedObjCClassSwizzler> mouse_location_swizzler_;
+  std::unique_ptr<base::mac::ScopedObjCClassSwizzler>
+      pressed_mouse_buttons_swizzler_;
+  std::unique_ptr<base::mac::ScopedObjCClassSwizzler>
+      mouse_location_outside_of_event_stream_swizzler_;
+};

[tapted, 2016/06/01 11:29:55]

nit: DISALLOW_COPY_AND_ASSIGN(..)

[themblsha, 2016/06/03 17:42:08]

Done.

+}  // namespace

[tapted, 2016/06/01 11:29:55]

nit: blank line before

[themblsha, 2016/06/03 17:42:08]

Done.

+
 namespace ui_controls {
 
 void EnableUIControls() {
@@ -239,9 +317,8 @@ bool SendMouseMove(long x, long y) {
 // platforms.  E.g. (0,0) is upper-left.
 bool SendMouseMoveNotifyWhenDone(long x, long y, const base::Closure& task) {
   CHECK(g_ui_controls_enabled);
-  CGFloat screenHeight =
-    [[[NSScreen screens] firstObject] frame].size.height;
-  g_mouse_location = NSMakePoint(x, screenHeight - y);  // flip!
+  g_mouse_location = gfx::ScreenPointToNSPoint(gfx::Point(x, y));  // flip!
+  NSEventSwizzler::Install();
 
   NSWindow* window = WindowAtCurrentMouseLocation();
 
@@ -250,16 +327,25 @@ bool SendMouseMoveNotifyWhenDone(long x, long y, const base::Closure& task) {
     pointInWindow = ui::ConvertPointFromScreenToWindow(window, pointInWindow);
   NSTimeInterval timestamp = TimeIntervalSinceSystemStartup();
 
+  NSEventType event_type = NSMouseMoved;
+  if (g_mouse_button_down[LEFT]) {
+    event_type = NSLeftMouseDragged;
+  } else if (g_mouse_button_down[RIGHT]) {
+    event_type = NSRightMouseDragged;
+  } else if (g_mouse_button_down[MIDDLE]) {
+    event_type = NSOtherMouseDragged;
+  }
+
   NSEvent* event =
-      [NSEvent mouseEventWithType:NSMouseMoved
+      [NSEvent mouseEventWithType:event_type
                          location:pointInWindow
                     modifierFlags:0
                         timestamp:timestamp
                      windowNumber:[window windowNumber]
                           context:nil
                       eventNumber:0
-                       clickCount:0
-                         pressure:0.0];
+                       clickCount:(event_type == NSMouseMoved ? 0 : 1)

[tapted, 2016/06/01 11:29:55]

outer () parens not required, same below

[themblsha, 2016/06/03 17:42:08]

Done. Although there are parens in SendMouseEventsNotifyWhenDone, with an extra
space before the closing one.

+                         pressure:(event_type == NSMouseMoved ? 0.0 : 1.0)];
   [[NSApplication sharedApplication] postEvent:event atStart:NO];
 
   if (!task.is_null()) {
@@ -284,35 +370,40 @@ bool SendMouseEventsNotifyWhenDone(MouseButton type, int state,
     return (SendMouseEventsNotifyWhenDone(type, DOWN, base::Closure()) &&
             SendMouseEventsNotifyWhenDone(type, UP, task));
   }
-  NSEventType etype = NSLeftMouseDown;
+  NSEventType event_type = NSLeftMouseDown;
   if (type == LEFT) {
     if (state == UP) {
-      etype = NSLeftMouseUp;
+      event_type = NSLeftMouseUp;
     } else {
-      etype = NSLeftMouseDown;
+      event_type = NSLeftMouseDown;
     }
   } else if (type == MIDDLE) {
     if (state == UP) {
-      etype = NSOtherMouseUp;
+      event_type = NSOtherMouseUp;
     } else {
-      etype = NSOtherMouseDown;
+      event_type = NSOtherMouseDown;
     }
   } else if (type == RIGHT) {
     if (state == UP) {
-      etype = NSRightMouseUp;
+      event_type = NSRightMouseUp;
     } else {
-      etype = NSRightMouseDown;
+      event_type = NSRightMouseDown;
     }
   } else {
+    NOTREACHED();
     return false;
   }
+  g_mouse_button_down[type] = state == DOWN;
+
   NSWindow* window = WindowAtCurrentMouseLocation();
   NSPoint pointInWindow = g_mouse_location;
   if (window)
     pointInWindow = ui::ConvertPointFromScreenToWindow(window, pointInWindow);
 
+  NSEventSwizzler::Install();
+
   NSEvent* event =
-      [NSEvent mouseEventWithType:etype
+      [NSEvent mouseEventWithType:event_type
                          location:pointInWindow
                     modifierFlags:0
                         timestamp:TimeIntervalSinceSystemStartup()