Incognito Reauth: Fix reauth screen not restored when activity is killed

Under memory pressure (emulated by the "Don't keep activities" setting), IncognitoReauthController gets killed after starting the reauthentication flow. When restored, savedInstanceState is checked for incognitoReauthPending when re-initializing the IncognitoReauthController, to determine whether to resume the reauthentication flow.

The problem lies when the SavedInstanceState is reset in
ChromeTabbedActivity::onStartWithNative which sets it to null before it
is checked for the incognitoReauthPending key in
RootUiCoordinator::onFinishNativeInitialization, making
mIsIncognitoReauthPendingOnRestore in IncognitoReauthController always
return false and the reauth screen not getting restored when the
activity is killed.

This CL saves the incognitoReauthPending key value from
SavedInstanceState in the the constructor of RootUiCoordinator before
it is reset.

(cherry picked from commit 318d0b3)

Demo: https://drive.google.com/file/d/16nBDAFA3bIk065EsGGmynMLTY_zU7Uop/view?usp=sharing
Bug: 1365633
Change-Id: I8a1484474ab8d7a75a9316b38cc0313199ec890e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4629112
Reviewed-by: Rohit Agarwal <roagarwal@chromium.org>
Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Boris Sazonov <bsazonov@chromium.org>
Commit-Queue: Zaina Al-Mashni <zalmashni@google.com>
Cr-Original-Commit-Position: refs/heads/main@{#1162306}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4649429
Cr-Commit-Position: refs/branch-heads/5845@{#132}
Cr-Branched-From: 5a5dff6-refs/heads/main@{#1160321}

chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java

@@ -1759,7 +1759,7 @@ protected RootUiCoordinator createRootUiCoordinator() {
                 // TODO(sinansahin): This currently only checks for incognito extras in the intent.
                 // We should make it more robust by using more signals.
                 IntentHandler.hasAnyIncognitoExtra(getIntent().getExtras()), mBackPressManager,
-                this::getSavedInstanceState);
+                getSavedInstanceState());
     }
 
     @Override

chrome/android/java/src/org/chromium/chrome/browser/app/ChromeActivity.java

@@ -532,7 +532,7 @@ mTabCreatorManagerSupplier, getFullscreenManager(), mCompositorViewHolderSupplie
                 /* statusBarColorProvider= */ this, getIntentRequestTracker(),
                 mTabReparentingControllerSupplier,
                 /*ephemeralTabCoordinatorSupplier=*/new ObservableSupplierImpl<>(),
-                false, mBackPressManager, ()->null);
+                false, mBackPressManager, null);
         // clang-format on
     }
 

chrome/android/java/src/org/chromium/chrome/browser/customtabs/BaseCustomTabRootUiCoordinator.java

@@ -190,7 +190,7 @@ public BaseCustomTabRootUiCoordinator(@NonNull AppCompatActivity activity,
                 snackbarManagerSupplier, activityType,
                 isInOverviewModeSupplier, isWarmOnResumeSupplier, appMenuDelegate,
                 statusBarColorProvider, intentRequestTracker, new OneshotSupplierImpl<>(),
-                ephemeralTabCoordinatorSupplier, false, backPressManager, ()->null);
+                ephemeralTabCoordinatorSupplier, false, backPressManager, null);
         // clang-format on
         mToolbarCoordinator = customTabToolbarCoordinator;
         mNavigationController = customTabNavigationController;

chrome/android/java/src/org/chromium/chrome/browser/tabbed_mode/TabbedRootUiCoordinator.java

