[ios] Active web state observer in tab collection.

ios/clean/chrome/browser/ui/tab_collection/tab_collection_view_controller.mm

Index: ios/clean/chrome/browser/ui/tab_collection/tab_collection_view_controller.mm
diff --git a/ios/clean/chrome/browser/ui/tab_collection/tab_collection_view_controller.mm b/ios/clean/chrome/browser/ui/tab_collection/tab_collection_view_controller.mm
index 5f64f9d9b6d245fb73e3ca8ef41d491c70632b49..2a1c1a5c3340b92824dd821a67d49e3b17a50500 100644
--- a/ios/clean/chrome/browser/ui/tab_collection/tab_collection_view_controller.mm
+++ b/ios/clean/chrome/browser/ui/tab_collection/tab_collection_view_controller.mm
@@ -14,6 +14,10 @@
 #error "This file requires ARC support."
 #endif
 
+namespace {
+const int kInvalidIndex = -1;
+}
+
 @interface TabCollectionViewController ()<UICollectionViewDelegate,
                                           SessionCellDelegate>
 @property(nonatomic, readwrite) UICollectionView* tabs;
@@ -49,8 +53,6 @@
     [self.tabs.leadingAnchor constraintEqualToAnchor:self.view.leadingAnchor],
     [self.tabs.trailingAnchor constraintEqualToAnchor:self.view.trailingAnchor],
   ]];
-
-  [self selectItemAtIndex:self.selectedIndex];
 }
 
 - (UIStatusBarStyle)preferredStatusBarStyle {
@@ -104,6 +106,7 @@
   [cell setAppearanceForTabTitle:self.items[index].title
                          favicon:nil
                         cellSize:CGSizeZero];
+  [cell setSelected:(index == self.selectedIndex)];
   return cell;
 }
 
@@ -139,13 +142,23 @@
 #pragma mark - TabCollectionConsumer methods
 
 - (void)insertItem:(TabCollectionItem*)item atIndex:(int)index {
+  DCHECK_GE(index, 0);
   DCHECK_LE(static_cast<NSUInteger>(index), self.items.count);
+  if (self.selectedIndex >= index) {
+    self.selectedIndex++;

[rohitrao (ping after 24h), 2017/05/26 12:44:01]

Is this VC independently trying to keep track of how the active tab index
changes when tabs are added and removed?  I don't think we should have this kind
of logic here -- it'll be too easy to get out of sync if we change the
corresponding logic in WebStateList.

Can we push the active tab index into the consumer instead?

[edchin, 2017/05/26 18:09:00]

The VC will get into an internally inconsistent state without this code. 
For example, a new cell is inserted by the end of this method. 
The highlighted cell was previously cell 5, but now it is cell 6. 
But the selected index was not updated, so this VC thinks it should
be highlighting cell 5. You have inconsistent state at the end of this
method.

We could pass the active index along with this method. (perhaps
that's what you were suggesting all along)

+  }
   [self.items insertObject:item atIndex:index];
   [self.tabs insertItemsAtIndexPaths:@[ [self indexPathForIndex:index] ]];
 }
 
 - (void)deleteItemAtIndex:(int)index {
+  DCHECK_GE(index, 0);
   DCHECK_LT(static_cast<NSUInteger>(index), self.items.count);
+  if (self.selectedIndex == index) {

[sczs, 2017/05/26 16:12:52]

I also agree with Rohit, is it possible to push the selectedIndex from the
mediator? 

[edchin, 2017/05/26 18:09:00]

See comment above.

+    self.selectedIndex = kInvalidIndex;
+  } else if (self.selectedIndex > index) {
+    self.selectedIndex--;
+  }
   [self.items removeObjectAtIndex:index];
   [self.tabs deleteItemsAtIndexPaths:@[ [self indexPathForIndex:index] ]];
 }
@@ -159,15 +172,23 @@
 }
 
 - (void)replaceItemAtIndex:(int)index withItem:(TabCollectionItem*)item {
-  [self.items removeObjectAtIndex:index];
-  [self.items insertObject:item atIndex:index];
+  DCHECK_GE(index, 0);
+  DCHECK_LT(static_cast<NSUInteger>(index), self.items.count);
+  self.items[index] = item;
+  [self.tabs reloadItemsAtIndexPaths:@[ [self indexPathForIndex:index] ]];
 }
 
 - (void)selectItemAtIndex:(int)index {
+  NSMutableArray* indexPaths = [[NSMutableArray alloc] init];
+  if (self.selectedIndex >= 0) {
+    [indexPaths addObject:[self indexPathForIndex:self.selectedIndex]];
+  }
+  if (index >= 0) {
+    DCHECK_LT(static_cast<NSUInteger>(index), self.items.count);
+    [indexPaths addObject:[self indexPathForIndex:index]];
+  }
   self.selectedIndex = index;
-  [self.tabs selectItemAtIndexPath:[self indexPathForIndex:index]
-                          animated:YES
-                    scrollPosition:UITableViewScrollPositionNone];
+  [self.tabs reloadItemsAtIndexPaths:indexPaths];
 }
 
 - (void)populateItems:(NSArray<TabCollectionItem*>*)items {