[iOS][Signin]Fixing crash to reload sign-in promo view in recent tabs

When the sign-in promo view is hidden (by collapsing the "Other Devices" section), Chrome crashes,
if the sign-in promo view has to be reloaded (with a new identity added by another app for
example).
If the section is collapse the sign-in promo should not be updated. The view will be updated later
when the user will uncollapse the section.

Bug: 776939
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Ie68ba68e40580b510a782b54ec744865311ed2bb
Reviewed-on: https://chromium-review.googlesource.com/811464
Commit-Queue: Jérôme Lebel <jlebel@chromium.org>
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522407}

ios/chrome/browser/ui/authentication/signin_earlgrey_utils.h

@@ -12,9 +12,16 @@
 // Methods used for the EarlGrey tests.
 @interface SigninEarlGreyUtils : NSObject
 
-// Checks that the sign-in promo view is visible using the right mode.
+// Checks that the sign-in promo view (with a close button) is visible using the
+// right mode.
 + (void)checkSigninPromoVisibleWithMode:(SigninPromoViewMode)mode;
 
+// Checks that the sign-in promo view is visible using the right mode. If
+// |closeButton| is set to YES, the close button in the sign-in promo has to be
+// visible.
++ (void)checkSigninPromoVisibleWithMode:(SigninPromoViewMode)mode
+                            closeButton:(BOOL)closeButton;
+
 // Checks that the sign-in promo view is not visible.
 + (void)checkSigninPromoNotVisible;
 

ios/chrome/browser/ui/authentication/signin_earlgrey_utils.mm

@@ -25,6 +25,11 @@ using chrome_test_util::SecondarySignInButton;
 @implementation SigninEarlGreyUtils
 
 + (void)checkSigninPromoVisibleWithMode:(SigninPromoViewMode)mode {
+  [self checkSigninPromoVisibleWithMode:mode closeButton:YES];
+}
+
++ (void)checkSigninPromoVisibleWithMode:(SigninPromoViewMode)mode
+                            closeButton:(BOOL)closeButton {
   [[EarlGrey
       selectElementWithMatcher:grey_allOf(
                                    grey_accessibilityID(kSigninPromoViewId),
@@ -48,11 +53,13 @@ using chrome_test_util::SecondarySignInButton;
           assertWithMatcher:grey_notNil()];
       break;
   }
-  [[EarlGrey
-      selectElementWithMatcher:grey_allOf(grey_accessibilityID(
-                                              kSigninPromoCloseButtonId),
-                                          grey_sufficientlyVisible(), nil)]
-      assertWithMatcher:grey_notNil()];
+  if (closeButton) {
+    [[EarlGrey
+        selectElementWithMatcher:grey_allOf(grey_accessibilityID(
+                                                kSigninPromoCloseButtonId),
+                                            grey_sufficientlyVisible(), nil)]
+        assertWithMatcher:grey_notNil()];
+  }
 }
 
 + (void)checkSigninPromoNotVisible {

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

@@ -90,8 +90,11 @@ source_set("eg_tests") {
     "//ios/chrome/app/strings",
     "//ios/chrome/browser/bookmarks:features",
     "//ios/chrome/browser/ui",
+    "//ios/chrome/browser/ui/authentication:eg_test_support",
+    "//ios/chrome/browser/ui/ntp/recent_tabs",
     "//ios/chrome/test/app:test_support",
     "//ios/chrome/test/earl_grey:test_support",
+    "//ios/public/provider/chrome/browser/signin:test_support",
     "//ios/third_party/earl_grey:earl_grey+link",
     "//ios/web/public/test/http_server",
     "//ui/base",

ios/chrome/browser/ui/ntp/recent_tabs/recent_tabs_table_egtest.mm

@@ -11,6 +11,8 @@
 #include "base/test/scoped_feature_list.h"
 #include "components/strings/grit/components_strings.h"
 #include "ios/chrome/browser/bookmarks/bookmark_new_generation_features.h"
+#import "ios/chrome/browser/ui/authentication/signin_earlgrey_utils.h"
+#import "ios/chrome/browser/ui/ntp/recent_tabs/recent_tabs_table_view_controller.h"
 #include "ios/chrome/browser/ui/ui_util.h"
 #include "ios/chrome/grit/ios_strings.h"
 #import "ios/chrome/test/app/tab_test_util.h"
@@ -18,6 +20,7 @@
 #import "ios/chrome/test/earl_grey/chrome_earl_grey_ui.h"
 #import "ios/chrome/test/earl_grey/chrome_matchers.h"
 #import "ios/chrome/test/earl_grey/chrome_test_case.h"
+#import "ios/public/provider/chrome/browser/signin/fake_chrome_identity_service.h"
 #import "ios/web/public/test/http_server/http_server.h"
 #include "ios/web/public/test/http_server/http_server_util.h"
 #import "ui/base/l10n/l10n_util.h"
@@ -81,6 +84,8 @@ id<GREYMatcher> RecentlyClosedLabelMatcher() {
       web::test::HttpServer::MakeUrl(kURLOfTestPage),
       std::string(kHTMLOfTestPage),
   }});
+  [NSUserDefaults.standardUserDefaults setObject:@{}
+                                          forKey:kCollapsedSectionsKey];
 }
 
 - (void)tearDown {
@@ -101,6 +106,23 @@ id<GREYMatcher> RecentlyClosedLabelMatcher() {
   }
 }
 
