[ios] Add test to update offscreen cells in tab grid

A crash will occur when an scrolled offscreen cell is updated.
This is because UICollectionView's |-cellForItemAtIndexPath:| returns
nil if the cell is offscreen.

This CL adds a unittest that fails without the fix, which is also
in this CL. The fix is to skip cell configuration if the cell is nil.

Bug: 851352
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Iccdb53e3a67b58f225a09ce32ac70f48d42431ef
Reviewed-on: https://chromium-review.googlesource.com/1094210Reviewed-by: Mark Cogan <marq@chromium.org>
Reviewed-by: edchin <edchin@chromium.org>
Commit-Queue: edchin <edchin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566733}

ios/chrome/browser/ui/main/BUILD.gn

@@ -87,9 +87,7 @@ source_set("test_support") {
   ]
   deps = [
     "//base",
-    "//base/test:test_support",
     "//ios/chrome/browser/ui/tab_switcher",
-    "//ios/chrome/test:block_cleanup_test",
-    "//testing/gtest",
+    "//ios/chrome/test:test_support",
   ]
 }

ios/chrome/browser/ui/main/main_view_controller_test.h

@@ -5,14 +5,12 @@
 #ifndef IOS_CHROME_BROWSER_UI_MAIN_MAIN_VIEW_CONTROLLER_TEST_H_
 #define IOS_CHROME_BROWSER_UI_MAIN_MAIN_VIEW_CONTROLLER_TEST_H_
 
-#import <UIKit/UIKit.h>
-
 #include "base/macros.h"
-#import "ios/chrome/test/block_cleanup_test.h"
+#import "ios/chrome/test/root_view_controller_test.h"
 
 @protocol TabSwitcher;
 
