[iOS] Fix bookkeeping for RecentTabs presentation

This CL makes sure that the animations of the RecentTabs are disabled
when the recent tabs aren't presented when BVC is contained.

Bug: 1038034
Change-Id: Ie458cb6b27bcba66637d5b9dad63fc5e6809b8a0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2071938Reviewed-by: Mark Cogan <marq@chromium.org>
Reviewed-by: Sergio Collazos <sczs@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#744707}

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

@@ -75,6 +75,7 @@ source_set("recent_tabs_ui") {
     "//ios/chrome/browser/sessions",
     "//ios/chrome/browser/sessions:serialisation",
     "//ios/chrome/browser/sync",
+    "//ios/chrome/browser/ui:feature_flags",
     "//ios/chrome/browser/ui/authentication",
     "//ios/chrome/browser/ui/authentication/cells",
     "//ios/chrome/browser/ui/commands",

ios/chrome/browser/ui/recent_tabs/recent_tabs_table_view_controller.h

@@ -33,6 +33,9 @@ class WebStateList;
 @property(nonatomic, weak) id<RecentTabsTableViewControllerDelegate> delegate;
 // WebStateList for tabs restored by this object.
 @property(nonatomic, assign) WebStateList* webStateList;
+// Whether the updates of the RecentTabs should be ignored. Setting this to NO
+// would trigger a reload of the TableView.
+@property(nonatomic, assign) BOOL preventUpdates;
 
 // Delegate to present the tab UI.
 @property(nonatomic, weak) id<RecentTabsPresentationDelegate>

ios/chrome/browser/ui/recent_tabs/recent_tabs_table_view_controller.mm

@@ -45,6 +45,7 @@
 #import "ios/chrome/browser/ui/table_view/cells/table_view_url_item.h"
 #import "ios/chrome/browser/ui/table_view/chrome_table_view_styler.h"
 #import "ios/chrome/browser/ui/table_view/table_view_favicon_data_source.h"
+#include "ios/chrome/browser/ui/ui_feature_flags.h"
 #include "ios/chrome/browser/ui/util/ui_util.h"
 #import "ios/chrome/browser/ui/util/uikit_ui_util.h"
 #import "ios/chrome/browser/url_loading/url_loading_params.h"
@@ -109,11 +110,6 @@ const int kRecentlyClosedTabsSectionIndex = 0;
                                             UIGestureRecognizerDelegate> {
   std::unique_ptr<synced_sessions::SyncedSessions> _syncedSessions;
 }
-// There is no need to update the table view when other view controllers
-// are obscuring the table 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 updatesTableView;
 // The service that manages the recently closed tabs
 @property(nonatomic, assign) sessions::TabRestoreService* tabRestoreService;
 // The sync state.
@@ -158,19 +154,26 @@ const int kRecentlyClosedTabsSectionIndex = 0;
 
 - (void)viewWillAppear:(BOOL)animated {
   [super viewWillAppear:animated];
-  self.updatesTableView = YES;
-  // The table view might get stale while hidden, so we need to forcibly refresh
-  // it here.
-  [self loadModel];
-  [self.tableView reloadData];
+  if (!self.preventUpdates) {
+    // The table view might get stale while hidden, so we need to forcibly
+    // refresh it here.
+    [self loadModel];
+    [self.tableView reloadData];
+  }
+  if (!base::FeatureList::IsEnabled(kContainedBVC)) {
+    self.preventUpdates = NO;
+  }
 }
 
 - (void)viewWillDisappear:(BOOL)animated {
-  self.updatesTableView = NO;
+  if (!base::FeatureList::IsEnabled(kContainedBVC)) {
+    self.preventUpdates = YES;
+  }
   [super viewWillDisappear:animated];
 }
 
 #pragma mark - Setters & Getters
+
 // Some RecentTabs services depend on objects not present in the OffTheRecord
 // BrowserState, in order to prevent crashes set |_browserState| to
 // |browserState|->OriginalChromeBrowserState. While doing this check if
@@ -182,6 +185,18 @@ const int kRecentlyClosedTabsSectionIndex = 0;
   }
 }
 