+- (void)closeRecentTabs {
+  // Get rid of the Recent Tabs Panel.
+  if (IsIPadIdiom()) {
+    // On iPad, the Recent Tabs panel is a new page in the navigation history.
+    // Go back to the previous page to restore the test page.
+    [[EarlGrey selectElementWithMatcher:chrome_test_util::BackButton()]
+        performAction:grey_tap()];
+    [ChromeEarlGrey waitForPageToFinishLoading];
+  } else {
+    // On iPhone, the Recent Tabs panel is shown in a modal view.
+    // Close that modal.
+    CloseRecentTabsPanelOnIphone();
+    // Wait until the recent tabs panel is dismissed.
+    [[GREYUIThreadExecutor sharedInstance] drainUntilIdle];
+  }
+}
+
 // Tests that a closed tab appears in the Recent Tabs panel, and that tapping
 // the entry in the Recent Tabs panel re-opens the closed tab.
 - (void)testClosedTabAppearsInRecentTabsPanel {
@@ -119,21 +141,7 @@ id<GREYMatcher> RecentlyClosedLabelMatcher() {
   OpenRecentTabsPanel();
   [[EarlGrey selectElementWithMatcher:TitleOfTestPage()]
       assertWithMatcher:grey_nil()];
-
-  // Get rid of the Recent Tabs Panel.
-  if (IsIPadIdiom()) {
-    // On iPad, the Recent Tabs panel is a new page in the navigation history.
-    // Go back to the previous page to restore the test page.
-    [[EarlGrey selectElementWithMatcher:chrome_test_util::BackButton()]
-        performAction:grey_tap()];
-    [ChromeEarlGrey waitForPageToFinishLoading];
-  } else {
-    // On iPhone, the Recent Tabs panel is shown in a modal view.
-    // Close that modal.
-    CloseRecentTabsPanelOnIphone();
-    // Wait until the recent tabs panel is dismissed.
-    [[GREYUIThreadExecutor sharedInstance] drainUntilIdle];
-  }
+  [self closeRecentTabs];
 
   // Close the tab containing the test page and wait for the stack view to
   // appear.
@@ -188,4 +196,66 @@ id<GREYMatcher> RecentlyClosedLabelMatcher() {
   chrome_test_util::CloseCurrentTab();
 }
 
+// Tests that the sign-in promo can be reloaded correctly.
+- (void)testRecentTabSigninPromoReloaded {
+  // TODO(crbug.com/782551): Rewrite this egtest for the new bookmark.
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndDisableFeature(kBookmarkNewGeneration);
+
+  OpenRecentTabsPanel();
+  // Sign-in promo should be visible with cold state.
+  [SigninEarlGreyUtils
+      checkSigninPromoVisibleWithMode:SigninPromoViewModeColdState
+                          closeButton:NO];
+  ChromeIdentity* identity = [SigninEarlGreyUtils fakeIdentity1];
+  ios::FakeChromeIdentityService::GetInstanceFromChromeProvider()->AddIdentity(
+      identity);
+  // Sign-in promo should be visible with warm state.
+  [SigninEarlGreyUtils
+      checkSigninPromoVisibleWithMode:SigninPromoViewModeWarmState
+                          closeButton:NO];
+  [self closeRecentTabs];
+  ios::FakeChromeIdentityService::GetInstanceFromChromeProvider()
+      ->RemoveIdentity(identity);
+}
+
+// Tests that the sign-in promo can be reloaded correctly while being hidden.
+// crbug.com/776939
+- (void)testRecentTabSigninPromoReloadedWhileHidden {
+  // TODO(crbug.com/782551): Rewrite this egtest for the new bookmark.
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndDisableFeature(kBookmarkNewGeneration);
+
+  OpenRecentTabsPanel();
+  [SigninEarlGreyUtils
+      checkSigninPromoVisibleWithMode:SigninPromoViewModeColdState
+                          closeButton:NO];
+
+  // Tap on "Other Devices", to hide the sign-in promo.
+  [[EarlGrey
+      selectElementWithMatcher:chrome_test_util::ButtonWithAccessibilityLabel(
+                                   l10n_util::GetNSString(
+                                       IDS_IOS_RECENT_TABS_OTHER_DEVICES))]
+      performAction:grey_tap()];
+  [SigninEarlGreyUtils checkSigninPromoNotVisible];
+
+  // Add an account.
+  ChromeIdentity* identity = [SigninEarlGreyUtils fakeIdentity1];
+  ios::FakeChromeIdentityService::GetInstanceFromChromeProvider()->AddIdentity(
+      identity);
+
+  // Tap on "Other Devcies", to show the sign-in promo.
+  [[EarlGrey
+      selectElementWithMatcher:chrome_test_util::ButtonWithAccessibilityLabel(
+                                   l10n_util::GetNSString(
+                                       IDS_IOS_RECENT_TABS_OTHER_DEVICES))]
+      performAction:grey_tap()];
+  [SigninEarlGreyUtils
+      checkSigninPromoVisibleWithMode:SigninPromoViewModeWarmState
+                          closeButton:NO];
+  [self closeRecentTabs];
+  ios::FakeChromeIdentityService::GetInstanceFromChromeProvider()
+      ->RemoveIdentity(identity);
+}
+
 @end

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

@@ -21,6 +21,9 @@ class ChromeBrowserState;
 @protocol ApplicationCommands;
 @protocol RecentTabsHandsetViewControllerCommand;
 
+// Key for saving collapsed session state in the UserDefaults.
+extern NSString* const kCollapsedSectionsKey;
+
 @protocol RecentTabsTableViewControllerDelegate<NSObject>
 // Tells the delegate when the table view content scrolled or changed size.
 - (void)recentTabsTableViewContentMoved:(UITableView*)tableView;

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

@@ -53,11 +53,11 @@
 #error "This file requires ARC support."
 #endif
 
-namespace {
-
 // Key for saving collapsed session state in the UserDefaults.
 NSString* const kCollapsedSectionsKey = @"ChromeRecentTabsCollapsedSections";
 
+namespace {
+
 // Key for saving whether the Other Device section is collapsed.
 NSString* const kOtherDeviceCollapsedKey = @"OtherDevicesCollapsed";
 
@@ -1005,6 +1005,8 @@ enum CellType {
             (SigninPromoViewConfigurator*)configurator
                              identityChanged:(BOOL)identityChanged {
   DCHECK(_signinPromoViewMediator);
+  if ([self sectionIsCollapsed:kOtherDeviceCollapsedKey])
+    return;
   NSInteger sectionIndex =
       [self sectionIndexForSectionType:OTHER_DEVICES_SECTION];
   DCHECK(sectionIndex != NSNotFound);