[multiball] Fix safe mode in multiwindow.

1. AppState now advertises when safe mode has completed.
2. SceneControllers now don't do anything while Safe Mode is active.
3. AppState now uses a foreground scene to display safe mode.
4. MainController waits for Safe Mode to complete before any
scene-event-related setup.
5. Scenes now observe AppState, and access it directly
(and not through MainController).

Bug: 1080907
Change-Id: Ifffcd7cf7dd822d068268cc4a3709221acea2caf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2193657
Commit-Queue: Stepan Khapugin <stkhapugin@chromium.org>
Reviewed-by: Mark Cogan <marq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#770820}

ios/chrome/app/application_delegate/app_state.h

@@ -25,6 +25,9 @@
 - (void)appState:(AppState*)appState
     firstSceneActivated:(SceneState*)sceneState;
 
+// Called after the app exits safe mode.
+- (void)appStateDidExitSafeMode:(AppState*)appState;
+
 @end
 
 // Represents the application state and responds to application state changes
@@ -43,9 +46,9 @@ initWithBrowserLauncher:(id<BrowserLauncher>)browserLauncher
 // application has been woken up by the system for background work.
 @property(nonatomic, readonly) BOOL userInteracted;
 
-// Window for the application, it is not set during the initialization method.
-// Set the property before calling methods related to it.
-@property(nonatomic, weak) UIWindow* window;
+// Current foreground active for the application, if any. Some scene's window
+// otherwise. For legacy use cases only, use scene windows instead.
+@property(nonatomic, readonly) UIWindow* window;
 
 // When multiwindow is unavailable, this is the only scene state. It is created
 // by the app delegate.

ios/chrome/app/application_delegate/app_state.mm

@@ -93,8 +93,6 @@ NSString* const kStartupAttemptReset = @"StartupAttempReset";
   __weak id<BrowserLauncher> _browserLauncher;
   // UIApplicationDelegate for the application.
   __weak MainApplicationDelegate* _mainApplicationDelegate;
-  // Window for the application.
-  __weak UIWindow* _window;
 
   // Variables backing properties of same name.
   SafeModeCoordinator* _safeModeCoordinator;
@@ -121,6 +119,9 @@ NSString* const kStartupAttemptReset = @"StartupAttempReset";
 // mode UI.
 @property(nonatomic, strong) SafeModeCoordinator* safeModeCoordinator;
 
+// Flag to track when the app is in safe mode.
+@property(nonatomic, assign, getter=isInSafeMode) BOOL inSafeMode;
+
 // Return value for -requiresHandlingAfterLaunchWithOptions that determines if
 // UIKit should make followup delegate calls such as
 // -performActionForShortcutItem or -openURL.
@@ -182,12 +183,9 @@ initWithBrowserLauncher:(id<BrowserLauncher>)browserLauncher
   _safeModeCoordinator = safeModeCoordinator;
 }
 
-- (void)setWindow:(UIWindow*)window {
-  _window = window;
-}
-
 - (UIWindow*)window {
-  return _window;
+  return self.foregroundActiveScene ? self.foregroundActiveScene.window
+                                    : self.connectedScenes.firstObject.window;
 }
 
 - (void)setsceneShowingBlockingUI:(SceneState*)sceneState {
@@ -497,10 +495,6 @@ initWithBrowserLauncher:(id<BrowserLauncher>)browserLauncher
   return self.shouldPerformAdditionalDelegateHandling;
 }
 
-- (BOOL)isInSafeMode {
-  return self.safeModeCoordinator != nil;
-}
-
 - (void)launchFromURLHandled:(BOOL)URLHandled {
   self.shouldPerformAdditionalDelegateHandling = !URLHandled;
 }
@@ -554,29 +548,41 @@ initWithBrowserLauncher:(id<BrowserLauncher>)browserLauncher
 
 - (void)coordinatorDidExitSafeMode:(nonnull SafeModeCoordinator*)coordinator {
   self.safeModeCoordinator = nil;
+  self.inSafeMode = NO;
   [_browserLauncher startUpBrowserToStage:INITIALIZATION_STAGE_FOREGROUND];
+  [self.observers appStateDidExitSafeMode:self];
+
   [_mainApplicationDelegate
       applicationDidBecomeActive:[UIApplication sharedApplication]];
 }
 
 #pragma mark - Internal methods.
 
