Fix keyboard navigation in app launcher (OSX)

This causes left/right to page flip on boundary and up/down to do nothing. This
patch also makes out of bounds selections on the last page snap to the last
available item instead of not switching page.

This patch updates the OSX app launcher to be consistent with revision 200812.

BUG=237092

Review URL: https://chromiumcodereview.appspot.com/15648003

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

2 files changed, 66 insertions, 55 deletions

diff --git a/ui/app_list/cocoa/apps_grid_controller.mm b/ui/app_list/cocoa/apps_grid_controller.mm
index 795979a..6a47ed4 100644
--- a/ui/app_list/cocoa/apps_grid_controller.mm
+++ b/ui/app_list/cocoa/apps_grid_controller.mm
@@ -603,9 +603,18 @@ class AppsGridDelegateBridge : public ui::ListModelObserver {
     return YES;
   }
 
-  if ((indexDelta < 0 && static_cast<NSUInteger>(-indexDelta) > oldIndex) ||
-      oldIndex + indexDelta >= [items_ count]) {
+  // Can't select a negative index.
+  if (indexDelta < 0 && static_cast<NSUInteger>(-indexDelta) > oldIndex)
     return NO;
+
+  // Can't select an index greater or equal to the number of items.
+  if (oldIndex + indexDelta >= [items_ count]) {
+    if (visiblePage_ == [pages_ count] - 1)
+      return NO;
+
+    // If we're not on the last page, then select the last item.
+    [self selectItemAtIndex:[items_ count] - 1];
+    return YES;
   }
 
   [self selectItemAtIndex:oldIndex + indexDelta];
@@ -629,17 +638,34 @@ class AppsGridDelegateBridge : public ui::ListModelObserver {
     return YES;
   }
 
-  if (command == @selector(moveLeft:))
-    return [self moveSelectionByDelta:-1];
+  NSUInteger oldIndex = [self selectedItemIndex];
+  // If nothing is currently selected, select the first item on the page.
+  if (oldIndex == NSNotFound) {
+    [self selectItemAtIndex:visiblePage_ * kItemsPerPage];
+    return YES;
+  }
+
+  if (command == @selector(moveLeft:)) {
+    return oldIndex % kFixedColumns == 0 ?
+        [self moveSelectionByDelta:-kItemsPerPage + kFixedColumns - 1] :
+        [self moveSelectionByDelta:-1];
+  }
 
-  if (command == @selector(moveRight:))
-    return [self moveSelectionByDelta:1];
+  if (command == @selector(moveRight:)) {
+    return oldIndex % kFixedColumns == kFixedColumns - 1 ?
+        [self moveSelectionByDelta:+kItemsPerPage - kFixedColumns + 1] :
+        [self moveSelectionByDelta:1];
+  }
 
-  if (command == @selector(moveUp:))
-    return [self moveSelectionByDelta:-kFixedColumns];
+  if (command == @selector(moveUp:)) {
+    return oldIndex / kFixedColumns % kFixedRows == 0 ?
+        NO : [self moveSelectionByDelta:-kFixedColumns];
+  }
 
-  if (command == @selector(moveDown:))
-    return [self moveSelectionByDelta:kFixedColumns];
+  if (command == @selector(moveDown:)) {
+    return oldIndex / kFixedColumns % kFixedRows == kFixedRows - 1 ?
+        NO : [self moveSelectionByDelta:kFixedColumns];
+  }
 
   if (command == @selector(pageUp:) ||
       command == @selector(scrollPageUp:))
diff --git a/ui/app_list/cocoa/apps_grid_controller_unittest.mm b/ui/app_list/cocoa/apps_grid_controller_unittest.mm
index 43cd5e7..fe52e8c 100644
--- a/ui/app_list/cocoa/apps_grid_controller_unittest.mm
+++ b/ui/app_list/cocoa/apps_grid_controller_unittest.mm
@@ -260,9 +260,9 @@ TEST_F(AppsGridControllerTest, FirstPageKeyboardNavigation) {
 
 // Tests keyboard navigation across pages.
 TEST_F(AppsGridControllerTest, CrossPageKeyboardNavigation) {
-  model()->PopulateApps(2 * kItemsPerPage);
+  model()->PopulateApps(kItemsPerPage + 10);
   EXPECT_EQ(kItemsPerPage, [[GetPageAt(0) content] count]);
-  EXPECT_EQ(kItemsPerPage, [[GetPageAt(1) content] count]);
+  EXPECT_EQ(10u, [[GetPageAt(1) content] count]);
 
   // Moving Left, Up, or PageUp from the top-left corner of the first page does
   // nothing.
@@ -274,77 +274,62 @@ TEST_F(AppsGridControllerTest, CrossPageKeyboardNavigation) {
   SimulateKeyAction(@selector(scrollPageUp:));
   EXPECT_EQ(0u, [apps_grid_controller_ selectedItemIndex]);
 
-  // Moving Right from the right side wraps to the next row.
+  // Moving Right from the right side goes to the next page. Moving Left goes
+  // back to the first page.
   [apps_grid_controller_ selectItemAtIndex:3];
   SimulateKeyAction(@selector(moveRight:));
-  EXPECT_EQ(4u, [apps_grid_controller_ selectedItemIndex]);
+  EXPECT_EQ(1u, [apps_grid_controller_ visiblePage]);
+  EXPECT_EQ(kItemsPerPage, [apps_grid_controller_ selectedItemIndex]);
+  SimulateKeyAction(@selector(moveLeft:));
+  EXPECT_EQ(0u, [apps_grid_controller_ visiblePage]);
+  EXPECT_EQ(3u, [apps_grid_controller_ selectedItemIndex]);
 
-  // Moving Down from the bottom goes to the next page and selects the same
-  // column. Moving Up again goes back to the first page.
+  // Moving Down from the bottom does nothing.
   [apps_grid_controller_ selectItemAtIndex:13];
   EXPECT_EQ(0u, [apps_grid_controller_ visiblePage]);
   SimulateKeyAction(@selector(moveDown:));
-  EXPECT_EQ(1u, [apps_grid_controller_ visiblePage]);
-  EXPECT_EQ(kItemsPerPage + 1, [apps_grid_controller_ selectedItemIndex]);
-  SimulateKeyAction(@selector(moveUp:));
-  EXPECT_EQ(0u, [apps_grid_controller_ visiblePage]);
   EXPECT_EQ(13u, [apps_grid_controller_ selectedItemIndex]);
 
-  // Moving Right from the bottom-right corner goes to the next page.
-  // Moving Left again goes back to the first page.
+  // Moving Right into a non-existent square on the next page will select the
+  // last item.
   [apps_grid_controller_ selectItemAtIndex:15];
   EXPECT_EQ(0u, [apps_grid_controller_ visiblePage]);
   SimulateKeyAction(@selector(moveRight:));
   EXPECT_EQ(1u, [apps_grid_controller_ visiblePage]);
-  EXPECT_EQ(kItemsPerPage, [apps_grid_controller_ selectedItemIndex]);
-  SimulateKeyAction(@selector(moveLeft:));
-  EXPECT_EQ(0u, [apps_grid_controller_ visiblePage]);
-  EXPECT_EQ(15u, [apps_grid_controller_ selectedItemIndex]);
+  EXPECT_EQ(kItemsPerPage + 9, [apps_grid_controller_ selectedItemIndex]);
 
   // PageDown and PageUp switches pages while maintaining the same selection
   // position.
-  [apps_grid_controller_ selectItemAtIndex:10];
+  [apps_grid_controller_ selectItemAtIndex:6];
   EXPECT_EQ(0u, [apps_grid_controller_ visiblePage]);
   SimulateKeyAction(@selector(scrollPageDown:));
   EXPECT_EQ(1u, [apps_grid_controller_ visiblePage]);
-  EXPECT_EQ(kItemsPerPage + 10, [apps_grid_controller_ selectedItemIndex]);
+  EXPECT_EQ(kItemsPerPage + 6, [apps_grid_controller_ selectedItemIndex]);
   SimulateKeyAction(@selector(scrollPageUp:));
   EXPECT_EQ(0u, [apps_grid_controller_ visiblePage]);
-  EXPECT_EQ(10u, [apps_grid_controller_ selectedItemIndex]);
+  EXPECT_EQ(6u, [apps_grid_controller_ selectedItemIndex]);
 
-  // Moving Right, Down, or PageDown from the bottom-right corner of the last
-  // page does nothing.
-  [apps_grid_controller_ selectItemAtIndex:(2 * kItemsPerPage - 1)];
-  EXPECT_EQ(1u, [apps_grid_controller_ visiblePage]);
-  SimulateKeyAction(@selector(moveRight:));
-  EXPECT_EQ(kItemsPerPage + 15, [apps_grid_controller_ selectedItemIndex]);
-  SimulateKeyAction(@selector(moveDown:));
-  EXPECT_EQ(kItemsPerPage + 15, [apps_grid_controller_ selectedItemIndex]);
+  // PageDown into a non-existent square on the next page will select the last
+  // item.
+  [apps_grid_controller_ selectItemAtIndex:11];
+  EXPECT_EQ(0u, [apps_grid_controller_ visiblePage]);
   SimulateKeyAction(@selector(scrollPageDown:));
-  EXPECT_EQ(kItemsPerPage + 15, [apps_grid_controller_ selectedItemIndex]);
+  EXPECT_EQ(1u, [apps_grid_controller_ visiblePage]);
+  EXPECT_EQ(kItemsPerPage + 9, [apps_grid_controller_ selectedItemIndex]);
 
-  // Moving down on the bottom of the last page does nothing.
-  [apps_grid_controller_ selectItemAtIndex:(2 * kItemsPerPage - 4)];
+  // Moving Right, Down, or PageDown from the bottom-right corner of the last
+  // page (not the last item) does nothing.
+  [apps_grid_controller_ selectItemAtIndex:kItemsPerPage + 9];
   EXPECT_EQ(1u, [apps_grid_controller_ visiblePage]);
+  SimulateKeyAction(@selector(moveRight:));
+  EXPECT_EQ(kItemsPerPage + 9, [apps_grid_controller_ selectedItemIndex]);
   SimulateKeyAction(@selector(moveDown:));
-  EXPECT_EQ(kItemsPerPage + 12, [apps_grid_controller_ selectedItemIndex]);
-
-  // Remove some items from the last page.
-  ReplaceTestModel(kItemsPerPage + 8);
-  EXPECT_EQ(8u, [[GetPageAt(1) content] count]);
-  EXPECT_EQ(NSNotFound, [apps_grid_controller_ selectedItemIndex]);
-
-  // PageUp/Down to an invalid selection does nothing.
-  [apps_grid_controller_ selectItemAtIndex:10];
-  EXPECT_EQ(0u, [apps_grid_controller_ visiblePage]);
-  SimulateKeyAction(@selector(scrollPageUp:));
-  EXPECT_EQ(0u, [apps_grid_controller_ visiblePage]);
-  EXPECT_EQ(10u, [apps_grid_controller_ selectedItemIndex]);
+  EXPECT_EQ(kItemsPerPage + 9, [apps_grid_controller_ selectedItemIndex]);
   SimulateKeyAction(@selector(scrollPageDown:));
-  EXPECT_EQ(0u, [apps_grid_controller_ visiblePage]);
-  EXPECT_EQ(10u, [apps_grid_controller_ selectedItemIndex]);
+  EXPECT_EQ(kItemsPerPage + 9, [apps_grid_controller_ selectedItemIndex]);
 
   // After page switch, arrow keys select first item on current page.
+  [apps_grid_controller_ scrollToPage:0];
   [apps_grid_controller_ scrollToPage:1];
   EXPECT_EQ(NSNotFound, [apps_grid_controller_ selectedItemIndex]);
   SimulateKeyAction(@selector(moveUp:));