Handle shift-selection in GroupTables.

Change the shift-click support in TableView so shift-clicks that span multiple
groups are prohibited. Clicking in one group, then shift-clicking in another
group will cause the shift-click to be treated as a normal click (i.e. just
select the item in the second group).

BUG=63787
TEST=Open task manager, click on item in one group, shift click on item in another group.

Review URL: http://codereview.chromium.org/5504001

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@68123 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 110 insertions, 21 deletions

diff --git a/views/controls/table/table_view.cc b/views/controls/table/table_view.cc
index cd7541a2..84c0538 100644
--- a/views/controls/table/table_view.cc
+++ b/views/controls/table/table_view.cc
@@ -680,13 +680,10 @@ LRESULT CALLBACK TableView::TableWndProc(HWND window,
               // Unselect everything.
               ListView_SetItemState(window, -1, 0, LVIS_SELECTED);
               select = false;
-
-              // Select from mark to mouse down location.
-              for (int i = std::min(view_index, mark_view_index),
-                   max_i = std::max(view_index, mark_view_index); i <= max_i;
-                   ++i) {
-                table_view->SetSelectedState(table_view->ViewToModel(i),
-                                             true);
+              if (!table_view->SelectMultiple(view_index, mark_view_index)) {
+                // Selection spans group boundary - reset selection to current.
+                table_view->SetSelectedState(model_index, true);
+                ListView_SetSelectionMark(window, view_index);
               }
             }
           }
@@ -918,6 +915,32 @@ void TableView::SortItemsAndUpdateMapping() {
   }
 }
 
