MacViews: Implement Tab Dragging

chrome/test/base/interactive_test_utils.h

Index: chrome/test/base/interactive_test_utils.h
diff --git a/chrome/test/base/interactive_test_utils.h b/chrome/test/base/interactive_test_utils.h
index f0ccc770f2a8cfae8ca6437cdfc83364527badf7..b61116b00995850e9fdeeefee6b093c7d6b6fcc5 100644
--- a/chrome/test/base/interactive_test_utils.h
+++ b/chrome/test/base/interactive_test_utils.h
@@ -8,10 +8,7 @@
 #include "chrome/browser/ui/view_ids.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "ui/base/test/ui_controls.h"
-
-namespace gfx {
-class Point;
-}
+#include "ui/gfx/geometry/point.h"
 
 #if defined(TOOLKIT_VIEWS)
 namespace views {
@@ -21,6 +18,59 @@ class View;
 
 namespace ui_test_utils {
 
+#if defined(OS_MACOSX)
+// Moves the mouse to the |from| position, presses left mouse button,
+// then moves mouse to the |to| position and releases left mouse button.
+// |steps| indicates number of intermediate points that are interpolated between
+// |from| and |to|.
+void DragAndDrop(const gfx::Point& from, const gfx::Point& to, int steps = 1);
+
+class DragAndDropOperation {

[tapted, 2016/05/23 07:29:27]

add a comment, e.g.

// Represents an operation in a sequence of simulated drag and drop events
executed asynchronously by DragAndDropSequence.

[themblsha, 2016/05/26 15:13:25]

Done.

+ public:
+  enum class Type {
+    Move,
+    MoveWithoutAck,

[tapted, 2016/05/23 07:29:27]

Can this (and the creator function) be removed? I don't think there's a
compelling use case for this and it seems to add unnecessary complexity.

[themblsha, 2016/05/26 15:13:25]

At the moment there's no need for it, yeah.

+    MouseDown,
+    MouseUp,
+    SetMousePositionOverride,
+    UnsetMousePositionOverride,
+    DebugDelay

[tapted, 2016/05/23 07:29:27]

I think we can remove DebugDelay. If we really want to keep it, it should be
documented heavily below.

[themblsha, 2016/05/26 15:13:24]

Yeah, I used it to look at what was actually happening on the screen while I was
debugging the tests. There's no need for it when tests are working fine. If some
tough-to-debug problem happens again this (or something similar) could be
re-added, I guess.

+  };
+
+  static DragAndDropOperation Move(const gfx::Point& p);
+  // Doesn't wait for the event to finish processing, instead it waits for
+  // |delay|.
+  static DragAndDropOperation MoveWithoutAck(const gfx::Point& p,
+                                             const base::TimeDelta& delay);
+  static DragAndDropOperation MouseDown();
+  static DragAndDropOperation MouseUp();
+  static DragAndDropOperation SetMousePositionOverride(const gfx::Point& p);
+  static DragAndDropOperation UnsetMousePositionOverride();
+  static DragAndDropOperation DebugDelay();

[tapted, 2016/05/23 07:29:27]

E.g.

// Creates a one-second delay. This is for debugging complex drag and drop
operations only. Do not use it in a test.

[themblsha, 2016/05/26 15:13:25]

Should I re-add this even though there's no immediate need for it? :-)

+
+  Type type() const { return type_; }
+  const gfx::Point& point() const { return point_; }
+  const base::TimeDelta& delay() const { return delay_; }
+
+ private:
+  DragAndDropOperation(Type type,
+                       const gfx::Point& p,
+                       const base::TimeDelta& delay = base::TimeDelta())
+      : type_(type), point_(p), delay_(delay) {}
+
+  Type type_;
+  gfx::Point point_;
+  base::TimeDelta delay_;

[tapted, 2016/05/23 07:29:27]

(then I think all the delays are one second, and we don't need to keep this
extra data member around)

+};
+
+// Performs a series of drag-and-drop operations on a background thread, while
+// spinning a RunLoop on the main thread. If there are nested eventloops on the
+// main thread's queue, our helper RunLoop won't quit and we'll get stuck.
+// BridgedNativeWidget::RunMoveLoop() creates a RunLoop, so we need to ensure
+// that Drag'n'Drop is finished before trying to return from the function.

[tapted, 2016/05/23 07:29:27]

nit: "Drag'n'Drop" -> "sequence"

[themblsha, 2016/05/26 15:13:25]

Done.

+void DragAndDrop(const std::list<DragAndDropOperation>& operations);

[tapted, 2016/05/23 07:29:27]

I don't think overloading is appropriate here - perhaps call this
DragAndDropSequence. And is there any advantage to using std::list? Nothing is
inserting in weird places, so I think vector is more appropriate.

[themblsha, 2016/05/26 15:13:25]

I'm removing items from the front as they're processed (there's some performance
overhead with list but it should be negligible in this case). Isn't that a good
enough reason?

+#endif  // OS_MACOSX
+
 // Brings the native window for |browser| to the foreground. Returns true on
 // success.
 bool BringBrowserWindowToFront(const Browser* browser) WARN_UNUSED_RESULT;