Improve TableView unit tests and fix bugs that were uncovered.

ui/views/controls/table/table_view_unittest.cc

Index: ui/views/controls/table/table_view_unittest.cc
diff --git a/ui/views/controls/table/table_view_unittest.cc b/ui/views/controls/table/table_view_unittest.cc
index 5c71032e61d3a05b052ff3180785be4c1ebe685b..e8ea245072954fc19656a0667cd0ba992b583b73 100644
--- a/ui/views/controls/table/table_view_unittest.cc
+++ b/ui/views/controls/table/table_view_unittest.cc
@@ -11,9 +11,11 @@
 #include "base/strings/utf_string_conversions.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/events/event_utils.h"
+#include "ui/events/test/event_generator.h"
 #include "ui/views/controls/table/table_grouper.h"
 #include "ui/views/controls/table/table_header.h"
 #include "ui/views/controls/table/table_view_observer.h"
+#include "ui/views/test/views_test_base.h"
 
 // Put the tests in the views namespace to make it easier to declare them as
 // friend classes.
@@ -41,10 +43,6 @@ class TableViewTestHelper {
     table_->SetSelectionModel(new_selection);
   }
 
-  void OnFocus() {
-    table_->OnFocus();
-  }
-
  private:
   TableView* table_;
 
@@ -184,30 +182,15 @@ std::string GetRowsInViewOrderAsString(TableView* table) {
   return result;
 }
 
-class TestTableView : public TableView {
- public:
-  TestTableView(ui::TableModel* model,
-                const std::vector<ui::TableColumn>& columns)
-      : TableView(model, columns, TEXT_ONLY, false) {
-  }
-
-  // View overrides:
-  bool HasFocus() const override {
-    // Overriden so key processing works.
-    return true;
-  }
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(TestTableView);
-};
-
 }  // namespace
 
