Remove updatesCollectionView from grid view controller.

In order to allow the tab thumbnails to be up to date when the thumb
strip is visible, the grid's collection view needs to be updated even
when other view controllers are obscuring it. When selecting a new item
from the collection view (blue highlight around the thumbnail), the item
that is currently selected needs to be deselected to prevent multiple
simultaneous selection.

Bug: 1127604
Change-Id: Ic1e4a2dd4d0f2c269031b9d27b47b38c56b07a40
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2410494Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Commit-Queue: Roberto Moura <mouraroberto@google.com>
Auto-Submit: Roberto Moura <mouraroberto@google.com>
Cr-Commit-Position: refs/heads/master@{#809263}

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

@@ -49,11 +49,6 @@ NSIndexPath* CreateIndexPath(NSInteger index) {
                                   UICollectionViewDelegate,
                                   UICollectionViewDragDelegate,
                                   UICollectionViewDropDelegate>
-// There is no need to update the collection view when other view controllers
-// are obscuring the collection view. Bookkeeping is based on |-viewWillAppear:|
-// and |-viewWillDisappear methods. Note that the |Did| methods are not reliably
-// called (e.g., edge case in multitasking).
-@property(nonatomic, assign) BOOL updatesCollectionView;
 // A collection view of items in a grid format.
 @property(nonatomic, weak) UICollectionView* collectionView;
 // The local model backing the collection view.
@@ -270,7 +265,6 @@ NSIndexPath* CreateIndexPath(NSInteger index) {
 }
 
 - (void)contentWillAppearAnimated:(BOOL)animated {
-  self.updatesCollectionView = YES;
   self.defaultLayout.animatesItemUpdates = YES;
   [self.collectionView reloadData];
   // Selection is invalid if there are no items.
@@ -288,7 +282,6 @@ NSIndexPath* CreateIndexPath(NSInteger index) {
 }
 
 - (void)contentWillDisappear {
-  self.updatesCollectionView = NO;
 }
 
 #pragma mark - UICollectionViewDataSource
@@ -512,17 +505,14 @@ NSIndexPath* CreateIndexPath(NSInteger index) {
 
   self.items = [items mutableCopy];
   self.selectedItemID = selectedItemID;
-  if ([self updatesCollectionView]) {
-    [self.collectionView reloadData];
-    [self.collectionView
-        selectItemAtIndexPath:CreateIndexPath(self.selectedIndex)
-                     animated:YES
-               scrollPosition:UICollectionViewScrollPositionTop];
-    if (self.items.count > 0) {
-      [self removeEmptyStateAnimated:YES];
-    } else {
-      [self animateEmptyStateIn];
-    }
+  [self.collectionView reloadData];
+  [self.collectionView selectItemAtIndexPath:CreateIndexPath(self.selectedIndex)
+                                    animated:YES
+                              scrollPosition:UICollectionViewScrollPositionTop];
+  if (self.items.count > 0) {
+    [self removeEmptyStateAnimated:YES];
+  } else {
+    [self animateEmptyStateIn];
   }
   // Whether the view is visible or not, the delegate must be updated.
   [self.delegate gridViewController:self didChangeItemCount:self.items.count];
@@ -601,9 +591,13 @@ NSIndexPath* CreateIndexPath(NSInteger index) {
 }
 
 - (void)selectItemWithID:(NSString*)selectedItemID {
-  self.selectedItemID = selectedItemID;
-  if (!([self updatesCollectionView] && self.showsSelectionUpdates))
+  if (self.selectedItemID == selectedItemID)
     return;
+
+  [self.collectionView
+      deselectItemAtIndexPath:CreateIndexPath(self.selectedIndex)
+                     animated:YES];
+  self.selectedItemID = selectedItemID;
   [self.collectionView
       selectItemAtIndexPath:CreateIndexPath(self.selectedIndex)
                    animated:YES
@@ -618,8 +612,6 @@ NSIndexPath* CreateIndexPath(NSInteger index) {
          [self indexOfItemWithID:item.identifier] == NSNotFound);
   NSUInteger index = [self indexOfItemWithID:itemID];
   self.items[index] = item;
-  if (![self updatesCollectionView])
-    return;
   GridCell* cell = base::mac::ObjCCastStrict<GridCell>(
       [self.collectionView cellForItemAtIndexPath:CreateIndexPath(index)]);
   // |cell| may be nil if it is scrolled offscreen.
@@ -660,22 +652,13 @@ NSIndexPath* CreateIndexPath(NSInteger index) {
 
 #pragma mark - Private
 
-// Performs model updates and view updates together if the view is appeared, or
-// only the model updates if the view is not appeared. |completion| is only run
-// if view is appeared.
+// Performs model updates and view updates.
 - (void)performModelUpdates:(ProceduralBlock)modelUpdates
               collectionViewUpdates:(ProceduralBlock)collectionViewUpdates
     collectionViewUpdatesCompletion:
         (ProceduralBlockWithBool)collectionViewUpdatesCompletion {
-  // If the view isn't visible, there's no need for the collection view to
-  // update.
-  if (![self updatesCollectionView]) {
-    modelUpdates();
-    return;
-  }
+  modelUpdates();
   [self.collectionView performBatchUpdates:^{
-    // Synchronize model and view updates.
-    modelUpdates();
     collectionViewUpdates();
   }
                                 completion:collectionViewUpdatesCompletion];

ios/chrome/browser/web/browsing_egtest.mm

@@ -77,12 +77,13 @@ class ReloadResponseProvider : public web::DataResponseProvider {
 
 @implementation BrowsingTestCase
 
-// Matcher for the title of the current tab (on tablet only).
+// Matcher for the title of the current tab (on tablet only), which is
+// sufficiently visible.
 id<GREYMatcher> TabWithTitle(const std::string& tab_title) {
   id<GREYMatcher> notPartOfOmnibox =
       grey_not(grey_ancestor(chrome_test_util::Omnibox()));
   return grey_allOf(grey_accessibilityLabel(base::SysUTF8ToNSString(tab_title)),
-                    notPartOfOmnibox, nil);
+                    notPartOfOmnibox, grey_sufficientlyVisible(), nil);
 }
 
 // Tests that page successfully reloads.
@@ -120,7 +121,7 @@ id<GREYMatcher> TabWithTitle(const std::string& tab_title) {
       destinationURL.spec().substr(destinationURL.scheme().length() + 3);
 
   [[EarlGrey selectElementWithMatcher:TabWithTitle(URLWithoutScheme)]
-      assertWithMatcher:grey_sufficientlyVisible()];
+      assertWithMatcher:grey_notNil()];
 }
 
 // Tests that after a PDF is loaded, the title appears in the tab bar on iPad.
@@ -138,7 +139,7 @@ id<GREYMatcher> TabWithTitle(const std::string& tab_title) {
       destinationURL.spec().substr(destinationURL.scheme().length() + 3);
 
   [[EarlGrey selectElementWithMatcher:TabWithTitle(URLWithoutScheme)]
-      assertWithMatcher:grey_sufficientlyVisible()];
+      assertWithMatcher:grey_notNil()];
 }
 
 // Tests that tab title is set to the specified title from a JavaScript.
@@ -155,7 +156,7 @@ id<GREYMatcher> TabWithTitle(const std::string& tab_title) {
   [ChromeEarlGrey loadURL:URL];
 
   [[EarlGrey selectElementWithMatcher:TabWithTitle(kPageTitle)]
-      assertWithMatcher:grey_sufficientlyVisible()];
+      assertWithMatcher:grey_notNil()];
 }
 
 // Tests clicking a link with target="_blank" and "event.stopPropagation()"