+- (void)startSafeMode {
+  DCHECK(self.foregroundActiveScene);
+  SafeModeCoordinator* safeModeCoordinator =
+      [[SafeModeCoordinator alloc] initWithWindow:self.window];
+
+  self.safeModeCoordinator = safeModeCoordinator;
+  [self.safeModeCoordinator setDelegate:self];
+
+  // Activate the main window, which will prompt the views to load.
+  [self.window makeKeyAndVisible];
+
+  [self.safeModeCoordinator start];
+}
+
 - (void)initializeUI {
   _userInteracted = YES;
   [self saveLaunchDetailsToDefaults];
 
-  DCHECK([_window rootViewController] == nil);
   if ([SafeModeCoordinator shouldStart]) {
-    SafeModeCoordinator* safeModeCoordinator =
-        [[SafeModeCoordinator alloc] initWithWindow:_window];
-
-    self.safeModeCoordinator = safeModeCoordinator;
-    [self.safeModeCoordinator setDelegate:self];
-
-    // Activate the main window, which will prompt the views to load.
-    [_window makeKeyAndVisible];
-
-    [self.safeModeCoordinator start];
+    self.inSafeMode = YES;
+    if (!IsMultiwindowSupported()) {
+      // Start safe mode immediately. Otherwise it should only start when a
+      // scene is connected and activates to allow displaying the safe mode UI.
+      [self startSafeMode];
+    }
     return;
   }
 
@@ -617,6 +623,12 @@ initWithBrowserLauncher:(id<BrowserLauncher>)browserLauncher
           base::mac::ObjCCastStrict<SceneDelegate>(scene.delegate);
       [self.observers appState:self
            firstSceneActivated:sceneDelegate.sceneState];
+
+      if (self.isInSafeMode) {
+        // Safe mode can only be started when there's a window, so the actual
+        // safe mode has been postponed until now.
+        [self startSafeMode];
+      }
       sceneDelegate.sceneState.presentingModalOverlay =
           self.sceneShowingBlockingUI &&
           (self.sceneShowingBlockingUI != sceneDelegate.sceneState);

ios/chrome/app/application_delegate/app_state_unittest.mm

@@ -270,7 +270,8 @@ class AppStateTest : public BlockCleanupTest {
                                 memoryHelper:memoryHelper
                                    tabOpener:tabOpener];
     // TODO(crbug.com/1065815): Inject scene states for multiwindow as well.
-    app_state_.mainSceneState = [[FakeSceneState alloc] init];
+    app_state_.mainSceneState =
+        [[FakeSceneState alloc] initWithAppState:app_state_];
     initializeIncognitoBlocker(window);
 
     return appState;
@@ -283,8 +284,8 @@ class AppStateTest : public BlockCleanupTest {
                                  startupInformation:startup_information_mock_
                                 applicationDelegate:main_application_delegate_];
       // TODO(crbug.com/1065815): Inject scene states for multiwindow as well.
-      app_state_.mainSceneState = [[FakeSceneState alloc] init];
-      [app_state_ setWindow:window_];
+      app_state_.mainSceneState =
+          [[FakeSceneState alloc] initWithAppState:app_state_];
     }
     return app_state_;
   }
@@ -296,8 +297,8 @@ class AppStateTest : public BlockCleanupTest {
                                  startupInformation:startup_information_mock_
                                 applicationDelegate:main_application_delegate_];
       // TODO(crbug.com/1065815): Inject scene states for multiwindow as well.
-      app_state_.mainSceneState = [[FakeSceneState alloc] init];
-      [app_state_ setWindow:window];
+      app_state_.mainSceneState =
+          [[FakeSceneState alloc] initWithAppState:app_state_];
       [window makeKeyAndVisible];
     }
     return app_state_;