@@ -254,7 +254,7 @@ public void destroy() {
      * @param tabReparentingControllerSupplier Supplier for the {@link TabReparentingController}.
      * @param initializeUiWithIncognitoColors Whether to initialize the UI with incognito colors.
      * @param backPressManager The {@link BackPressManager} handling back press.
-     * @param savedInstanceStateSupplier Supplies the saved instance state.
+     * @param savedInstanceState The saved bundle for the last recorded state.
      */
     public TabbedRootUiCoordinator(@NonNull AppCompatActivity activity,
             @Nullable Callback<Boolean> onOmniboxFocusChangedListener,
@@ -296,7 +296,7 @@ public TabbedRootUiCoordinator(@NonNull AppCompatActivity activity,
             @NonNull Function<Tab, Boolean> backButtonShouldCloseTabFn,
             OneshotSupplier<TabReparentingController> tabReparentingControllerSupplier,
             boolean initializeUiWithIncognitoColors, @NonNull BackPressManager backPressManager,
-            @NonNull Supplier<Bundle> savedInstanceStateSupplier) {
+            @Nullable Bundle savedInstanceState) {
         super(activity, onOmniboxFocusChangedListener, shareDelegateSupplier, tabProvider,
                 profileSupplier, bookmarkModelSupplier, tabBookmarkerSupplier,
                 contextualSearchManagerSupplier, tabModelSelectorSupplier, startSurfaceSupplier,
@@ -310,7 +310,7 @@ public TabbedRootUiCoordinator(@NonNull AppCompatActivity activity,
                 activityType, isInOverviewModeSupplier, isWarmOnResumeSupplier, appMenuDelegate,
                 statusBarColorProvider, intentRequestTracker, tabReparentingControllerSupplier,
                 ephemeralTabCoordinatorSupplier, initializeUiWithIncognitoColors, backPressManager,
-                savedInstanceStateSupplier);
+                savedInstanceState);
         mControlContainerHeightResource = controlContainerHeightResource;
         mInsetObserverViewSupplier = insetObserverViewSupplier;
         mBackButtonShouldCloseTabFn = backButtonShouldCloseTabFn;

chrome/android/java/src/org/chromium/chrome/browser/ui/RootUiCoordinator.java

@@ -309,7 +309,7 @@ public class RootUiCoordinator
     private final Supplier<EphemeralTabCoordinator> mEphemeralTabCoordinatorSupplier;
     @Nullable
     private final BackPressManager mBackPressManager;
-    private final @NonNull Supplier<Bundle> mSavedInstanceStateSupplier;
+    private final boolean mIsIncognitoReauthPendingOnRestore;
     protected final ExpandedSheetHelper mExpandedBottomSheetHelper;
     @Nullable
     private PageZoomCoordinator mPageZoomCoordinator;
@@ -364,7 +364,7 @@ public class RootUiCoordinator
      * @param ephemeralTabCoordinatorSupplier Supplies the {@link EphemeralTabCoordinator}.
      * @param initializeUiWithIncognitoColors Whether to initialize the UI with incognito colors.
      * @param backPressManager The {@link BackPressManager} handling back press.
-     * @param savedInstanceStateSupplier Supplies the saved instance state.
+     * @param savedInstanceState The saved bundle for the last recorded state.
      */
     public RootUiCoordinator(@NonNull AppCompatActivity activity,
             @Nullable Callback<Boolean> onOmniboxFocusChangedListener,
@@ -403,7 +403,7 @@ public RootUiCoordinator(@NonNull AppCompatActivity activity,
             @NonNull OneshotSupplier<TabReparentingController> tabReparentingControllerSupplier,
             @NonNull Supplier<EphemeralTabCoordinator> ephemeralTabCoordinatorSupplier,
             boolean initializeUiWithIncognitoColors, @Nullable BackPressManager backPressManager,
-            @NonNull Supplier<Bundle> savedInstanceStateSupplier) {
+            @Nullable Bundle savedInstanceState) {
         mCallbackController = new CallbackController();
         mActivity = activity;
         mWindowAndroid = windowAndroid;
@@ -430,7 +430,9 @@ public RootUiCoordinator(@NonNull AppCompatActivity activity,
         mTabReparentingControllerSupplier = tabReparentingControllerSupplier;
         mInitializeUiWithIncognitoColors = initializeUiWithIncognitoColors;
         mBackPressManager = backPressManager;
-        mSavedInstanceStateSupplier = savedInstanceStateSupplier;
+        mIsIncognitoReauthPendingOnRestore = savedInstanceState != null
+                && savedInstanceState.getBoolean(
+                        IncognitoReauthControllerImpl.KEY_IS_INCOGNITO_REAUTH_PENDING, false);
 
         mMenuOrKeyboardActionController = menuOrKeyboardActionController;
         mMenuOrKeyboardActionController.registerMenuOrKeyboardActionHandler(this);
@@ -816,14 +818,11 @@ private void initIncognitoReauthController() {
                 getIncognitoReauthCoordinatorFactory();
         assert incognitoReauthCoordinatorFactory
                 != null : "Sub-classes need to provide a valid factory instance.";
-        boolean isIncognitoReauthPendingOnRestore = mSavedInstanceStateSupplier.hasValue()
-                && mSavedInstanceStateSupplier.get().getBoolean(
-                        IncognitoReauthControllerImpl.KEY_IS_INCOGNITO_REAUTH_PENDING, false);
         mIncognitoReauthController =
                 new IncognitoReauthControllerImpl(mTabModelSelectorSupplier.get(),
                         mActivityLifecycleDispatcher, mLayoutStateProviderOneShotSupplier,
                         mProfileSupplier, incognitoReauthCoordinatorFactory,
-                        () -> isIncognitoReauthPendingOnRestore, mActivity.getTaskId());
+                        () -> mIsIncognitoReauthPendingOnRestore, mActivity.getTaskId());
         mIncognitoReauthControllerOneshotSupplier.set(mIncognitoReauthController);
     }
 

chrome/browser/incognito/android/javatests/src/org/chromium/chrome/browser/incognito/reauth/TabSwitcherIncognitoReauthViewTest.java

@@ -14,9 +14,11 @@
 import static org.hamcrest.core.IsNot.not;
 import static org.junit.Assert.assertTrue;
 
-import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.createTabs;
+import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.addBlankTabs;
 import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.enterTabSwitcher;
+import static org.chromium.ui.test.util.ViewUtils.onViewWaiting;
 
+import androidx.test.filters.LargeTest;
 import androidx.test.filters.MediumTest;
 
 import org.junit.After;
@@ -38,6 +40,7 @@
 import org.chromium.chrome.browser.lifecycle.StartStopWithNativeObserver;
 import org.chromium.chrome.browser.preferences.Pref;
 import org.chromium.chrome.browser.profiles.Profile;
+import org.chromium.chrome.browser.tab.TabStateExtractor;
 import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
 import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
 import org.chromium.chrome.test.util.ChromeRenderTestRule;
@@ -113,7 +116,8 @@ private void triggerIncognitoReauthCustomView(ChromeTabbedActivity cta) {
 
     private void openIncognitoReauth(ChromeTabbedActivity cta) {
         // Open incognito tab.
-        createTabs(cta, true, 1);
+        addBlankTabs(cta, true, 1);
+
         assertTrue(cta.getActivityTab().isIncognito());
 
         // Enter tab switcher in incognito mode.
@@ -154,4 +158,21 @@ public void testIncognitoReauthView_TabSwitcherRenderTest() throws IOException {
         mRenderTestRule.render(
                 cta.findViewById(R.id.action_bar_root), "incognito_reauth_view_tab_switcher");
     }
+
+    @Test
+    @LargeTest
+    public void testIncognitoReauthViewIsRestored_WhenActivityIsKilled() {
+        final ChromeTabbedActivity cta = mActivityTestRule.getActivity();
+        openIncognitoReauth(cta);
+
+        // Need to wait for contentsState to be initialized for the tab to restore correctly.
+        CriteriaHelper.pollUiThread(() -> {
+            return TabStateExtractor.from(cta.getActivityTab()).contentsState != null;
+        });
+
+        mActivityTestRule.recreateActivity();
+
+        onViewWaiting(withId(R.id.incognito_reauth_unlock_incognito_button))
+                .check(matches(isDisplayed()));
+    }
 }