MacViews: Fix some TableView tests, add ui::EventGenerator::set_assume_window_at_origin(bool)

ui/events/test/event_generator.h

Index: ui/events/test/event_generator.h
diff --git a/ui/events/test/event_generator.h b/ui/events/test/event_generator.h
index 51268390a42ba1637d3bae85982daa1b656e0e9e..1bd227926b902084cada49320f07198a37344423 100644
--- a/ui/events/test/event_generator.h
+++ b/ui/events/test/event_generator.h
@@ -159,6 +159,20 @@ class EventGenerator {
   void set_flags(int flags) { flags_ = flags; }
   int flags() const { return flags_; }
 
+  // Many tests assume a window created at (0,0) will remain there when shown.
+  // However, an operating system's window manager may reposition the window
+  // into the work area. This can disrupt the coordinates used on test events,
+  // so an EventGeneratorDelegate may skip the step that remaps coordinates in
+  // the root window to window coordinates when dispatching events.
+  // Setting this to false skips that step, in which case the test must ensure
+  // it correctly maps coordinates in window coordinates to root window (screen)
+  // coordinates when calling, e.g., set_current_location().
+  // Default is true. This only has any effect on Mac.
+  void set_assume_window_at_origin(bool assume_window_at_origin) {

[sky, 2017/04/05 15:39:53]

Is a better name and description for this coordinates_relative_to_root_window?
Where the root_window is generally the widget?

[tapted, 2017/04/05 23:57:18]

I see how that would fit with the way the coordinates are remapped, but it might
not capture the use case properly. E.g. consider one of the problematic tests,
CustomButtonTest.HideInkDropHighlightOnDisable [1]

It's doing

  ui::test::EventGenerator generator(widget()->GetNativeWindow());
  generator.MoveMouseToInHost(10, 10);

And it sets up and Show()s the Widget with

    Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_POPUP);
    params.ownership = views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
    params.bounds = gfx::Rect(0, 0, 650, 650);
    widget_->Init(params);
    widget_->Show();
    button_ = new TestCustomButton(false);
    widget_->SetContentsView(button_);

POPUP + origin=(0,0) results in a Window with its ContentsView at (0,0) in Ash,
but other window managers (Mac and Linux with desktop widgets) are free move the
window.

(Also, using a different Widget type (e.g. TYPE_WINDOW) will add a titlebar
under Ash as well.)

So, for example, if the widget set up set a non-zero origin:

    params.bounds = gfx::Rect(100, 100, 650, 650);

The test as it stands will fail under Linux (non-desktop widgets).

That is, "MoveMouseToInHost(10, 10)" only works because the test case is
assuming that the window is at the origin of the root window, but really the
coordinates it _wants_ to use should be relative to the window's ContentView
(i.e. the CustomButton View).

So when TableViewTest now comes along and actually _does_ have a titlebar on its
Widget, it must rightly offset its coordinates on all platforms:
  gfx::Point GetPointForRow(int row) {
    const int y = (row + 0.5) * table_->row_height();
    return table_->GetBoundsInScreen().origin() + gfx::Vector2d(5, y);
  }


The problem with set_coordinates_relative_to_root_window(bool) is that
HideInkDropHighlightOnDisable would need a value of `false`, and that feels
misleading because the problem is that HideInkDropHighlightOnDisable *is*
sending coordinates relative to the root window - not that it isn't.



[1]
https://cs.chromium.org/chromium/src/ui/views/controls/button/custom_button_u...

+    assume_window_at_origin_ = assume_window_at_origin;
+  }
+  bool assume_window_at_origin() { return assume_window_at_origin_; }
+
   // Generates a left button press event.
   void PressLeftButton();
 
@@ -446,15 +460,23 @@ class EventGenerator {
 
   std::unique_ptr<EventGeneratorDelegate> delegate_;
   gfx::Point current_location_;
-  EventTarget* current_target_;
-  int flags_;
-  bool grab_;
+  EventTarget* current_target_ = nullptr;
+  int flags_ = 0;
+  bool grab_ = false;
+
   ui::PointerDetails touch_pointer_details_;
 
   std::list<std::unique_ptr<Event>> pending_events_;
+
   // Set to true to cause events to be posted asynchronously.
-  bool async_;
-  Target target_;
+  bool async_ = false;
+
+  // Whether to skip mapping of coordinates from the root window to a hit window
+  // when dispatching events.
+  bool assume_window_at_origin_ = true;

[tapted, 2017/04/05 04:54:17]

alternatively.. we could default this to `false` and update the ~8 test
harnesses that are relying on it -- see http://crbug.com/708401

+
+  Target target_ = Target::WIDGET;
+
   std::unique_ptr<base::TickClock> tick_clock_;
 
   DISALLOW_COPY_AND_ASSIGN(EventGenerator);