[Signin][Android] Fix ConfirmSyncDataStateMachine crash

This CL fixes the ConfirmSyncDataStateMachine crash related to the
DialogFragment update to androidx.

Bug: 1083833
Change-Id: I7f78f491f1b8e66668b1f5552c1483ac68c48624
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2216070
Commit-Queue: Alice Wang <aliceywang@chromium.org>
Reviewed-by: Boris Sazonov <bsazonov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#772229}

chrome/android/chrome_test_java_sources.gni

@@ -455,6 +455,7 @@ chrome_test_java_sources = [
   "javatests/src/org/chromium/chrome/browser/share/ShareUrlTest.java",
   "javatests/src/org/chromium/chrome/browser/signin/AccountPickerDialogFragmentTest.java",
   "javatests/src/org/chromium/chrome/browser/signin/ConfirmManagedSyncDataDialogIntegrationTest.java",
+  "javatests/src/org/chromium/chrome/browser/signin/ConfirmSyncDataIntegrationTest.java",
   "javatests/src/org/chromium/chrome/browser/signin/DummyAccountPickerTargetFragment.java",
   "javatests/src/org/chromium/chrome/browser/signin/IdentityManagerIntegrationTest.java",
   "javatests/src/org/chromium/chrome/browser/signin/ProfileDataCacheRenderTest.java",

chrome/android/java/src/org/chromium/chrome/browser/signin/ConfirmImportSyncDataDialog.java

@@ -53,7 +53,6 @@ public class ConfirmImportSyncDataDialog extends DialogFragment
     private RadioButtonWithDescription mKeepSeparateOption;
 
     private Listener mListener;
-    private boolean mListenerCalled;
 
     /**
      * Creates a new instance of ConfirmImportSyncDataDialog, a dialog that gives the
@@ -76,15 +75,13 @@ public class ConfirmImportSyncDataDialog extends DialogFragment
     }
 
     private void setListener(Listener listener) {
-        assert mListener == null;
+        assert listener != null;
         mListener = listener;
     }
 
     @Override
     public Dialog onCreateDialog(Bundle savedInstanceState) {
-        // If the dialog is being recreated it won't have the listener set and so won't be
-        // functional. Therefore we dismiss, and the user will need to open the dialog again.
-        if (savedInstanceState != null) {
+        if (mListener == null) {
             dismiss();
         }
         String oldAccountName = getArguments().getString(KEY_OLD_ACCOUNT_NAME);
@@ -127,8 +124,6 @@ public class ConfirmImportSyncDataDialog extends DialogFragment
 
     @Override
     public void onClick(DialogInterface dialog, int which) {
-        if (mListener == null) return;
-
         if (which == AlertDialog.BUTTON_POSITIVE) {
             assert mConfirmImportOption.isChecked() ^ mKeepSeparateOption.isChecked();
 
@@ -142,15 +137,12 @@ public class ConfirmImportSyncDataDialog extends DialogFragment
             RecordUserAction.record("Signin_ImportDataPrompt_Cancel");
             mListener.onCancel();
         }
-        mListenerCalled = true;
     }
 
     @Override
-    public void onDismiss(DialogInterface dialog) {
-        super.onDismiss(dialog);
-        if (mListener != null && !mListenerCalled) {
-            mListener.onCancel();
-        }
+    public void onCancel(DialogInterface dialog) {
+        super.onCancel(dialog);
+        mListener.onCancel();
     }
 }
 

chrome/android/java/src/org/chromium/chrome/browser/signin/ConfirmManagedSyncDataDialog.java

@@ -38,7 +38,6 @@ public class ConfirmManagedSyncDataDialog extends DialogFragment
     private static final String KEY_DOMAIN = "domain";
 
     private Listener mListener;
-    private boolean mListenerCalled;
 
     /**
      * Creates {@link ConfirmManagedSyncDataDialog} when signing in to a managed account
@@ -87,15 +86,12 @@ public class ConfirmManagedSyncDataDialog extends DialogFragment
             assert which == AlertDialog.BUTTON_NEGATIVE;
             mListener.onCancel();
         }
-        mListenerCalled = true;
     }
 
     @Override
-    public void onDismiss(DialogInterface dialog) {
-        super.onDismiss(dialog);
-        if (mListener != null && !mListenerCalled) {
-            mListener.onCancel();
-        }
+    public void onCancel(DialogInterface dialog) {
+        super.onCancel(dialog);
+        mListener.onCancel();
     }
 }
 

chrome/android/javatests/src/org/chromium/chrome/browser/signin/ConfirmSyncDataIntegrationTest.java

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chrome.browser.signin;
+
+import static android.support.test.espresso.Espresso.onView;
+import static android.support.test.espresso.action.ViewActions.click;
+import static android.support.test.espresso.matcher.RootMatchers.isDialog;
+import static android.support.test.espresso.matcher.ViewMatchers.withId;
+import static android.support.test.espresso.matcher.ViewMatchers.withText;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+import static org.mockito.MockitoAnnotations.initMocks;
+
+import androidx.test.filters.MediumTest;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+
+import org.chromium.base.Callback;
+import org.chromium.base.test.util.CommandLineFlags;
+import org.chromium.base.test.util.JniMocker;
+import org.chromium.chrome.R;
+import org.chromium.chrome.browser.flags.ChromeSwitches;
+import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
+import org.chromium.content_public.browser.test.util.TestThreadUtils;
+import org.chromium.ui.test.util.DummyUiActivityTestCase;
+
+/**
+ * This class regroups the integration tests for {@link ConfirmSyncDataStateMachine}.
+ *
+ * In this class we use a real {@link ConfirmSyncDataStateMachineDelegate} to walk through
+ * different states of the state machine by clicking on the dialogs shown with the delegate.
+ * This way we tested the invocation of delegate inside state machine and vice versa.
+ *
+ * In contrast, {@link ConfirmSyncDataStateMachineTest} takes a delegate mock to check the
+ * interaction between the state machine and its delegate in one level.
+ */
+@RunWith(ChromeJUnit4ClassRunner.class)
+@CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE})
+public class ConfirmSyncDataIntegrationTest extends DummyUiActivityTestCase {
+    @Rule
+    public final JniMocker mocker = new JniMocker();
+
+    @Mock
+    private SigninManager.Natives mSigninManagerNativeMock;
+
+    @Mock
+    private SigninManager mSigninManagerMock;
+
+    @Mock
+    private IdentityServicesProvider mIdentityServicesProviderMock;
+
+    @Mock
+    private ConfirmSyncDataStateMachine.Listener mListenerMock;
+
+    private ConfirmSyncDataStateMachineDelegate mDelegate;
+
+    @Before
+    public void setUp() {
+        initMocks(this);
+        mocker.mock(SigninManagerJni.TEST_HOOKS, mSigninManagerNativeMock);
+        IdentityServicesProvider.setInstanceForTests(mIdentityServicesProviderMock);
+        when(IdentityServicesProvider.get().getSigninManager()).thenReturn(mSigninManagerMock);
+        mDelegate =
+                new ConfirmSyncDataStateMachineDelegate(getActivity().getSupportFragmentManager());
+    }
+
+    @Test
+    @MediumTest
+    public void testNonManagedAccountPositiveButtonInConfirmImportSyncDataDialog() {
+        mockSigninManagerIsAccountManaged(false);
+        TestThreadUtils.runOnUiThreadBlocking(() -> {
+            ConfirmSyncDataStateMachine stateMachine = new ConfirmSyncDataStateMachine(
+                    mDelegate, "test.account1@gmail.com", "test.account2@gmail.com", mListenerMock);
+        });
+        onView(withId(R.id.sync_keep_separate_choice)).inRoot(isDialog()).perform(click());
+        onView(withText(R.string.continue_button)).perform(click());
+        verify(mListenerMock).onConfirm(true);
+    }
+
+    @Test
+    @MediumTest
+    public void testManagedAccountPositiveButtonInConfirmImportSyncDataDialog() {
+        mockSigninManagerIsAccountManaged(true);
+        String managedNewAccountName = "test.account@manageddomain.com";
+        String domain = "manageddomain.com";
+        when(mSigninManagerNativeMock.extractDomainName(managedNewAccountName)).thenReturn(domain);
+        TestThreadUtils.runOnUiThreadBlocking(() -> {
+            ConfirmSyncDataStateMachine stateMachine = new ConfirmSyncDataStateMachine(
+                    mDelegate, "test.account1@gmail.com", managedNewAccountName, mListenerMock);
+        });
+        onView(withId(R.id.sync_confirm_import_choice)).inRoot(isDialog()).perform(click());
+        onView(withText(R.string.continue_button)).perform(click());
+        onView(withText(R.string.policy_dialog_proceed)).inRoot(isDialog()).perform(click());
+        verify(mListenerMock).onConfirm(false);
+    }
+
+    private void mockSigninManagerIsAccountManaged(boolean isAccountManaged) {
+        doAnswer(invocation -> {
+            Callback<Boolean> callback = invocation.getArgument(1);
+            callback.onResult(isAccountManaged);
+            return null;
+        })
+                .when(mSigninManagerMock)
+                .isAccountManaged(anyString(), any());
+    }
+}

