[iOS] Reland "Fix bookkeeping for RecentTabs presentation"

This CL is a Reland of crrev.com/c/2071938
This CL makes sure that the animations of the RecentTabs are disabled
when the recent tabs aren't presented when BVC is contained.

The original CL wasn't working because the updates weren't prevented
when the class was instantiated. If the user is signed in, the first
presentation is trying to update the TableView even if it wasn't shown
yet.
Fix it by preventing updates in -init.

Bug: 1038034
Change-Id: Id2ec3e63dd78a014a87abf91ec99fe001b27df4d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2095116Reviewed-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@{#748601}

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

@@ -76,6 +76,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_coordinator.mm

@@ -110,6 +110,8 @@
         setModalPresentationStyle:UIModalPresentationCustom];
   }
 
+  recentTabsTableViewController.preventUpdates = NO;
+
   [self.baseViewController
       presentViewController:self.recentTabsNavigationController
                    animated:YES

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

@@ -230,4 +230,13 @@ id<GREYMatcher> TitleOfTestPage() {
   [ChromeEarlGrey closeCurrentTab];
 }
 
+// Tests that the Recent Tabs can be opened while signed in (prevent regression
+// for https://crbug.com/1056613).
+- (void)testOpenWhileSignedIn {
+  FakeChromeIdentity* fakeIdentity = [SigninEarlGreyUtils fakeIdentity1];
+  [SigninEarlGreyUI signinWithFakeIdentity:fakeIdentity];
+
+  OpenRecentTabsPanel();
+}
+
 @end

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.
@@ -140,6 +136,7 @@ const int kRecentlyClosedTabsSectionIndex = 0;
     _sessionState = SessionsSyncUserState::USER_SIGNED_OUT;
     _syncedSessions.reset(new synced_sessions::SyncedSessions());
     _restoredTabDisposition = WindowOpenDisposition::CURRENT_TAB;
+    _preventUpdates = YES;
   }
   return self;
 }
@@ -163,15 +160,21 @@ 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 (!base::FeatureList::IsEnabled(kContainedBVC)) {
+    self.preventUpdates = NO;
+  }
+  if (!self.preventUpdates) {
+    // The table view might get stale while hidden, so we need to forcibly
+    // refresh it here.
+    [self loadModel];
+    [self.tableView reloadData];
+  }
 }
 
 - (void)viewWillDisappear:(BOOL)animated {
-  self.updatesTableView = NO;
+  if (!base::FeatureList::IsEnabled(kContainedBVC)) {
+    self.preventUpdates = YES;
+  }
   [super viewWillDisappear:animated];
 }
 
@@ -193,6 +196,18 @@ const int kRecentlyClosedTabsSectionIndex = 0;
   return self.browser->GetWebStateList();
 }
 
+- (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 {
@@ -562,7 +577,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.
@@ -586,6 +601,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;
@@ -594,7 +610,7 @@ const int kRecentlyClosedTabsSectionIndex = 0;
 }
 
 - (void)refreshRecentlyClosedTabs {
-  if (!self.updatesTableView)
+  if (self.preventUpdates)
     return;
 
   [self.tableView performBatchUpdates:^{

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;
   }
 }