-class MainViewControllerTest : public BlockCleanupTest {
+class MainViewControllerTest : public RootViewControllerTest {
  public:
   MainViewControllerTest() = default;
   ~MainViewControllerTest() override = default;
@@ -21,18 +19,7 @@ class MainViewControllerTest : public BlockCleanupTest {
   // Creates and returns an object that conforms to the TabSwitcher protocol.
   id<TabSwitcher> CreateTestTabSwitcher();
 
-  // Sets the current key window's rootViewController and saves a pointer to
-  // the original VC to allow restoring it at the end of the test.
-  void SetRootViewController(UIViewController* new_root_view_controller);
-
-  // testing::Test implementation.
-  void TearDown() override;
-
  private:
-  // The key window's original root view controller, which must be restored at
-  // the end of the test.
-  UIViewController* original_root_view_controller_;
-
   DISALLOW_COPY_AND_ASSIGN(MainViewControllerTest);
 };
 

ios/chrome/browser/ui/main/main_view_controller_test.mm

@@ -7,7 +7,6 @@
 #import <UIKit/UIKit.h>
 
 #import "ios/chrome/browser/ui/tab_switcher/tab_switcher.h"
-#include "testing/gtest_mac.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
 #error "This file requires ARC support."
@@ -55,21 +54,3 @@
 id<TabSwitcher> MainViewControllerTest::CreateTestTabSwitcher() {
   return [[TestTabSwitcherViewController alloc] init];
 }
-
-// Sets the current key window's rootViewController and saves a pointer to
-// the original VC to allow restoring it at the end of the test.
-void MainViewControllerTest::SetRootViewController(
-    UIViewController* new_root_view_controller) {
-  original_root_view_controller_ =
-      [[[UIApplication sharedApplication] keyWindow] rootViewController];
-  [[UIApplication sharedApplication] keyWindow].rootViewController =
-      new_root_view_controller;
-}
-
-void MainViewControllerTest::TearDown() {
-  if (original_root_view_controller_) {
-    [[UIApplication sharedApplication] keyWindow].rootViewController =
-        original_root_view_controller_;
-    original_root_view_controller_ = nil;
-  }
-}

ios/chrome/browser/ui/tab_grid/grid/BUILD.gn

@@ -49,8 +49,9 @@ source_set("unit_tests") {
 
   deps = [
     ":grid_ui",
-    "//base/test:test_support",
+    "//base",
+    "//ios/chrome/test:test_support",
+    "//ios/testing:ios_test_support",
     "//testing/gtest",
-    "//third_party/ocmock",
   ]
 }

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

@@ -361,6 +361,8 @@ NSIndexPath* CreateIndexPath(NSInteger index) {
     return;
   GridCell* cell = base::mac::ObjCCastStrict<GridCell>(
       [self.collectionView cellForItemAtIndexPath:CreateIndexPath(index)]);
+  // |cell| may be nil if it is scrolled offscreen.
+  if (cell)
     [self configureCell:cell withItem:item];
 }
 

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

@@ -7,9 +7,10 @@
 #import "base/mac/foundation_util.h"
 #import "base/numerics/safe_conversions.h"
 #import "ios/chrome/browser/ui/tab_grid/grid/grid_item.h"
+#import "ios/chrome/test/root_view_controller_test.h"
+#import "ios/testing/wait_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #import "testing/gtest_mac.h"
-#include "testing/platform_test.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
 #error "This file requires ARC support."
@@ -20,11 +21,13 @@
 @property(nonatomic, readonly) NSMutableArray<GridItem*>* items;
 @property(nonatomic, readonly) NSUInteger selectedIndex;
 @property(nonatomic, readonly) UICollectionView* collectionView;
+@property(nonatomic, assign, getter=isViewAppeared) BOOL viewAppeared;
 @end
 @implementation TestGridViewController
 @dynamic items;
 @dynamic selectedIndex;
 @dynamic collectionView;
+@dynamic viewAppeared;
 @end
 
 // Fake object that conforms to GridViewControllerDelegate.
@@ -55,7 +58,7 @@
 }
 @end
 
-class GridViewControllerTest : public PlatformTest {
+class GridViewControllerTest : public RootViewControllerTest {
  public:
   GridViewControllerTest() {
     view_controller_ = [[TestGridViewController alloc] init];
@@ -91,9 +94,9 @@ TEST_F(GridViewControllerTest, InitializeItems) {
 TEST_F(GridViewControllerTest, InsertItem) {
   // Previously: The grid had 2 items and selectedIndex was 0. The delegate had
   // an itemCount of 2.
-  [view_controller_ insertItem:[[GridItem alloc] initWithIdentifier:@"C"]
+  [view_controller_ insertItem:[[GridItem alloc] initWithIdentifier:@"NEW-ITEM"]
                        atIndex:2
-                selectedItemID:@"C"];
+                selectedItemID:@"NEW-ITEM"];
   EXPECT_EQ(3U, view_controller_.items.count);
   EXPECT_EQ(2U, view_controller_.selectedIndex);
   EXPECT_EQ(3U, delegate_.itemCount);
@@ -158,3 +161,31 @@ TEST_F(GridViewControllerTest, MoveUnselectedItem) {
   EXPECT_EQ(1U, view_controller_.selectedIndex);
   EXPECT_EQ(2U, delegate_.itemCount);
 }
+
+// Tests that |-replaceItemID:withItem:| does not crash when updating an item
+// that is scrolled offscreen.
+TEST_F(GridViewControllerTest, ReplaceScrolledOffScreenCell) {
+  // This test requires that the collection view be placed on the screen.
+  SetRootViewController(view_controller_);
+  bool condition_met = testing::WaitUntilConditionOrTimeout(
+      testing::kWaitForUIElementTimeout, ^bool {
+        return [view_controller_ isViewAppeared];
+      });
+  EXPECT_TRUE(condition_met);
+  NSArray* visibleCells = view_controller_.collectionView.visibleCells;
+  NSArray* items = view_controller_.items;
+  // Keep adding items until we get an item that is offscreen. Since device
+  // sizes may vary, this is better than creating a fixed number of items that
+  // we think will overflow to offscreen items.
+  while (visibleCells.count >= items.count) {
+    NSString* uniqueID =
+        [NSString stringWithFormat:@"%d", base::checked_cast<int>(items.count)];
+    GridItem* item = [[GridItem alloc] initWithIdentifier:uniqueID];
+    [view_controller_ insertItem:item atIndex:0 selectedItemID:@"A"];
+    visibleCells = view_controller_.collectionView.visibleCells;
+  }
+  // The last item ("B") is scrolled off screen.
+  GridItem* item = [[GridItem alloc] initWithIdentifier:@"NEW-ITEM"];
+  // Do not crash due to cell being nil.
+  [view_controller_ replaceItemID:@"B" withItem:item];
+}

ios/chrome/test/BUILD.gn

@@ -28,6 +28,8 @@ source_set("test_support") {
     "ios_chrome_scoped_testing_local_state.h",
     "ios_chrome_unit_test_suite.h",
     "ios_chrome_unit_test_suite.mm",
+    "root_view_controller_test.h",
+    "root_view_controller_test.mm",
     "scoped_block_popups_pref.h",
     "scoped_block_popups_pref.mm",
     "scoped_key_window.h",

ios/chrome/test/root_view_controller_test.h

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef IOS_CHROME_TEST_ROOT_VIEW_CONTROLLER_TEST_H_
+#define IOS_CHROME_TEST_ROOT_VIEW_CONTROLLER_TEST_H_
+
+#import <UIKit/UIKit.h>
+
+#include "base/macros.h"
+#include "testing/platform_test.h"
+
+// Base test fixture that restores the key window's original root view
+// controller at tear down.
+class RootViewControllerTest : public PlatformTest {
+ public:
+  RootViewControllerTest() = default;
+  ~RootViewControllerTest() override = default;
+
+ protected:
+  // Sets the current key window's rootViewController and saves a pointer to
+  // the original VC to allow restoring it at the end of the test.
+  void SetRootViewController(UIViewController* new_root_view_controller);
+
+  // PlatformTest implementation.
+  void TearDown() override;
+
+ private:
+  // The key window's original root view controller, which must be restored at
+  // the end of the test.
+  UIViewController* original_root_view_controller_;
+
+  DISALLOW_COPY_AND_ASSIGN(RootViewControllerTest);
+};
+
+#endif  // IOS_CHROME_TEST_ROOT_VIEW_CONTROLLER_TEST_H_

ios/chrome/test/root_view_controller_test.mm

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#import "ios/chrome/test/root_view_controller_test.h"
+
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
+// Sets the current key window's rootViewController and saves a pointer to
+// the original VC to allow restoring it at the end of the test.
+void RootViewControllerTest::SetRootViewController(
+    UIViewController* new_root_view_controller) {
+  original_root_view_controller_ =
+      [[[UIApplication sharedApplication] keyWindow] rootViewController];
+  [[UIApplication sharedApplication] keyWindow].rootViewController =
+      new_root_view_controller;
+}
+
+void RootViewControllerTest::TearDown() {
+  if (original_root_view_controller_) {
+    [[UIApplication sharedApplication] keyWindow].rootViewController =
+        original_root_view_controller_;
+    original_root_view_controller_ = nil;
+  }
+  PlatformTest::TearDown();
+}