Avoid showing the signed in accounts modal dialog after an update from M77.

In M77, showing the signed-in account view was disabled due to the fact
that the preferences used to compute authService->HaveAccountsChanged()
were not correctly updated (see crbug.com/1006717).

To avoid user confusion, it is important to avoid showing the signed-in
accounts dialog on the first session after an update from M77 in order
to allow the authentication service to update its internal preferences.

Bug: 1007899

Change-Id: I81aa30beee0fe6c108724e83bc62e462c2f92994
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1824259
Commit-Queue: Mihai Sardarescu <msarda@chromium.org>
Reviewed-by: Jérôme Lebel <jlebel@chromium.org>
Reviewed-by: Olivier Robin <olivierrobin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#700137}

ios/chrome/browser/metrics/previous_session_info.h

@@ -90,6 +90,10 @@ enum class DeviceBatteryState {
 // is available.
 @property(nonatomic, strong, readonly) NSString* OSVersion;
 
+// The version of the previous session or nil if no previous session data is
+// available.
+@property(nonatomic, strong, readonly) NSString* previousSessionVersion;
+
 // The time at which the previous sesion ended. Note that this is only an
 // estimate and is updated whenever another value of the receiver is updated.
 @property(nonatomic, strong, readonly) NSDate* sessionEndTime;

ios/chrome/browser/metrics/previous_session_info.mm

@@ -113,6 +113,7 @@ NSString* const kOSStartTime = @"OSStartTime";
 @property(nonatomic, assign) BOOL isFirstSessionAfterLanguageChange;
 @property(nonatomic, assign) BOOL OSRestartedAfterPreviousSession;
 @property(nonatomic, strong) NSString* OSVersion;
+@property(nonatomic, strong) NSString* previousSessionVersion;
 @property(nonatomic, strong) NSDate* sessionEndTime;
 
 @end
@@ -175,6 +176,8 @@ static PreviousSessionInfo* gSharedInstance = nil;
     gSharedInstance.OSVersion = versionOfOSAtLastRun;
 
     NSString* lastRanVersion = [defaults stringForKey:kLastRanVersion];
+    gSharedInstance.previousSessionVersion = lastRanVersion;
+
     NSString* currentVersion =
         base::SysUTF8ToNSString(version_info::GetVersionNumber());
     gSharedInstance.isFirstSessionAfterUpgrade =

ios/chrome/browser/metrics/previous_session_info_private.h

@@ -14,6 +14,7 @@
 @property(nonatomic, assign)
     previous_session_info_constants::DeviceBatteryState deviceBatteryState;
 @property(nonatomic, assign) BOOL OSRestartedAfterPreviousSession;
+@property(nonatomic, strong) NSString* previousSessionVersion;
 
 + (void)resetSharedInstanceForTesting;
 

ios/chrome/browser/metrics/previous_session_info_unittest.mm

@@ -45,6 +45,7 @@ TEST_F(PreviousSessionInfoTest, InitializationWithEmptyDefaults) {
   EXPECT_FALSE([sharedInstance didSeeMemoryWarningShortlyBeforeTerminating]);
   EXPECT_TRUE([sharedInstance isFirstSessionAfterUpgrade]);
   EXPECT_TRUE([sharedInstance isFirstSessionAfterLanguageChange]);
+  EXPECT_FALSE([sharedInstance previousSessionVersion]);
 }
 
 TEST_F(PreviousSessionInfoTest, InitializationWithSameLanguage) {
@@ -96,6 +97,7 @@ TEST_F(PreviousSessionInfoTest, InitializationWithSameVersionNoMemoryWarning) {
   // Checks the values.
   EXPECT_FALSE([sharedInstance didSeeMemoryWarningShortlyBeforeTerminating]);
   EXPECT_FALSE([sharedInstance isFirstSessionAfterUpgrade]);
+  EXPECT_NSEQ(currentVersion, [sharedInstance previousSessionVersion]);
 }
 
 TEST_F(PreviousSessionInfoTest, InitializationWithSameVersionMemoryWarning) {
@@ -118,6 +120,7 @@ TEST_F(PreviousSessionInfoTest, InitializationWithSameVersionMemoryWarning) {
   // Checks the values.
   EXPECT_TRUE([sharedInstance didSeeMemoryWarningShortlyBeforeTerminating]);
   EXPECT_FALSE([sharedInstance isFirstSessionAfterUpgrade]);
+  EXPECT_NSEQ(currentVersion, [sharedInstance previousSessionVersion]);
 }
 
 TEST_F(PreviousSessionInfoTest, InitializationDifferentVersionNoMemoryWarning) {
@@ -135,6 +138,7 @@ TEST_F(PreviousSessionInfoTest, InitializationDifferentVersionNoMemoryWarning) {
   // Checks the values.
   EXPECT_FALSE([sharedInstance didSeeMemoryWarningShortlyBeforeTerminating]);
   EXPECT_TRUE([sharedInstance isFirstSessionAfterUpgrade]);
+  EXPECT_NSEQ(@"Fake Version", [sharedInstance previousSessionVersion]);
 }
 
 TEST_F(PreviousSessionInfoTest, InitializationDifferentVersionMemoryWarning) {
@@ -155,6 +159,7 @@ TEST_F(PreviousSessionInfoTest, InitializationDifferentVersionMemoryWarning) {
   // Checks the values.
   EXPECT_TRUE([sharedInstance didSeeMemoryWarningShortlyBeforeTerminating]);
   EXPECT_TRUE([sharedInstance isFirstSessionAfterUpgrade]);
+  EXPECT_NSEQ(@"Fake Version", [sharedInstance previousSessionVersion]);
 }
 
 // Creates conditions that exist on the first app run and tests

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

@@ -46,6 +46,7 @@ source_set("authentication") {
     "//ios/chrome/browser",
     "//ios/chrome/browser/browser_state",
     "//ios/chrome/browser/infobars",
+    "//ios/chrome/browser/metrics",
     "//ios/chrome/browser/signin",
     "//ios/chrome/browser/sync",
     "//ios/chrome/browser/tabs",
@@ -121,6 +122,7 @@ source_set("unit_tests") {
     "//ios/chrome/browser",
     "//ios/chrome/browser/browser_state:test_support",
     "//ios/chrome/browser/infobars",
+    "//ios/chrome/browser/metrics",
     "//ios/chrome/browser/prefs:browser_prefs",
     "//ios/chrome/browser/signin",
     "//ios/chrome/browser/signin:test_support",

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

@@ -8,6 +8,7 @@
 #include "components/signin/public/identity_manager/identity_manager.h"
 #import "components/signin/public/identity_manager/objc/identity_manager_observer_bridge.h"
 #include "ios/chrome/browser/browser_state/chrome_browser_state.h"
+#import "ios/chrome/browser/metrics/previous_session_info.h"
 #include "ios/chrome/browser/signin/authentication_service.h"
 #include "ios/chrome/browser/signin/authentication_service_factory.h"
 #include "ios/chrome/browser/signin/chrome_identity_service_observer_bridge.h"
@@ -202,6 +203,22 @@ BOOL gSignedInAccountsViewControllerIsShown = NO;
   if (!browserState || browserState->IsOffTheRecord()) {
     return NO;
   }
+
+  PreviousSessionInfo* prevSessionInfo = [PreviousSessionInfo sharedInstance];
+  if (prevSessionInfo.isFirstSessionAfterUpgrade &&
+      [prevSessionInfo.previousSessionVersion hasPrefix:@"77."]) {
+    // In M77, showing the signed-in account view was disabled due to the fact
+    // that the preferences used to compute authService->HaveAccountsChanged()
+    // were not correctly updated (see crbug.com/1006717).
+    // To avoid user confusion, it is important to avoid showing the signed-in
+    // accounts dialog on the first session after an update from M77 in order
+    // to allow the authentication service to update its internal preferences.
+    //
+    // TODO(crbug.com/1007990) Remove this code after M81 (revert
+    // https://chromium-review.googlesource.com/c/chromium/src/+/1824259 ).
+    return NO;
+  }
+
   AuthenticationService* authService =
       AuthenticationServiceFactory::GetForBrowserState(browserState);
   return !gSignedInAccountsViewControllerIsShown &&

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

@@ -9,6 +9,8 @@
 #include "base/bind.h"
 #include "base/strings/sys_string_conversions.h"
 #include "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
+#import "ios/chrome/browser/metrics/previous_session_info.h"
+#include "ios/chrome/browser/metrics/previous_session_info_private.h"
 #include "ios/chrome/browser/signin/authentication_service_factory.h"
 #import "ios/chrome/browser/signin/authentication_service_fake.h"
 #include "ios/chrome/test/block_cleanup_test.h"
@@ -65,3 +67,45 @@ TEST_F(SignedInAccountsViewControllerTest,
   EXPECT_TRUE([SignedInAccountsViewController
       shouldBePresentedForBrowserState:browser_state_.get()]);
 }
+
+// Tests that the signed in accounts view shouldn't be presented on the first
+// session after upgrade.
+TEST_F(SignedInAccountsViewControllerTest,
+       ShouldBePresentedForBrowserStateAfterUpgrade) {
+  auth_service_->SetHaveAccountsChanged(true);
+
+  {
+    [PreviousSessionInfo resetSharedInstanceForTesting];
+    PreviousSessionInfo* prevSessionInfo = [PreviousSessionInfo sharedInstance];
+    [prevSessionInfo setIsFirstSessionAfterUpgrade:YES];
+    [prevSessionInfo setPreviousSessionVersion:nil];
+    EXPECT_TRUE([SignedInAccountsViewController
+        shouldBePresentedForBrowserState:browser_state_.get()]);
+  }
+
+  {
+    [PreviousSessionInfo resetSharedInstanceForTesting];
+    PreviousSessionInfo* prevSessionInfo = [PreviousSessionInfo sharedInstance];
+    [prevSessionInfo setIsFirstSessionAfterUpgrade:YES];
+    EXPECT_TRUE([SignedInAccountsViewController
+        shouldBePresentedForBrowserState:browser_state_.get()]);
+  }
+
+  {
+    [PreviousSessionInfo resetSharedInstanceForTesting];
+    PreviousSessionInfo* prevSessionInfo = [PreviousSessionInfo sharedInstance];
+    [prevSessionInfo setIsFirstSessionAfterUpgrade:YES];
+    [prevSessionInfo setPreviousSessionVersion:@"77.0.1.0"];
+    EXPECT_FALSE([SignedInAccountsViewController
+        shouldBePresentedForBrowserState:browser_state_.get()]);
+  }
+
+  {
+    [PreviousSessionInfo resetSharedInstanceForTesting];
+    PreviousSessionInfo* prevSessionInfo = [PreviousSessionInfo sharedInstance];
+    [prevSessionInfo setIsFirstSessionAfterUpgrade:YES];
+    [prevSessionInfo setPreviousSessionVersion:@"78.0.1.0"];
+    EXPECT_TRUE([SignedInAccountsViewController
+        shouldBePresentedForBrowserState:browser_state_.get()]);
+  }
+}