+bool TableView::SelectMultiple(int view_index, int mark_view_index) {
+  int group_id = 0;
+  if (model_->HasGroups()) {
+    group_id = model_->GetGroupID(ViewToModel(view_index));
+    if (group_id != model_->GetGroupID(ViewToModel(mark_view_index))) {
+      // User is trying to do a cross-group selection - bail out.
+      return false;
+    }
+  }
+
+  // Select from mark to mouse down location.
+  for (int i = std::min(view_index, mark_view_index),
+       max_i = std::max(view_index, mark_view_index); i <= max_i;
+       ++i) {
+    // Items between the view_index and mark_view_index are not necessarily in
+    // the same group, so don't select anything outside the group the user
+    // just clicked in.
+    if (model_->HasGroups() &&
+        model_->GetGroupID(ViewToModel(i)) != group_id) {
+      continue;
+    }
+    SetSelectedState(ViewToModel(i), true);
+  }
+  return true;
+}
+
 // static
 int CALLBACK TableView::SortFunc(LPARAM model_index_1_p,
                                  LPARAM model_index_2_p,
diff --git a/views/controls/table/table_view.h b/views/controls/table/table_view.h
index eb557da..b8ae795 100644
--- a/views/controls/table/table_view.h
+++ b/views/controls/table/table_view.h
@@ -18,6 +18,7 @@ typedef struct tagNMLVCUSTOMDRAW NMLVCUSTOMDRAW;
 
 #include "app/keyboard_codes.h"
 #include "app/table_model_observer.h"
+#include "base/gtest_prod_util.h"
 #include "third_party/skia/include/core/SkColor.h"
 #if defined(OS_WIN)
 // TODO(port): remove the ifdef when native_control.h is ported.
@@ -323,6 +324,9 @@ class TableView : public NativeControl,
 
   friend class ListViewParent;
   friend class TableSelectionIterator;
+  friend class GroupModelTableViewTest;
+  FRIEND_TEST_ALL_PREFIXES(GroupModelTableViewTest, ShiftSelectAcrossGroups);
+  FRIEND_TEST_ALL_PREFIXES(GroupModelTableViewTest, ShiftSelectSameGroup);
 
   LRESULT OnCustomDraw(NMLVCUSTOMDRAW* draw_info);
 
@@ -340,6 +344,13 @@ class TableView : public NativeControl,
   // model_to_view) appropriately.
   void SortItemsAndUpdateMapping();
 
+  // Selects multiple items from the current view row to the marked view row
+  // (implements shift-click behavior). |view_index| is the most recent row
+  // that the user clicked on, and so there is no guarantee that
+  // |view_index| > |mark_view_index| or vice-versa. Returns false if the
+  // selection attempt was rejected because it crossed group boundaries.
+  bool SelectMultiple(int view_index, int mark_view_index);
+
   // Method invoked by ListView to compare the two values. Invokes CompareRows.
   static int CALLBACK SortFunc(LPARAM model_index_1_p,
                                LPARAM model_index_2_p,
diff --git a/views/controls/table/table_view_unittest.cc b/views/controls/table/table_view_unittest.cc
index c6954fa..2959534 100644
--- a/views/controls/table/table_view_unittest.cc
+++ b/views/controls/table/table_view_unittest.cc
@@ -19,7 +19,9 @@
 #include "views/window/window_gtk.h"
 #endif
 
-using views::TableView;
+// Put the tests in the views namespace to make it easier to declare them as
+// friend classes.
+namespace views {
 
 // TestTableModel --------------------------------------------------------------
 
@@ -59,6 +61,27 @@ class TestTableModel : public TableModel {
   DISALLOW_COPY_AND_ASSIGN(TestTableModel);
 };
 
+// Same behavior as TestTableModel, except even items are in one group, while
+// odd items are put in a different group.
+class GroupTestTableModel : public TestTableModel {
+  virtual bool HasGroups() { return true; }
+
+  virtual Groups GetGroups() {
+    Groups groups;
+    Group group1, group2;
+    group1.title = L"Group 1";
+    group1.id = 0;
+    group2.title = L"Group 2";
+    group2.id = 0;
+    groups.push_back(group1);
+    groups.push_back(group2);
+    return groups;
+  }
+
+  // Return group = 0 if row is even, otherwise group = 1.
+  virtual int GetGroupID(int row) { return row % 2; }
+};
+
 TestTableModel::TestTableModel() : observer_(NULL) {
   AddRow(0, 0, 1);
   AddRow(1, 1, 1);
@@ -120,12 +143,12 @@ class TableViewTest : public testing::Test, views::WindowDelegate {
 
  protected:
   // Creates the model.
-  TestTableModel* CreateModel();
+  virtual TestTableModel* CreateModel();
 
   // Verifies the view order matches that of the supplied arguments. The
   // arguments are in terms of the model. For example, values of '1, 0' indicate
   // the model index at row 0 is 1 and the model index at row 1 is 0.
-  void VeriyViewOrder(int first, ...);
+  void VerifyViewOrder(int first, ...);
 
   // Verifies the selection matches the supplied arguments. The supplied
   // arguments are in terms of this model. This uses the iterator returned by
@@ -168,7 +191,7 @@ void TableViewTest::TearDown() {
   OleUninitialize();
 }
 
-void TableViewTest::VeriyViewOrder(int first, ...) {
+void TableViewTest::VerifyViewOrder(int first, ...) {
   va_list marker;
   va_start(marker, first);
   int value = first;
@@ -209,7 +232,7 @@ void TableViewTest::SetUpMultiSelectTestState(bool sort) {
   TableView::SortDescriptors sd;
   sd.push_back(TableView::SortDescriptor(0, false));
   table_->SetSortDescriptors(sd);
-  VeriyViewOrder(2, 1, 0, -1);
+  VerifyViewOrder(2, 1, 0, -1);
   if (HasFatalFailure())
     return;
 
@@ -232,6 +255,14 @@ class NullModelTableViewTest : public TableViewTest {
   }
 };
 
+// GroupModelTableViewTest -----------------------------------------------------
+class GroupModelTableViewTest : public TableViewTest {
+ protected:
+  TestTableModel* CreateModel() {
+    return new GroupTestTableModel();
+  }
+};
+
 // Tests -----------------------------------------------------------------------
 
 // Failing: http://crbug.com/45015
@@ -241,7 +272,7 @@ TEST_F(TableViewTest, DISABLED_Sort) {
   TableView::SortDescriptors sort;
   sort.push_back(TableView::SortDescriptor(0, false));
   table_->SetSortDescriptors(sort);
-  VeriyViewOrder(2, 1, 0, -1);
+  VerifyViewOrder(2, 1, 0, -1);
   if (HasFatalFailure())
     return;
 
@@ -251,13 +282,13 @@ TEST_F(TableViewTest, DISABLED_Sort) {
   sort.push_back(TableView::SortDescriptor(0, false));
   sort[1].ascending = false;
   table_->SetSortDescriptors(sort);
-  VeriyViewOrder(1, 0, 2, -1);
+  VerifyViewOrder(1, 0, 2, -1);
   if (HasFatalFailure())
     return;
 
   // Clear the sort.
   table_->SetSortDescriptors(TableView::SortDescriptors());
-  VeriyViewOrder(0, 1, 2, -1);
+  VerifyViewOrder(0, 1, 2, -1);
   if (HasFatalFailure())
     return;
 }
@@ -269,12 +300,12 @@ TEST_F(TableViewTest, DISABLED_SortThenChange) {
   TableView::SortDescriptors sort;
   sort.push_back(TableView::SortDescriptor(0, false));
   table_->SetSortDescriptors(sort);
-  VeriyViewOrder(2, 1, 0, -1);
+  VerifyViewOrder(2, 1, 0, -1);
   if (HasFatalFailure())
     return;
 
   model_->ChangeRow(0, 3, 1);
-  VeriyViewOrder(0, 2, 1, -1);
+  VerifyViewOrder(0, 2, 1, -1);
 }
 
 // Failing: http://crbug.com/45015
@@ -284,19 +315,19 @@ TEST_F(TableViewTest, DISABLED_AddToSorted) {
   TableView::SortDescriptors sort;
   sort.push_back(TableView::SortDescriptor(0, false));
   table_->SetSortDescriptors(sort);
-  VeriyViewOrder(2, 1, 0, -1);
+  VerifyViewOrder(2, 1, 0, -1);
   if (HasFatalFailure())
     return;
 
   // Add row so that it occurs first.
   model_->AddRow(0, 5, -1);
-  VeriyViewOrder(0, 3, 2, 1, -1);
+  VerifyViewOrder(0, 3, 2, 1, -1);
   if (HasFatalFailure())
     return;
 
   // Add row so that it occurs last.
   model_->AddRow(0, -1, -1);
-  VeriyViewOrder(1, 4, 3, 2, 0, -1);
+  VerifyViewOrder(1, 4, 3, 2, 0, -1);
 }
 
 // Failing: http://crbug.com/45015
@@ -309,7 +340,7 @@ TEST_F(TableViewTest, DISABLED_PersistSelectionOnSort) {
   TableView::SortDescriptors sort;
   sort.push_back(TableView::SortDescriptor(0, false));
   table_->SetSortDescriptors(sort);
-  VeriyViewOrder(2, 1, 0, -1);
+  VerifyViewOrder(2, 1, 0, -1);
   if (HasFatalFailure())
     return;
 
@@ -395,6 +426,28 @@ TEST_F(TableViewTest, DISABLED_PersistMultiSelectionOnAdd) {
   VerifySelectedRows(1, 0, -1);
 }
 
+TEST_F(GroupModelTableViewTest, IndividualSelectAcrossGroups) {
+  table_->SetSelectedState(0, true);
+  table_->SetSelectedState(1, true);
+  table_->SetSelectedState(2, true);
+  VerifySelectedRows(2, 1, 0, -1);
+}
+
+TEST_F(GroupModelTableViewTest, ShiftSelectAcrossGroups) {
+  table_->SetSelectedState(0, true);
+  // Try to select across groups - this should fail.
+  ASSERT_FALSE(table_->SelectMultiple(1, 0));
+  VerifySelectedRows(0, -1);
+}
+
+TEST_F(GroupModelTableViewTest, ShiftSelectSameGroup) {
+  table_->SetSelectedState(0, true);
+  // Try to select in the same group - this should work but should only select
+  // items in the "even" group.
+  ASSERT_TRUE(table_->SelectMultiple(2, 0));
+  VerifySelectedRows(2, 0, -1);
+}
+
 // Crashing: http://crbug.com/45015
 TEST_F(NullModelTableViewTest, DISABLED_NullModel) {
   // There's nothing explicit to test. If there is a bug in TableView relating
@@ -572,3 +625,5 @@ TEST_F(TableView2Test, RowFocusTest) {
   EXPECT_EQ(-1, table_->GetFirstSelectedRow());
 }
 #endif
+
+}  // namespace views