[iOS] Add item reordering support to GridViewController

This CL adds support (via a gesture recognizer) for long-press-drag
movement of grid cells to GridViewController.

The cell movement gets the appropriate delegate plumbing so it can be
fed back to the mediator.

The GridViewController's consumer implementation is tweaked so that
cell movement caused by user interaction doesn't trigger a collection
view update (since that update has already happened as part of the move).

Bug: 804592
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Ic8ba431d598dfee39f4a72f36f02a3d9ab52443d
Reviewed-on: https://chromium-review.googlesource.com/986379
Commit-Queue: Mark Cogan <marq@chromium.org>
Reviewed-by: Stepan Khapugin <stkhapugin@chromium.org>
Reviewed-by: Mark Cogan <marq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549556}

ios/chrome/browser/ui/tab_grid/grid/grid_commands.h

@@ -15,6 +15,10 @@
 // Tells the receiver to select the item with identifier |itemID|. If there is
 // no item with that identifier, no change in selection should be made.
 - (void)selectItemWithID:(NSString*)itemID;
+// Tells the receiver to move the item with identifier |itemID| to |index|. If
+// there is no item with that identifier, no move should be made. It is an error
+// to pass a value for |index| outside of the bounds of the items array.
+- (void)moveItemWithID:(NSString*)itemID toIndex:(NSUInteger)index;
 // Tells the receiver to close the item with identifier |itemID|. If there is
 // no item with that identifier, no item is closed.
 - (void)closeItemWithID:(NSString*)itemID;

ios/chrome/browser/ui/tab_grid/grid/grid_view_controller.h

@@ -24,6 +24,11 @@
 // |gridViewController|.
 - (void)gridViewController:(GridViewController*)gridViewController
         didCloseItemWithID:(NSString*)itemID;
+// Tells the delegate that the item at |sourceIndex| was moved to
+// |destinationIndex|.
+- (void)gridViewController:(GridViewController*)gridViewController
+         didMoveItemWithID:(NSString*)itemID
+                   toIndex:(NSUInteger)destinationIndex;
 // Tells the delegate that the the number of items in |gridViewController|
 // changed to |count|.
 - (void)gridViewController:(GridViewController*)gridViewController

ios/chrome/browser/ui/tab_grid/grid/grid_view_controller.mm