chrome/android/junit/src/org/chromium/chrome/browser/signin/ConfirmImportSyncDataDialogTest.java

@@ -74,12 +74,12 @@ public class ConfirmImportSyncDataDialogTest {
     }
 
     @Test
-    public void testListenerCancelledWhenDialogDismissed() {
+    public void testListenerOnCancelNotCalledWhenDialogDismissed() {
         getConfirmImportSyncDataDialog();
         Assert.assertEquals(1, mFragmentManager.getFragments().size());
         mStateMachineDelegate.dismissAllDialogs();
         Assert.assertEquals(0, mFragmentManager.getFragments().size());
-        verify(mMockListener).onCancel();
+        verify(mMockListener, never()).onCancel();
     }
 
     @Test

chrome/android/junit/src/org/chromium/chrome/browser/signin/ConfirmManagedSyncDataDialogTest.java

@@ -64,13 +64,13 @@ public class ConfirmManagedSyncDataDialogTest {
     }
 
     @Test
-    public void testListenerCancelledWhenDialogDismissed() {
+    public void testListenerOnCancelNotCalledWhenDialogDismissed() {
         getSignInToManagedAccountDialog();
         FragmentManager fragmentManager = mActivity.getSupportFragmentManager();
         Assert.assertEquals(1, fragmentManager.getFragments().size());
         mStateMachineDelegate.dismissAllDialogs();
         Assert.assertEquals(0, fragmentManager.getFragments().size());
-        verify(mMockListener).onCancel();
+        verify(mMockListener, never()).onCancel();
     }
 
     @Test