[ios] Fix loss of incognito tabs on app startup

A single boolean was used to save whether the UI was showing incognito
or regular UI, however with multi-window enabled, each window can be
displaying a different UI.

This global was read on launch to decide whether incognito UI should
be displayed or not. If the window in the foreground when the app was
terminated was showing incognito UI, but user start the app by opening
a window with no incognito tab, the startup code would consider this
an inconsistency, delete all incognito state and switch to regular UI.

This behaviour is incorrect when multi-window is enabled and can lead
to losing the incognito tabs on app startup. To fix this, convert the
bool indicating whether the UI was showing incognito or regular tabs
to be per-session (i.e. per window).

Also in case of inconsistency (which can still happen if user quit
the app while showing the incognito tab switcher after having closed
the last incognito tab of the current window), do *not* clear the
incognito data (it will only be cleared when the app restart after a
crash).

Bug: 1163505
Change-Id: I8410acf6f8feadb747d01889a2812e91d5391709
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2611101
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Stepan Khapugin <stkhapugin@chromium.org>
Reviewed-by: Mohammad Refaat <mrefaat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#842098}

ios/chrome/app/application_delegate/fake_startup_information.mm

@@ -52,11 +52,6 @@
   // Stub.
 }
 
-- (BOOL)canLaunchInIncognito {
-  // Stub.
-  return NO;
-}
-
 - (NSDictionary*)launchOptions {
   // Stub.
   return @{};

ios/chrome/app/application_delegate/startup_information.h

@@ -29,8 +29,6 @@ class TimeTicks;
 // Keeps track of the restore state during startup.
 @property(nonatomic, strong) CrashRestoreHelper* restoreHelper;
 
-- (BOOL)canLaunchInIncognito;
-
 // Only for iOS 12 compat.
 - (NSDictionary*)launchOptions;
 

ios/chrome/app/application_delegate/url_opener_unittest.mm

@@ -64,10 +64,6 @@ enum class ExternalFilesLoadedInWebStateFeature {
 - (void)stopChromeMain {
 }
 
-- (BOOL)canLaunchInIncognito {
-  return NO;
-}
-
 - (NSDictionary*)launchOptions {
   return @{};
 }

ios/chrome/app/main_controller.mm

@@ -650,21 +650,6 @@ void MainControllerAuthenticationServiceDelegate::ClearBrowsingData(
   _firstUserActionRecorder.reset();
 }
 
-- (BOOL)canLaunchInIncognito {
-  NSUserDefaults* standardDefaults = [NSUserDefaults standardUserDefaults];
-  if (![standardDefaults boolForKey:kIncognitoCurrentKey])
-    return NO;
-  // If the application crashed in incognito mode, don't stay in incognito
-  // mode, since the prompt to restore should happen in non-incognito
-  // context.
-  if ([self mustShowRestoreInfobar])
-    return NO;
-  // If there are no incognito tabs, then ensure the app starts in normal mode,
-  // since the UI isn't supposed to ever put the user in incognito mode without
-  // any incognito tabs.
-  return !(self.otrBrowser->GetWebStateList()->empty());
-}
-
 - (void)expireFirstUserActionRecorderAfterDelay:(NSTimeInterval)delay {
   [self performSelector:@selector(expireFirstUserActionRecorder)
              withObject:nil

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

@@ -15,13 +15,6 @@
 class ChromeBrowserState;
 @class SceneState;
 
-namespace {
-
-// Preference key used to store which profile is current.
-NSString* kIncognitoCurrentKey = @"IncognitoActive";
-
-}  // namespace
-
 // Wrangler (a class in need of further refactoring) for handling the creation
 // and ownership of Browser instances and their associated
 // BrowserViewControllers.
@@ -65,9 +58,6 @@ NSString* kIncognitoCurrentKey = @"IncognitoActive";
 // instance of this class to deallocate without a call to this method first.
 - (void)shutdown;
 
-// Switch all global states for the given mode (normal or incognito).
-- (void)switchGlobalStateToMode:(ApplicationMode)mode;
-
 @end
 
 #endif  // IOS_CHROME_BROWSER_UI_MAIN_BROWSER_VIEW_WRANGLER_H_

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

@@ -221,11 +221,6 @@
   if (self.currentInterface) {
     // Tell the current BVC it moved to the background.
     [self.currentInterface setPrimary:NO];
-
-    // Data storage for the browser is always owned by the current BVC, so it
-    // must be updated when switching between BVCs.
-    [self changeStorageFromBrowserState:self.currentInterface.browserState
-                         toBrowserState:interface.browserState];
   }
 
   _currentInterface = interface;
@@ -292,27 +287,6 @@
   _otrBrowser = std::move(otrBrowser);
 }
 
-#pragma mark - Mode Switching
-
-- (void)switchGlobalStateToMode:(ApplicationMode)mode {
-  // TODO(crbug.com/1048690): use scene-local storage in multiwindow.
-  const BOOL incognito = (mode == ApplicationMode::INCOGNITO);
-  // Write the state to disk of what is "active".
-  NSUserDefaults* standardDefaults = [NSUserDefaults standardUserDefaults];
-  [standardDefaults setBool:incognito forKey:kIncognitoCurrentKey];
-  // Save critical state information for switching between normal and
-  // incognito.
-  [standardDefaults synchronize];
-}
-
-// Updates the local storage, cookie store, and sets the global state.
-- (void)changeStorageFromBrowserState:(ChromeBrowserState*)oldState
-                       toBrowserState:(ChromeBrowserState*)newState {
-  ApplicationMode mode = newState->IsOffTheRecord() ? ApplicationMode::INCOGNITO
-                                                    : ApplicationMode::NORMAL;
-  [self switchGlobalStateToMode:mode];
-}
-
 #pragma mark - Other public methods
 
 - (void)willDestroyIncognitoBrowserState {

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

@@ -675,22 +675,20 @@ const char kMultiWindowOpenInNewWindowHistogram[] =
     }
   }
 
-  // Before bringing up the UI, make sure the launch mode is correct, and
-  // check for previous crashes.
-  BOOL startInIncognito =
-      [[NSUserDefaults standardUserDefaults] boolForKey:kIncognitoCurrentKey];
-  BOOL switchFromIncognito =
-      startInIncognito &&
-      !self.sceneState.appState.startupInformation.canLaunchInIncognito;
-
-  if (self.sceneState.appState.postCrashLaunch || switchFromIncognito) {
+  // Make sure the launch mode is correct and consistent with the mode used
+  // when the application was terminated. It is possible for the incognito
+  // UI to have been presented but with no tabs (e.g. the tab switcher was
+  // active and user closed the last tab). In that case, switch to regular
+  // UI. Also, if the app crashed, always switch back to regular UI.
+  const BOOL startInIncognito =
+      self.sceneState.incognitoContentVisible &&
+      !self.sceneState.appState.postCrashLaunch &&
+      !self.interfaceProvider.incognitoInterface.browser->GetWebStateList()
+           ->empty();
+
+  // If the application crashed, clear incognito state.
+  if (self.sceneState.appState.postCrashLaunch)
     [self clearIOSSpecificIncognitoData];
-    if (switchFromIncognito)
-      [self.browserViewWrangler
-          switchGlobalStateToMode:ApplicationMode::NORMAL];
-  }
-  if (switchFromIncognito)
-    startInIncognito = NO;
 
   [self createInitialUI:(startInIncognito ? ApplicationMode::INCOGNITO
                                           : ApplicationMode::NORMAL)];
@@ -738,8 +736,6 @@ const char kMultiWindowOpenInNewWindowHistogram[] =
   // Decide if the First Run UI needs to run.
   const bool firstRun = ShouldPresentFirstRunExperience();
 
-  [self.browserViewWrangler switchGlobalStateToMode:launchMode];
-
   Browser* browser;
   if (launchMode == ApplicationMode::INCOGNITO) {
     browser = self.incognitoInterface.browser;

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

@@ -120,6 +120,14 @@ typedef NS_ENUM(NSUInteger, SceneActivationLevel) {
 // Array of all agents added to this scene state.
 - (NSArray*)connectedAgents;
 
+// Retrieves per-session preference for |key|. May return nil if the key is
+// not found.
+- (NSObject*)sessionObjectForKey:(NSString*)key;
+
+// Stores |object| as a per-session preference if supported by the device or
+// into NSUserDefaults otherwise (old table, phone, ...).
+- (void)setSessionObject:(NSObject*)object forKey:(NSString*)key;
+
 @end
 
 #endif  // IOS_CHROME_BROWSER_UI_MAIN_SCENE_STATE_H_

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

@@ -6,6 +6,7 @@
 
 #import "base/ios/crb_protocol_observers.h"
 #include "base/logging.h"
+#import "base/mac/foundation_util.h"
 #include "base/notreached.h"
 #include "base/strings/sys_string_conversions.h"
 #import "ios/chrome/app/application_delegate/app_state.h"
@@ -17,6 +18,28 @@
 #error "This file requires ARC support."
 #endif
 
+namespace {
+
+// Preference key used to store which profile is current.
+NSString* kIncognitoCurrentKey = @"IncognitoActive";
+
+// Represents the state of the -[SceneState incognitoContentVisible] property
+// that is saved in session storage (and thus unknown during app startup and
+// will be lazily loaded when needed).
+enum class ContentVisibility {
+  kUnknown,
+  kRegular,
+  kIncognito,
+};
+
+// Returns the value of ContentVisibility depending on |isIncognito| boolean.
+ContentVisibility ContentVisibilityForIncognito(BOOL isIncognito) {
+  return isIncognito ? ContentVisibility::kIncognito
+                     : ContentVisibility::kRegular;
+}
+
+}  // namespace
+
 @interface SceneStateObserverList : CRBProtocolObservers <SceneStateObserver>
 @end
 
@@ -35,7 +58,10 @@
 
 @end
 
-@implementation SceneState
+@implementation SceneState {
+  ContentVisibility _contentVisibility;
+}
+
 @synthesize window = _window;
 
 - (instancetype)initWithAppState:(AppState*)appState {
@@ -44,6 +70,7 @@
     _appState = appState;
     _observers = [SceneStateObserverList
         observersWithProtocol:@protocol(SceneStateObserver)];
+    _contentVisibility = ContentVisibility::kUnknown;
     _agents = [[NSMutableArray alloc] init];
 
     // AppState might be nil in tests.
@@ -160,11 +187,38 @@
   }
 }
 
+- (BOOL)incognitoContentVisible {
+  switch (_contentVisibility) {
+    case ContentVisibility::kRegular:
+      return NO;
+
+    case ContentVisibility::kIncognito:
+      return YES;
+
+    case ContentVisibility::kUnknown: {
+      const BOOL incognitoContentVisible = [base::mac::ObjCCast<NSNumber>(
+          [self sessionObjectForKey:kIncognitoCurrentKey]) boolValue];
+
+      _contentVisibility =
+          ContentVisibilityForIncognito(incognitoContentVisible);
+      DCHECK_NE(_contentVisibility, ContentVisibility::kUnknown);
+
+      return incognitoContentVisible;
+    }
+  }
+}
+
 - (void)setIncognitoContentVisible:(BOOL)incognitoContentVisible {
-  if (incognitoContentVisible == _incognitoContentVisible) {
+  const ContentVisibility contentVisibility =
+      ContentVisibilityForIncognito(incognitoContentVisible);
+  if (contentVisibility == _contentVisibility)
     return;
-  }
-  _incognitoContentVisible = incognitoContentVisible;
+
+  _contentVisibility = contentVisibility;
+
+  [self setSessionObject:@(incognitoContentVisible)
+                  forKey:kIncognitoCurrentKey];
+
   [self.observers sceneState:self
       isDisplayingIncognitoContent:incognitoContentVisible];
 }
@@ -229,4 +283,67 @@
       [NSString stringWithFormat:@"SceneState %p (%@)", self, activityString];
 }
 
+#pragma mark - Session scoped defaults.
+
+// Helper methods to get/set values that are "per-scene" (such as whether the
+// incognito or regular UI is presented, ...). Those methods store/fetch the
+// values from -userInfo property of UISceneSession for devices that support
+// multi-window or in NSUserDefaults for other device.
+//
+// The reason the values are not always stored in UISceneSession -userInfo is
+// that iOS consider that the "swipe gesture" can mean "close the window" even
+// on device that do not support multi-window (such as iPhone) if multi-window
+// support is enabled. As enabling the support is done in the Info.plist and
+// Chrome does not want to distribute a different app to phones and tablets,
+// this means that on iPhone the scene may be closed by the OS and the session
+// destroyed. On device that support multi-window, the user has the option to
+// re-open the window via a shortcut presented by the OS, but there is no such
+// options for device that do not support multi-window.
+//
+// Finally, the methods also support moving the value from NSUserDefaults to
+// UISceneSession -userInfo as required when Chrome is updated from an old
+// version to one where multi-window is enabled (or when the users upgrade
+// their devices).
+//
+// The heuristic is:
+// -  if the device does not support multi-window, NSUserDefaults is used,
+// -  otherwise, the value is first looked up in UISceneSession -userInfo,
+//    if present, it is used (and any copy in NSUserDefaults is deleted),
+//    if not present, the value is looked in NSUserDefaults.
+
+- (NSObject*)sessionObjectForKey:(NSString*)key {
+  if (@available(ios 13, *)) {
+    if (IsMultipleScenesSupported()) {
+      NSObject* value = [_scene.session.userInfo objectForKey:key];
+      if (value) {
+        NSUserDefaults* userDefaults = [NSUserDefaults standardUserDefaults];
+        if ([userDefaults objectForKey:key]) {
+          [userDefaults removeObjectForKey:key];
+          [userDefaults synchronize];
+        }
+        return value;
+      }
+    }
+  }
+
+  NSUserDefaults* userDefaults = [NSUserDefaults standardUserDefaults];
+  return [userDefaults objectForKey:key];
+}
+
+- (void)setSessionObject:(NSObject*)object forKey:(NSString*)key {
+  if (@available(ios 13, *)) {
+    if (IsMultipleScenesSupported()) {
+      NSMutableDictionary<NSString*, id>* userInfo = [NSMutableDictionary
+          dictionaryWithDictionary:_scene.session.userInfo];
+      [userInfo setObject:object forKey:key];
+      _scene.session.userInfo = userInfo;
+      return;
+    }
+  }
+
+  NSUserDefaults* userDefaults = [NSUserDefaults standardUserDefaults];
+  [userDefaults setObject:object forKey:key];
+  [userDefaults synchronize];
+}
+
 @end