@@ -350,25 +351,6 @@ class AppStateWithThreadTest : public PlatformTest {
 
 #pragma mark - Tests.
 
-// Tests -isInSafeMode returns true if there is a SafeModeController.
-TEST_F(AppStateTest, isInSafeModeTest) {
-  // Setup.
-  id safeModeContollerMock =
-      [OCMockObject mockForClass:[SafeModeCoordinator class]];
-
-  AppState* appState = getAppStateWithMock();
-
-  appState.safeModeCoordinator = nil;
-  ASSERT_FALSE([appState isInSafeMode]);
-  [appState setSafeModeCoordinator:safeModeContollerMock];
-
-  // Action.
-  BOOL result = [appState isInSafeMode];
-
-  // Test.
-  EXPECT_TRUE(result);
-}
-
 // Tests that if the application is in background
 // -requiresHandlingAfterLaunchWithOptions saves the launchOptions and returns
 // YES (to handle the launch options later).
@@ -419,6 +401,10 @@ TEST_F(AppStateTest, requiresHandlingAfterLaunchWithOptionsForegroundSafeMode) {
 
   ASSERT_FALSE([appState isInSafeMode]);
 
+  appState.mainSceneState.activationLevel =
+      SceneActivationLevelForegroundActive;
+  appState.mainSceneState.window = getWindowMock();
+
   // Action.
   BOOL result = [appState requiresHandlingAfterLaunchWithOptions:launchOptions
                                                  stateBackground:NO];
@@ -484,7 +470,6 @@ TEST_F(AppStateNoFixtureTest, willResignActive) {
 
   id applicationDelegate =
       [OCMockObject mockForClass:[MainApplicationDelegate class]];
-  id window = [OCMockObject mockForClass:[UIWindow class]];
 
   FakeStartupInformation* startupInformation =
       [[FakeStartupInformation alloc] init];
@@ -494,7 +479,6 @@ TEST_F(AppStateNoFixtureTest, willResignActive) {
       [[AppState alloc] initWithBrowserLauncher:browserLauncher
                              startupInformation:startupInformation
                             applicationDelegate:applicationDelegate];
-  [appState setWindow:window];
 
   ASSERT_TRUE([startupInformation isColdStart]);
 
@@ -515,7 +499,6 @@ TEST_F(AppStateWithThreadTest, willTerminate) {
       [OCMockObject mockForProtocol:@protocol(BrowserLauncher)];
   id applicationDelegate =
       [OCMockObject mockForClass:[MainApplicationDelegate class]];
-  id window = [OCMockObject mockForClass:[UIWindow class]];
   StubBrowserInterfaceProvider* interfaceProvider =
       [[StubBrowserInterfaceProvider alloc] init];
   interfaceProvider.mainInterface.userInteractionEnabled = YES;
@@ -532,7 +515,6 @@ TEST_F(AppStateWithThreadTest, willTerminate) {
       [[AppState alloc] initWithBrowserLauncher:browserLauncher
                              startupInformation:startupInformation
                             applicationDelegate:applicationDelegate];
-  [appState setWindow:window];
 
   id application = [OCMockObject mockForClass:[UIApplication class]];
 
@@ -796,10 +778,19 @@ TEST_F(AppStateTest,
       browserInitializationStage];
 
   [[[window stub] andReturn:nil] rootViewController];
-  [[window expect] makeKeyAndVisible];
   [[window stub] setRootViewController:[OCMArg any]];
   swizzleSafeModeShouldStart(YES);
 
+  // The helper below calls makeKeyAndVisible.
+  [[window expect] makeKeyAndVisible];
+  AppState* appState = getAppStateWithRealWindow(window);
+
+  // Starting safe mode will call makeKeyAndVisible on the window.
+  [[window expect] makeKeyAndVisible];
+  appState.mainSceneState.activationLevel =
+      SceneActivationLevelForegroundActive;
+  appState.mainSceneState.window = window;
+
   // Actions.
   [getAppStateWithMock() applicationWillEnterForeground:application
                                         metricsMediator:metricsMediator
@@ -811,34 +802,6 @@ TEST_F(AppStateTest,
   EXPECT_TRUE([getAppStateWithMock() isInSafeMode]);
 }
 
-// Tests that -applicationWillEnterForeground returns directly if the
-// application is in safe mode and in foreground
-TEST_F(AppStateTest, applicationWillEnterForegroundFromForegroundSafeMode) {
-  // Setup.
-  id application = [OCMockObject mockForClass:[UIApplication class]];
-  id metricsMediator = [OCMockObject mockForClass:[MetricsMediator class]];
-  id memoryHelper = [OCMockObject mockForClass:[MemoryWarningHelper class]];
-  id tabOpener = [OCMockObject mockForProtocol:@protocol(TabOpening)];
-
-  BrowserInitializationStageType stage = INITIALIZATION_STAGE_FOREGROUND;
-  [[[getBrowserLauncherMock() stub] andReturnValue:@(stage)]
-      browserInitializationStage];
-
-  AppState* appState = getAppStateWithMock();
-
-  UIWindow* window = [[UIWindow alloc] init];
-  appState.safeModeCoordinator =
-      [[SafeModeCoordinator alloc] initWithWindow:window];
-
-  ASSERT_TRUE([appState isInSafeMode]);
-
-  // Actions.
-  [appState applicationWillEnterForeground:application
-                           metricsMediator:metricsMediator
-                              memoryHelper:memoryHelper
-                                 tabOpener:tabOpener];
-}
-
 // Tests that -applicationDidEnterBackground creates an incognito blocker.
 TEST_F(AppStateTest, applicationDidEnterBackgroundIncognito) {
   // Setup.

ios/chrome/app/main_application_delegate.h

@@ -7,8 +7,14 @@
 
 #import <UIKit/UIKit.h>
 
+@class AppState;
+
 // The main delegate of the application.
 @interface MainApplicationDelegate : NSObject<UIApplicationDelegate>
+
+// Handles the application stage changes.
+@property(nonatomic, strong) AppState* appState;
+
 @end
 
 #endif  // IOS_CHROME_APP_MAIN_APPLICATION_DELEGATE_H_

ios/chrome/app/main_application_delegate.mm

@@ -43,8 +43,6 @@
   id<StartupInformation> _startupInformation;
   // Helper to open new tabs.
   id<TabOpening> _tabOpener;
-  // Handles the application stage changes.
-  AppState* _appState;
   // Handles tab switcher.
   id<TabSwitching> _tabSwitcherProtocol;
 }
@@ -79,7 +77,7 @@
       // When the UIScene APU is not supported, this object holds a "scene"
       // state and a "scene" controller. This allows the rest of the app to be
       // mostly multiwindow-agnostic.
-      _sceneState = [[SceneState alloc] init];
+      _sceneState = [[SceneState alloc] initWithAppState:_appState];
       _appState.mainSceneState = _sceneState;
       _sceneController =
           [[SceneController alloc] initWithSceneState:_sceneState];
@@ -117,8 +115,6 @@
   startup_loggers::RegisterAppDidFinishLaunchingTime();
 
   _mainController.window = self.window;
-  // self.window has been set by this time. _appState window can now be set.
-  _appState.window = self.window;
 
   BOOL inBackground =
       [application applicationState] == UIApplicationStateBackground;

ios/chrome/app/main_application_delegate_testing.h

@@ -12,7 +12,6 @@
 
 @interface MainApplicationDelegate ()
 @property(nonatomic, readonly) MainController* mainController;
-@property(nonatomic, readonly) AppState* appState;
 
 + (MainController*)sharedMainController;
 + (AppState*)sharedAppState;

ios/chrome/app/main_controller.mm

@@ -617,6 +617,13 @@ void MainControllerAuthenticationServiceDelegate::ClearBrowsingData(
 // Called when the first scene becomes active.
 - (void)appState:(AppState*)appState
     firstSceneActivated:(SceneState*)sceneState {
+  if (appState.isInSafeMode) {
+    return;
+  }
+  [self startUpAfterFirstWindowCreated];
+}
+
+- (void)appStateDidExitSafeMode:(AppState*)appState {
   [self startUpAfterFirstWindowCreated];
 }
 

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

@@ -118,7 +118,8 @@ const NSTimeInterval kDisplayPromoDelay = 0.1;
 
 }  // namespace
 
-@interface SceneController () <UserFeedbackDataSource,
+@interface SceneController () <AppStateObserver,
+                               UserFeedbackDataSource,
                                SettingsNavigationControllerDelegate,
                                SceneURLLoadingServiceDelegate,
                                WebStateListObserving> {
@@ -206,6 +207,7 @@ const NSTimeInterval kDisplayPromoDelay = 0.1;
   if (self) {
     _sceneState = sceneState;
     [_sceneState addObserver:self];
+    [_sceneState.appState addObserver:self];
     // The window is necessary very early in the app/scene lifecycle, so it
     // should be created right away.
     // When multiwindow is supported, the window is created by SceneDelegate,
@@ -264,6 +266,14 @@ const NSTimeInterval kDisplayPromoDelay = 0.1;
 
 - (void)sceneState:(SceneState*)sceneState
     transitionedToActivationLevel:(SceneActivationLevel)level {
+  AppState* appState = self.sceneState.appState;
+  if (appState.isInSafeMode) {
+    // Nothing at all should happen in safe mode. Code in
+    // appStateDidExitSafeMode will ensure the updates happen once safe mode
+    // ends.
+    return;
+  }
+
   if (level > SceneActivationLevelBackground && !self.hasInitializedUI) {
     [self initializeUI];
   }
@@ -310,6 +320,14 @@ const NSTimeInterval kDisplayPromoDelay = 0.1;
   }
 }
 
+#pragma mark - AppStateObserver
+
+- (void)appStateDidExitSafeMode:(AppState*)appState {
+  // All events were postponed in safe mode. Resend them.
+  [self sceneState:self.sceneState
+      transitionedToActivationLevel:self.sceneState.activationLevel];
+}
+
 #pragma mark - SceneControllerGuts
 - (void)initializeUI {
   if (self.hasInitializedUI) {

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

@@ -16,7 +16,7 @@ namespace {
 class SceneControllerTest : public PlatformTest {
  protected:
   SceneControllerTest() {
-    scene_state_ = [[SceneState alloc] init];
+    scene_state_ = [[SceneState alloc] initWithAppState:nil];
     scene_controller_ =
         [[SceneController alloc] initWithSceneState:scene_state_];
   }

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

@@ -6,6 +6,7 @@
 
 #include "base/mac/foundation_util.h"
 #import "ios/chrome/app/chrome_overlay_window.h"
+#import "ios/chrome/app/main_application_delegate.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
 #error "This file requires ARC support."
@@ -15,7 +16,10 @@
 
 - (SceneState*)sceneState {
   if (!_sceneState) {
-    _sceneState = [[SceneState alloc] init];
+    MainApplicationDelegate* appDelegate =
+        base::mac::ObjCCastStrict<MainApplicationDelegate>(
+            UIApplication.sharedApplication.delegate);
+    _sceneState = [[SceneState alloc] initWithAppState:appDelegate.appState];
     _sceneController = [[SceneController alloc] initWithSceneState:_sceneState];
     _sceneState.controller = _sceneController;
   }

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

@@ -7,6 +7,7 @@
 
 #import <UIKit/UIKit.h>
 
+@class AppState;
 @class SceneController;
 @class SceneState;
 @protocol BrowserInterfaceProvider;
@@ -47,6 +48,12 @@ typedef NS_ENUM(NSUInteger, SceneActivationLevel) {
 // corresponds to one scene.
 @interface SceneState : NSObject
 
+- (instancetype)initWithAppState:(AppState*)appState NS_DESIGNATED_INITIALIZER;
+- (instancetype)init NS_UNAVAILABLE;
+
+// The app state for the app that owns this scene. Set in init.
+@property(nonatomic, weak, readonly) AppState* appState;
+
 // The current activation level.
 @property(nonatomic, assign) SceneActivationLevel activationLevel;
 

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

@@ -32,9 +32,10 @@
 @synthesize window = _window;
 @synthesize windowID = _windowID;
 
-- (instancetype)init {
+- (instancetype)initWithAppState:(AppState*)appState {
   self = [super init];
   if (self) {
+    _appState = appState;
     _observers = [SceneStateObserverList
         observersWithProtocol:@protocol(SceneStateObserver)];
     if (@available(iOS 13, *)) {

ios/chrome/browser/ui/main/test/fake_scene_state.mm

@@ -29,8 +29,8 @@
 
 @synthesize interfaceProvider = _interfaceProvider;
 
-- (instancetype)init {
-  if (self = [super init]) {
+- (instancetype)initWithAppState:(AppState*)appState {
+  if (self = [super initWithAppState:appState]) {
     self.activationLevel = SceneActivationLevelForegroundInactive;
     self.interfaceProvider = [[StubBrowserInterfaceProvider alloc] init];
     StubBrowserInterface* mainInterface = static_cast<StubBrowserInterface*>(
@@ -44,7 +44,7 @@
 + (NSArray<FakeSceneState*>*)sceneArrayWithCount:(int)count {
   NSMutableArray<SceneState*>* scenes = [NSMutableArray array];
   for (int i = 0; i < count; i++) {
-    [scenes addObject:[[self alloc] init]];
+    [scenes addObject:[[self alloc] initWithAppState:nil]];
   }
   return [scenes copy];
 }

ios/chrome/browser/ui/safe_mode/safe_mode_coordinator.mm

@@ -50,6 +50,7 @@ const int kStartupCrashLoopThreshold = 2;
   // method.
   SafeModeViewController* viewController =
       [[SafeModeViewController alloc] initWithDelegate:self];
+  DCHECK(self.window);
   [self.window setRootViewController:viewController];
 
   // Reset the crash count; the user may change something based on the recovery