@@ -40,6 +40,9 @@ NSIndexPath* CreateIndexPath(NSInteger index) {
 @property(nonatomic, copy) NSString* selectedItemID;
 // Index of the selected item in |items|.
 @property(nonatomic, readonly) NSUInteger selectedIndex;
+// The gesture recognizer used for interactive item reordering.
+@property(nonatomic, strong)
+    UILongPressGestureRecognizer* itemReorderRecognizer;
 @end
 
 @implementation GridViewController
@@ -51,6 +54,7 @@ NSIndexPath* CreateIndexPath(NSInteger index) {
 @synthesize collectionView = _collectionView;
 @synthesize items = _items;
 @synthesize selectedItemID = _selectedItemID;
+@synthesize itemReorderRecognizer = _itemReorderRecognizer;
 
 - (instancetype)init {
   if (self = [super init]) {
@@ -72,6 +76,16 @@ NSIndexPath* CreateIndexPath(NSInteger index) {
   if (@available(iOS 11, *))
     collectionView.contentInsetAdjustmentBehavior =
         UIScrollViewContentInsetAdjustmentAlways;
+  self.itemReorderRecognizer = [[UILongPressGestureRecognizer alloc]
+      initWithTarget:self
+              action:@selector(handleItemReorderingWithGesture:)];
+  // The collection view cells will by default get touch events in parallel with
+  // the reorder recognizer. When this happens, long-pressing on a non-selected
+  // cell will cause the selected cell to briefly become unselected and then
+  // selected again. To avoid this, the recognizer delays touchesBegan: calls
+  // until it fails to recognize a long-press.
+  self.itemReorderRecognizer.delaysTouchesBegan = YES;
+  [collectionView addGestureRecognizer:self.itemReorderRecognizer];
   self.collectionView = collectionView;
   self.view = collectionView;
 }
@@ -180,6 +194,28 @@ NSIndexPath* CreateIndexPath(NSInteger index) {
   return cell;
 }
 
+- (BOOL)collectionView:(UICollectionView*)collectionView
+    canMoveItemAtIndexPath:(NSIndexPath*)indexPath {
+  return indexPath && self.items.count > 1;
+}
+
+- (void)collectionView:(UICollectionView*)collectionView
+    moveItemAtIndexPath:(NSIndexPath*)sourceIndexPath
+            toIndexPath:(NSIndexPath*)destinationIndexPath {
+  NSUInteger source = base::checked_cast<NSUInteger>(sourceIndexPath.item);
+  NSUInteger destination =
+      base::checked_cast<NSUInteger>(destinationIndexPath.item);
+  // Update |items| before informing the delegate, so the state of the UI
+  // is correctly represented before any updates occur.
+  GridItem* item = self.items[source];
+  [self.items removeObjectAtIndex:source];
+  [self.items insertObject:item atIndex:destination];
+
+  [self.delegate gridViewController:self
+                  didMoveItemWithID:item.identifier
+                            toIndex:destination];
+}
+
 #pragma mark - UICollectionViewDelegate
 
 - (void)collectionView:(UICollectionView*)collectionView
@@ -312,11 +348,17 @@ NSIndexPath* CreateIndexPath(NSInteger index) {
 
 - (void)moveItemWithID:(NSString*)itemID toIndex:(NSUInteger)toIndex {
   NSUInteger fromIndex = [self indexOfItemWithID:itemID];
+  // If this move would be a no-op, early return and avoid spurious UI updates.
+  if (fromIndex == toIndex)
+    return;
+
   auto performDataSourceUpdates = ^{
     GridItem* item = self.items[fromIndex];
     [self.items removeObjectAtIndex:fromIndex];
     [self.items insertObject:item atIndex:toIndex];
   };
+  // If the view isn't visible, there's no need for the collection view to
+  // update.
   if (![self isViewVisible]) {
     performDataSourceUpdates();
     return;
@@ -342,7 +384,7 @@ NSIndexPath* CreateIndexPath(NSInteger index) {
   return [self indexOfItemWithID:self.selectedItemID];
 }
 
-#pragma mark - Private properties
+#pragma mark - Private
 
 // Returns the index in |self.items| of the first item whose identifier is
 // |identifier|.
@@ -355,9 +397,7 @@ NSIndexPath* CreateIndexPath(NSInteger index) {
 
 // If the view is not visible, there is no need to update the collection view.
 - (BOOL)isViewVisible {
-  // Invoking the view method causes the view to load (if it is not loaded)
-  // which is unnecessary.
-  return (self.isViewLoaded && self.view.window);
+  return self.viewIfLoaded.window != nil;
 }
 
 // Animates the empty state into view.
@@ -382,4 +422,36 @@ NSIndexPath* CreateIndexPath(NSInteger index) {
                    completion:nil];
 }
 
+// Handle the long-press gesture used to reorder cells in the collection view.
+- (void)handleItemReorderingWithGesture:(UIGestureRecognizer*)gesture {
+  DCHECK(gesture == self.itemReorderRecognizer);
+  CGPoint location = [gesture locationInView:self.collectionView];
+  switch (gesture.state) {
+    case UIGestureRecognizerStateBegan: {
+      NSIndexPath* path =
+          [self.collectionView indexPathForItemAtPoint:location];
+      BOOL moving =
+          [self.collectionView beginInteractiveMovementForItemAtIndexPath:path];
+      if (!moving) {
+        gesture.enabled = NO;
+      }
+      break;
+    }
+    case UIGestureRecognizerStateChanged:
+      [self.collectionView updateInteractiveMovementTargetPosition:location];
+      break;
+    case UIGestureRecognizerStateEnded:
+      [self.collectionView endInteractiveMovement];
+      break;
+    case UIGestureRecognizerStateCancelled:
+      [self.collectionView cancelInteractiveMovement];
+      // Re-enable cancelled gesture.
+      gesture.enabled = YES;
+      break;
+    case UIGestureRecognizerStatePossible:
+    case UIGestureRecognizerStateFailed:
+      NOTREACHED() << "Unexpected long-press recognizer state";
+  }
+}
+
 @end

ios/chrome/browser/ui/tab_grid/grid/grid_view_controller_unittest.mm

@@ -42,6 +42,12 @@
   // No-op for unittests. This is only called when a user taps on a cell, not
   // generically when selectedIndex is updated.
 }
+- (void)gridViewController:(GridViewController*)gridViewController
+         didMoveItemWithID:(NSString*)itemID
+                   toIndex:(NSUInteger)destinationIndex {
+  // No-op for unittests. This is only called when a user interactively moves
+  // an item, not generically when items are moved in the data source.
+}
 - (void)gridViewController:(GridViewController*)gridViewController
         didCloseItemWithID:(NSString*)itemID {
   // No-op for unittests. This is only called when a user taps to close a cell,

ios/chrome/browser/ui/tab_grid/tab_grid_mediator.mm

@@ -209,6 +209,12 @@ int GetIndexOfTabWithId(WebStateList* webStateList, NSString* identifier) {
   self.webStateList->GetWebStateAt(index)->OpenURL(openParams);
 }
 
+- (void)moveItemWithID:(NSString*)itemID toIndex:(NSUInteger)destinationIndex {
+  int sourceIndex = GetIndexOfTabWithId(self.webStateList, itemID);
+  if (sourceIndex >= 0)
+    self.webStateList->MoveWebStateAt(sourceIndex, destinationIndex);
+}
+
 - (void)selectItemWithID:(NSString*)itemID {
   int index = GetIndexOfTabWithId(self.webStateList, itemID);
   if (index >= 0)

ios/chrome/browser/ui/tab_grid/tab_grid_mediator_unittest.mm

@@ -148,6 +148,8 @@ class TabGridMediatorTest : public PlatformTest {
   NSString* original_selected_identifier_;
 };
 
+#pragma mark - Consumer tests
+
 // Tests that the consumer is populated after the tab model is set on the
 // mediator.
 TEST_F(TabGridMediatorTest, ConsumerPopulateItems) {
@@ -216,35 +218,44 @@ TEST_F(TabGridMediatorTest, ConsumerMoveItem) {
   EXPECT_NSEQ(item2, consumer_.items[1]);
 }
 
+#pragma mark - Command tests
+
 // Tests that the active index is updated when |-selectItemWithID:| is called.
+// Tests that the consumer's selected index is updated.
 TEST_F(TabGridMediatorTest, SelectItemCommand) {
   // Previous selected index is 1.
   NSString* identifier =
       TabIdTabHelper::FromWebState(web_state_list_->GetWebStateAt(2))->tab_id();
   [mediator_ selectItemWithID:identifier];
   EXPECT_EQ(2, web_state_list_->active_index());
+  EXPECT_NSEQ(identifier, consumer_.selectedItemID);
 }
 
 // Tests that the |web_state_list_| count is decremented when
 // |-closeItemWithID:| is called.
+// Tests that the consumer's item count is also decremented.
 TEST_F(TabGridMediatorTest, CloseItemCommand) {
   // Previously there were 3 items.
   NSString* identifier =
       TabIdTabHelper::FromWebState(web_state_list_->GetWebStateAt(0))->tab_id();
   [mediator_ closeItemWithID:identifier];
   EXPECT_EQ(2, web_state_list_->count());
+  EXPECT_EQ(2UL, consumer_.items.count);
 }
 
 // Tests that the |web_state_list_| is empty when |-closeAllItems| is called.
+// Tests that the consumer's item list is also empty.
 TEST_F(TabGridMediatorTest, CloseAllItemsCommand) {
   // Previously there were 3 items.
   [mediator_ closeAllItems];
   EXPECT_EQ(0, web_state_list_->count());
+  EXPECT_EQ(0UL, consumer_.items.count);
 }
 
 // Tests that when |-addNewItem| is called, the |web_state_list_| count is
 // incremented, the |active_index| is the newly added index, the new web state
 // has no opener, and the URL is the new tab page.
+// Tests that the consumer has added an item with the correct identifier.
 TEST_F(TabGridMediatorTest, AddNewItemAtEndCommand) {
   // Previously there were 3 items and the selected index was 1.
   [mediator_ addNewItem];
@@ -259,11 +270,16 @@ TEST_F(TabGridMediatorTest, AddNewItemAtEndCommand) {
   EXPECT_EQ(kChromeUINewTabURL, request->params.url.spec());
   NSString* identifier = TabIdTabHelper::FromWebState(web_state)->tab_id();
   EXPECT_FALSE([original_identifiers_ containsObject:identifier]);
+  // Consumer checks.
+  EXPECT_EQ(4UL, consumer_.items.count);
+  EXPECT_NSEQ(identifier, consumer_.selectedItemID);
+  EXPECT_NSEQ(identifier, consumer_.items[3]);
 }
 
 // Tests that when |-insertNewItemAtIndex:| is called, the |web_state_list_|
 // count is incremented, the |active_index| is the newly added index, the new
 // web state has no opener, and the URL is the new tab page.
+// Checks that the consumer has added an item with the correct identifier.
 TEST_F(TabGridMediatorTest, InsertNewItemCommand) {
   // Previously there were 3 items and the selected index was 1.
   [mediator_ insertNewItemAtIndex:0];
@@ -278,4 +294,39 @@ TEST_F(TabGridMediatorTest, InsertNewItemCommand) {
   EXPECT_EQ(kChromeUINewTabURL, request->params.url.spec());
   NSString* identifier = TabIdTabHelper::FromWebState(web_state)->tab_id();
   EXPECT_FALSE([original_identifiers_ containsObject:identifier]);
+  // Consumer checks.
+  EXPECT_EQ(4UL, consumer_.items.count);
+  EXPECT_NSEQ(identifier, consumer_.selectedItemID);
+  EXPECT_NSEQ(identifier, consumer_.items[0]);
+}
+
+// Tests that when |-moveItemFromIndex:toIndex:| is called, there is no change
+// in the item count in |web_state_list_|, but that the constituent web states
+// have been reordered.
+TEST_F(TabGridMediatorTest, MoveItemCommand) {
+  // Capture ordered original IDs.
+  NSMutableArray<NSString*>* pre_move_ids = [[NSMutableArray alloc] init];
+  for (int i = 0; i < 3; i++) {
+    web::WebState* web_state = web_state_list_->GetWebStateAt(i);
+    [pre_move_ids addObject:TabIdTabHelper::FromWebState(web_state)->tab_id()];
+  }
+  NSString* pre_move_selected_id =
+      pre_move_ids[web_state_list_->active_index()];
+  // Items start ordered [A, B, C].
+  [mediator_ moveItemWithID:consumer_.items[0] toIndex:2];
+  // Items should now be ordered [B, C, A] -- the pre-move identifiers should
+  // still be in this order.
+  // Item count hasn't changed.
+  EXPECT_EQ(3, web_state_list_->count());
+  // Active index has moved -- it was 1, now 0.
+  EXPECT_EQ(0, web_state_list_->active_index());
+  // Identifier at 0, 1, 2 should match the original_identifier_ at 1, 2, 0.
+  for (int index = 0; index < 2; index++) {
+    web::WebState* web_state = web_state_list_->GetWebStateAt(index);
+    ASSERT_TRUE(web_state);
+    NSString* identifier = TabIdTabHelper::FromWebState(web_state)->tab_id();
+    EXPECT_NSEQ(identifier, pre_move_ids[(index + 1) % 3]);
+    EXPECT_NSEQ(identifier, consumer_.items[index]);
+  }
+  EXPECT_EQ(pre_move_selected_id, consumer_.selectedItemID);
 }

ios/chrome/browser/ui/tab_grid/tab_grid_view_controller.mm

@@ -733,6 +733,16 @@ UIAlertController* NotImplementedAlert() {
   }
 }
 
+- (void)gridViewController:(GridViewController*)gridViewController
+         didMoveItemWithID:(NSString*)itemID
+                   toIndex:(NSUInteger)destinationIndex {
+  if (gridViewController == self.regularTabsViewController) {
+    [self.regularTabsDelegate moveItemWithID:itemID toIndex:destinationIndex];
+  } else if (gridViewController == self.incognitoTabsViewController) {
+    [self.incognitoTabsDelegate moveItemWithID:itemID toIndex:destinationIndex];
+  }
+}
+
 - (void)gridViewController:(GridViewController*)gridViewController
         didChangeItemCount:(NSUInteger)count {
   [self configureButtonsForOriginalAndCurrentPage];