+- (void)setPreventUpdates:(BOOL)preventUpdates {
+  if (_preventUpdates == preventUpdates)
+    return;
+
+  _preventUpdates = preventUpdates;
+
+  if (preventUpdates || !base::FeatureList::IsEnabled(kContainedBVC))
+    return;
+  [self loadModel];
+  [self.tableView reloadData];
+}
+
 #pragma mark - TableViewModel
 
 - (void)loadModel {
@@ -551,7 +566,7 @@ const int kRecentlyClosedTabsSectionIndex = 0;
       SessionSyncServiceFactory::GetForBrowserState(self.browserState);
   _syncedSessions.reset(new synced_sessions::SyncedSessions(syncService));
 
-  if (self.updatesTableView) {
+  if (!self.preventUpdates) {
     // Update the TableView and TableViewModel sections to match the new
     // sessionState.
     // Turn Off animations since UITableViewRowAnimationNone still animates.
@@ -575,6 +590,7 @@ const int kRecentlyClosedTabsSectionIndex = 0;
   // Table updates must happen before |sessionState| gets updated, since some
   // table updates rely on knowing the previous state.
   self.sessionState = newSessionState;
+
   if (self.sessionState != SessionsSyncUserState::USER_SIGNED_OUT) {
     [self.signinPromoViewMediator signinPromoViewIsRemoved];
     self.signinPromoViewMediator.consumer = nil;
@@ -583,7 +599,7 @@ const int kRecentlyClosedTabsSectionIndex = 0;
 }
 
 - (void)refreshRecentlyClosedTabs {
-  if (!self.updatesTableView)
+  if (self.preventUpdates)
     return;
 
   [self.tableView performBatchUpdates:^{

ios/chrome/browser/ui/settings/autofill/autofill_profile_settings_egtest.mm

@@ -114,7 +114,8 @@ id<GREYMatcher> NavigationBarEditButton() {
       ButtonWithAccessibilityLabelId(IDS_AUTOFILL_ADDRESSES_SETTINGS_TITLE);
   [[[EarlGrey selectElementWithMatcher:addressesButton]
          usingSearchAction:grey_scrollInDirection(kGREYDirectionDown, 200)
-      onElementWithMatcher:grey_kindOfClassName(@"UITableView")]
+      onElementWithMatcher:grey_allOf(grey_kindOfClassName(@"UITableView"),
+                                      grey_sufficientlyVisible(), nil)]
       performAction:grey_tap()];
 }
 

ios/chrome/browser/ui/settings/google_services/accounts_table_egtest.mm

@@ -235,7 +235,7 @@ id<GREYMatcher> NoBookmarksLabel() {
 // Tests that the user isn't signed out and the UI is correct when the
 // disconnect is cancelled in the Account Settings screen.
 - (void)testSignInDisconnectCancelled {
-// TODO(crbug.com/669613): Re-enable this test on devices.
+  // TODO(crbug.com/669613): Re-enable this test on devices.
 #if !TARGET_IPHONE_SIMULATOR
   EARL_GREY_TEST_DISABLED(@"Test disabled on device.");
 #endif

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

@@ -378,6 +378,7 @@ NSUInteger GetPageIndexFromPage(TabGridPage page) {
   if (base::FeatureList::IsEnabled(kContainedBVC)) {
     [self.incognitoTabsViewController contentWillAppearAnimated:animated];
     [self.regularTabsViewController contentWillAppearAnimated:animated];
+    self.remoteTabsViewController.preventUpdates = NO;
   }
 }
 
@@ -405,6 +406,7 @@ NSUInteger GetPageIndexFromPage(TabGridPage page) {
   if (base::FeatureList::IsEnabled(kContainedBVC)) {
     [self.incognitoTabsViewController contentWillDisappear];
     [self.regularTabsViewController contentWillDisappear];
+    self.remoteTabsViewController.preventUpdates = YES;
   }
 }