-class TableViewTest : public testing::Test {
+class TableViewTest : public ViewsTestBase {
  public:
   TableViewTest() : table_(NULL) {}
 
   void SetUp() override {
+    ViewsTestBase::SetUp();
+
     model_.reset(new TestTableModel2);
     std::vector<ui::TableColumn> columns(2);
     columns[0].title = base::ASCIIToUTF16("Title Column 0");
@@ -215,27 +198,36 @@ class TableViewTest : public testing::Test {
     columns[1].title = base::ASCIIToUTF16("Title Column 1");
     columns[1].id = 1;
     columns[1].sortable = true;
-    table_ = new TestTableView(model_.get(), columns);
-    parent_.reset(table_->CreateParentIfNecessary());
-    parent_->SetBounds(0, 0, 10000, 10000);
-    parent_->Layout();
+    table_ = new TableView(model_.get(), columns, TEXT_ONLY, false);
+    View* parent = table_->CreateParentIfNecessary();
+    parent->SetBounds(0, 0, 10000, 10000);
+    parent->Layout();
     helper_.reset(new TableViewTestHelper(table_));
+
+    Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_WINDOW);
+    params.ownership = views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
+    params.bounds = gfx::Rect(0, 0, 650, 650);

[tapted, 2017/04/04 00:07:36]

Offsetting the (x,y) coordinate here might resolve the issues. E.g. use (100,
100, 650, 650); (but that might throw off coordinates elsewhere in the tests).

Alternatively, I think Widget::InitParams::TYPE_WINDOW_FRAMELESS above will work
too (perhaps better, but changing the Widget type can have other effects around
activation and things).

On Mac, when you Show() a real Widget (with a titlebar, etc.) at (0,0), the
window manager moves it into the work area (below menu bar, or possibly a Dock
hugging the left of the screen). Anticipating the move, or using a window type
that the window manager doesn't move should work around that.

If you're still stuck, I might have a chance later to look into it more.. I
should be able to detect this situation in the event generator and warn about it
too.. (sadly auto-correcting for it turned out to be a dead-end since tests
sometimes do and sometimes don't convert from root window coordinates to Widget
coordinates -- so some tests will break if we auto-correct).

+    widget_.reset(new Widget);
+    widget_->Init(params);
+    widget_->GetContentsView()->AddChildView(parent);
+    widget_->Show();
+  }
+
+  void TearDown() override {
+    widget_.reset();
+    ViewsTestBase::TearDown();
   }
 
   void ClickOnRow(int row, int flags) {
-    const int y = row * table_->row_height();
-    const ui::MouseEvent pressed(ui::ET_MOUSE_PRESSED, gfx::Point(0, y),
-                                 gfx::Point(0, y), ui::EventTimeForNow(),
-                                 ui::EF_LEFT_MOUSE_BUTTON | flags,
-                                 ui::EF_LEFT_MOUSE_BUTTON);
-    table_->OnMousePressed(pressed);
+    ui::test::EventGenerator generator(widget_->GetNativeWindow());
+    generator.set_flags(flags);
+    generator.set_current_location(GetPointForRow(row));
+    generator.PressLeftButton();
   }
 
   void TapOnRow(int row) {
-    const int y = row * table_->row_height();
-    const ui::GestureEventDetails event_details(ui::ET_GESTURE_TAP);
-    ui::GestureEvent tap(0, y, 0, base::TimeTicks(), event_details);
-    table_->OnGestureEvent(&tap);
+    ui::test::EventGenerator generator(widget_->GetNativeWindow());
+    generator.GestureTapAt(GetPointForRow(row));
   }
 
   // Returns the state of the selection model as a string. The format is:
@@ -256,8 +248,8 @@ class TableViewTest : public testing::Test {
   }
 
   void PressKey(ui::KeyboardCode code) {
-    ui::KeyEvent event(ui::ET_KEY_PRESSED, code, ui::EF_NONE);
-    table_->OnKeyPressed(event);
+    ui::test::EventGenerator generator(widget_->GetNativeWindow());
+    generator.PressKey(code, ui::EF_NONE);
   }
 
  protected:
@@ -269,7 +261,12 @@ class TableViewTest : public testing::Test {
   std::unique_ptr<TableViewTestHelper> helper_;
 
  private:
-  std::unique_ptr<View> parent_;
+  gfx::Point GetPointForRow(int row) {
+    const int y = (row + 0.5) * table_->row_height();
+    return table_->GetBoundsInScreen().origin() + gfx::Vector2d(5, y);
+  }
+
+  std::unique_ptr<Widget> widget_;
 
   DISALLOW_COPY_AND_ASSIGN(TableViewTest);
 };
@@ -735,7 +732,7 @@ TEST_F(TableViewTest, SelectOnTap) {
   TableViewObserverImpl observer;
   table_->set_observer(&observer);
 
-  // Click on the first row, should select it.
+  // Tap on the first row, should select it.
   TapOnRow(0);
   EXPECT_EQ(1, observer.GetChangedCountAndClear());
   EXPECT_EQ("active=0 anchor=0 selection=0", SelectionStateAsString());
@@ -762,6 +759,7 @@ TEST_F(TableViewTest, KeyUpDown) {
 
   TableViewObserverImpl observer;
   table_->set_observer(&observer);
+  table_->RequestFocus();
 
   // Initially no selection.
   EXPECT_EQ("active=-1 anchor=-1 selection=", SelectionStateAsString());
@@ -892,6 +890,7 @@ TEST_F(TableViewTest, HomeEnd) {
 
   TableViewObserverImpl observer;
   table_->set_observer(&observer);
+  table_->RequestFocus();
 
   // Initially no selection.
   EXPECT_EQ("active=-1 anchor=-1 selection=", SelectionStateAsString());
@@ -904,6 +903,10 @@ TEST_F(TableViewTest, HomeEnd) {
   EXPECT_EQ(1, observer.GetChangedCountAndClear());
   EXPECT_EQ("active=4 anchor=4 selection=3 4", SelectionStateAsString());
 
+  PressKey(ui::VKEY_HOME);
+  EXPECT_EQ(1, observer.GetChangedCountAndClear());
+  EXPECT_EQ("active=0 anchor=0 selection=0 1", SelectionStateAsString());
+
   table_->set_observer(NULL);
 }
 
@@ -1020,31 +1023,7 @@ TEST_F(TableViewTest, FocusAfterRemovingAnchor) {
   new_selection.set_anchor(0);
   helper_->SetSelectionModel(new_selection);
   model_->RemoveRow(0);
-  helper_->OnFocus();
-}
-
-// Tests that focusing the table will activate the first row, but only if
-// there's no active row.
-TEST_F(TableViewTest, InitialFocusActivatesFirstRow) {
-  EXPECT_EQ(-1, table_->selection_model().active());
-  helper_->OnFocus();
-  EXPECT_EQ(0, table_->selection_model().active());
-
-  ui::ListSelectionModel new_selection;
-  new_selection.set_active(1);
-  helper_->SetSelectionModel(new_selection);
-  EXPECT_EQ(1, table_->selection_model().active());
-  helper_->OnFocus();
-  EXPECT_EQ(1, table_->selection_model().active());
-
-  // Remove all rows; focusing should not activate a non existent first row.
-  while (model_->RowCount())
-    model_->RemoveRow(0);
-
-  new_selection.set_active(-1);
-  helper_->SetSelectionModel(new_selection);
-  helper_->OnFocus();
-  EXPECT_EQ(-1, table_->selection_model().active());
+  table_->RequestFocus();
 }
 
 }  // namespace views