Commit 137bcf85 authored by Tanmoy Mollik's avatar Tanmoy Mollik Committed by Commit Bot

[Signin][Andriod] SetFirstSetupComplete in OnDestroy and implement tests for...

[Signin][Andriod] SetFirstSetupComplete in OnDestroy and implement tests for https://crrev.com/c/1744487

There was some new logic introduced to start sync in andoird in this cl
https://crrev.com/c/1744487. This cl is for testing the logic. Also the
call to setFirstSetupComplete in SyncAndServicesPreferences has been moved
from onBackPressed to onDestroy as that provides more consistency when the
fragment is destroyed.

Bug: 1009965, 951330
Change-Id: I8f095490c81e145e4722e5f592922d735d5b9a9e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1855986Reviewed-by: default avatarMarc Treib <treib@chromium.org>
Reviewed-by: default avatarBoris Sazonov <bsazonov@chromium.org>
Commit-Queue: Tanmoy Mollik <triploblastic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710301}
parent 2e024871
......@@ -86,8 +86,10 @@ public class SyncAndServicesPreferences extends PreferenceFragmentCompat
private static final String PREF_SIGNIN = "sign_in";
private static final String PREF_MANAGE_YOUR_GOOGLE_ACCOUNT = "manage_your_google_account";
private static final String PREF_SYNC_CATEGORY = "sync_category";
private static final String PREF_SYNC_ERROR_CARD = "sync_error_card";
@VisibleForTesting
public static final String PREF_SYNC_CATEGORY = "sync_category";
@VisibleForTesting
public static final String PREF_SYNC_ERROR_CARD = "sync_error_card";
private static final String PREF_SYNC_DISABLED_BY_ADMINISTRATOR =
"sync_disabled_by_administrator";
@VisibleForTesting
......@@ -243,6 +245,16 @@ public class SyncAndServicesPreferences extends PreferenceFragmentCompat
@Override
public void onDestroy() {
super.onDestroy();
if (ChromeFeatureList.isEnabled(ChromeFeatureList.SYNC_MANUAL_START_ANDROID)
&& wasSigninFlowInterrupted()) {
// If the setup flow was previously interrupted, and now the user dismissed the page
// without turning sync on, then mark first setup as complete (so that we won't show the
// error again), but turn sync off.
assert !mSyncRequested.isChecked();
SyncPreferenceUtils.enableSync(false);
mProfileSyncService.setFirstSetupComplete(
SyncFirstSetupCompleteSource.ADVANCED_FLOW_INTERRUPTED_LEAVE_SYNC_OFF);
}
mSyncSetupInProgressHandle.close();
}
......@@ -629,16 +641,6 @@ public class SyncAndServicesPreferences extends PreferenceFragmentCompat
@Override
public boolean onBackPressed() {
if (ChromeFeatureList.isEnabled(ChromeFeatureList.SYNC_MANUAL_START_ANDROID)
&& wasSigninFlowInterrupted()) {
// If the setup flow was previously interrupted, and now the user dismissed the page
// without turning sync on, then mark first setup as complete (so that we won't show the
// error again), but turn sync off.
assert !mSyncRequested.isChecked();
SyncPreferenceUtils.enableSync(false);
mProfileSyncService.setFirstSetupComplete(
SyncFirstSetupCompleteSource.ADVANCED_FLOW_INTERRUPTED_LEAVE_SYNC_OFF);
}
if (!mIsFromSigninScreen) return false; // Let parent activity handle it.
showCancelSyncDialog();
return true;
......
......@@ -136,6 +136,17 @@ public class ProfileSyncService {
mNativeProfileSyncServiceAndroid, ProfileSyncService.this);
}
/**
* Checks whether sync machinery is active.
*
* @return true if the transport state is active.
*/
@VisibleForTesting
public boolean isTransportStateActive() {
return ProfileSyncServiceJni.get().isTransportStateActive(
mNativeProfileSyncServiceAndroid, ProfileSyncService.this);
}
/**
* Checks whether Sync-the-feature can (attempt to) start. This means that there is a primary
* account and no disable reasons. Note that the Sync machinery may start up in transport-only
......@@ -644,6 +655,8 @@ public class ProfileSyncService {
long nativeProfileSyncServiceAndroid, ProfileSyncService caller);
boolean isEncryptEverythingEnabled(
long nativeProfileSyncServiceAndroid, ProfileSyncService caller);
boolean isTransportStateActive(
long nativeProfileSyncServiceAndroid, ProfileSyncService caller);
void enableEncryptEverything(
long nativeProfileSyncServiceAndroid, ProfileSyncService caller);
boolean isPassphraseRequiredForPreferredDataTypes(
......
......@@ -4,9 +4,16 @@
package org.chromium.chrome.browser.sync;
import android.app.Activity;
import android.content.Context;
import android.content.Intent;
import android.os.Bundle;
import android.support.test.InstrumentationRegistry;
import android.support.test.filters.SmallTest;
import android.support.test.uiautomator.UiDevice;
import android.support.v4.app.FragmentTransaction;
import android.support.v7.preference.Preference;
import android.support.v7.preference.PreferenceCategory;
import org.junit.After;
import org.junit.Assert;
......@@ -14,14 +21,19 @@ import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.chromium.base.ActivityState;
import org.chromium.base.ApplicationStatus;
import org.chromium.base.test.util.CallbackHelper;
import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.base.test.util.DisabledTest;
import org.chromium.base.test.util.Feature;
import org.chromium.chrome.browser.ChromeSwitches;
import org.chromium.chrome.browser.preferences.ChromeSwitchPreference;
import org.chromium.chrome.browser.preferences.Preferences;
import org.chromium.chrome.browser.preferences.PreferencesLauncher;
import org.chromium.chrome.browser.preferences.sync.SyncAndServicesPreferences;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.util.ApplicationTestUtils;
import org.chromium.chrome.test.util.browser.sync.SyncTestUtil;
import org.chromium.components.sync.AndroidSyncSettings;
import org.chromium.content_public.browser.test.util.TestThreadUtils;
......@@ -147,6 +159,106 @@ public class SyncAndServicesPreferencesTest {
mSyncTestRule.hasServerAutofillCreditCards());
}
@Test
@SmallTest
@Feature({"Sync"})
public void testDismissedSettingsDoesNotSetFirstSetupComplete() throws Exception {
mSyncTestRule.setUpTestAccountAndSignInWithSyncSetupAsIncomplete();
startPreferencesForAdvancedSyncFlowAndInterruptIt();
// FirstSetupComplete should not be set.
TestThreadUtils.runOnUiThreadBlocking(
() -> { Assert.assertFalse(ProfileSyncService.get().isFirstSetupComplete()); });
}
@Test
@SmallTest
@Feature({"Sync"})
public void testDismissedSettingsShowsSyncSwitchOffByDefault() throws Exception {
mSyncTestRule.setUpTestAccountAndSignInWithSyncSetupAsIncomplete();
startPreferencesForAdvancedSyncFlowAndInterruptIt();
SyncAndServicesPreferences fragment = startSyncAndServicesPreferences();
assertSyncOffState(fragment);
}
@Test
@SmallTest
@Feature({"Sync"})
public void testDismissedSettingsShowsSyncErrorCard() throws Exception {
mSyncTestRule.setUpTestAccountAndSignInWithSyncSetupAsIncomplete();
startPreferencesForAdvancedSyncFlowAndInterruptIt();
SyncAndServicesPreferences fragment = startSyncAndServicesPreferences();
Assert.assertNotNull("Sync error card should be shown", getSyncErrorCard(fragment));
}
@Test
@SmallTest
@Feature({"Sync"})
public void testFirstSetupCompleteIsSetAfterSettingsOpenedAndBackPressed() throws Exception {
mSyncTestRule.setUpTestAccountAndSignInWithSyncSetupAsIncomplete();
startPreferencesForAdvancedSyncFlowAndInterruptIt();
// Open Settings and leave sync off.
SyncAndServicesPreferences fragment = startSyncAndServicesPreferences();
pressBackAndDismissActivity(fragment.getActivity());
// FirstSetupComplete should be set.
TestThreadUtils.runOnUiThreadBlocking(
() -> { Assert.assertTrue(ProfileSyncService.get().isFirstSetupComplete()); });
fragment = startSyncAndServicesPreferences();
Assert.assertNull("Sync error card should not be shown", getSyncErrorCard(fragment));
assertSyncOffState(fragment);
Assert.assertFalse(AndroidSyncSettings.get().isChromeSyncEnabled());
}
@Test
@SmallTest
@Feature({"Sync"})
public void testFirstSetupCompleteIsSetAfterSettingsOpenedAndDismissed() throws Exception {
mSyncTestRule.setUpTestAccountAndSignInWithSyncSetupAsIncomplete();
startPreferencesForAdvancedSyncFlowAndInterruptIt();
// Open Settings and leave sync off.
SyncAndServicesPreferences fragment = startSyncAndServicesPreferences();
ApplicationTestUtils.finishActivity(fragment.getActivity());
// FirstSetupComplete should be set.
TestThreadUtils.runOnUiThreadBlocking(
() -> { Assert.assertTrue(ProfileSyncService.get().isFirstSetupComplete()); });
fragment = startSyncAndServicesPreferences();
Assert.assertNull("Sync error card should not be shown", getSyncErrorCard(fragment));
assertSyncOffState(fragment);
Assert.assertFalse(AndroidSyncSettings.get().isChromeSyncEnabled());
}
@Test
@SmallTest
@Feature({"Sync"})
public void testFirstSetupCompleteIsSetAfterSyncTurnedOn() throws Exception {
mSyncTestRule.setUpTestAccountAndSignInWithSyncSetupAsIncomplete();
startPreferencesForAdvancedSyncFlowAndInterruptIt();
// Open Settings and turn sync on.
SyncAndServicesPreferences fragment = startSyncAndServicesPreferences();
ChromeSwitchPreference syncSwitch = getSyncSwitch(fragment);
mSyncTestRule.togglePreference(syncSwitch);
Assert.assertTrue(syncSwitch.isChecked());
Assert.assertTrue(AndroidSyncSettings.get().isChromeSyncEnabled());
// FirstSetupComplete should be set.
TestThreadUtils.runOnUiThreadBlocking(
() -> { Assert.assertTrue(ProfileSyncService.get().isFirstSetupComplete()); });
Assert.assertNull("Sync error card should not be shown", getSyncErrorCard(fragment));
}
/**
* Start SyncAndServicesPreferences signin screen and dissmiss it without pressing confirm or
* cancel.
*/
private void startPreferencesForAdvancedSyncFlowAndInterruptIt() throws Exception {
Context context = InstrumentationRegistry.getTargetContext();
String fragmentName = SyncAndServicesPreferences.class.getName();
final Bundle arguments = SyncAndServicesPreferences.createArguments(true);
Intent intent =
PreferencesLauncher.createIntentForSettingsPage(context, fragmentName, arguments);
Activity activity = InstrumentationRegistry.getInstrumentation().startActivitySync(intent);
Assert.assertTrue(activity instanceof Preferences);
ApplicationTestUtils.finishActivity(activity);
}
private SyncAndServicesPreferences startSyncAndServicesPreferences() {
mPreferences = mSyncTestRule.startPreferences(SyncAndServicesPreferences.class.getName());
InstrumentationRegistry.getInstrumentation().waitForIdleSync();
......@@ -161,11 +273,50 @@ public class SyncAndServicesPreferencesTest {
InstrumentationRegistry.getInstrumentation().waitForIdleSync();
}
// TODO(https://crbug.com/1015449): Move this function to ApplicationTestUtils.
private void waitForActivityState(Activity activity, @ActivityState int state)
throws Exception {
final CallbackHelper callbackHelper = new CallbackHelper();
final ApplicationStatus.ActivityStateListener activityStateListener =
(activity1, newState) -> {
if (newState == state) {
callbackHelper.notifyCalled();
}
};
try {
boolean correctState = TestThreadUtils.runOnUiThreadBlocking(() -> {
if (ApplicationStatus.getStateForActivity(activity) == state) {
return true;
}
ApplicationStatus.registerStateListenerForActivity(activityStateListener, activity);
activity.finish();
return false;
});
if (!correctState) {
callbackHelper.waitForCallback(0);
}
} finally {
ApplicationStatus.unregisterActivityStateListener(activityStateListener);
}
}
private void pressBackAndDismissActivity(Activity activity) throws Exception {
UiDevice device = UiDevice.getInstance(InstrumentationRegistry.getInstrumentation());
device.pressBack();
waitForActivityState(activity, ActivityState.DESTROYED);
}
private ChromeSwitchPreference getSyncSwitch(SyncAndServicesPreferences fragment) {
return (ChromeSwitchPreference) fragment.findPreference(
SyncAndServicesPreferences.PREF_SYNC_REQUESTED);
}
private Preference getSyncErrorCard(SyncAndServicesPreferences fragment) {
return ((PreferenceCategory) fragment.findPreference(
SyncAndServicesPreferences.PREF_SYNC_CATEGORY))
.findPreference(SyncAndServicesPreferences.PREF_SYNC_ERROR_CARD);
}
private void assertSyncOnState(SyncAndServicesPreferences fragment) {
Assert.assertTrue("The sync switch should be on.", getSyncSwitch(fragment).isChecked());
Assert.assertTrue(
......
......@@ -136,12 +136,27 @@ public class SyncTestRule extends ChromeActivityTestRule<ChromeActivity> {
return account;
}
/**
* Set up a test account, sign in and enable sync. FirstSetupComplete bit will be set after
* this. For most purposes this function should be used as this emulates the basic sign in flow.
* @return the test account that is signed in.
*/
public Account setUpTestAccountAndSignIn() {
Account account = setUpTestAccount();
signinAndEnableSync(account);
return account;
}
/**
* Set up a test account, sign in but don't mark sync setup complete.
* @return the test account that is signed in.
*/
public Account setUpTestAccountAndSignInWithSyncSetupAsIncomplete() {
Account account = setUpTestAccount();
signinAndEnableSyncInternal(account, false);
return account;
}
public void startSync() {
TestThreadUtils.runOnUiThreadBlocking(() -> { mProfileSyncService.requestStart(); });
}
......@@ -157,29 +172,7 @@ public class SyncTestRule extends ChromeActivityTestRule<ChromeActivity> {
}
public void signinAndEnableSync(final Account account) {
TestThreadUtils.runOnUiThreadBlocking(() -> {
IdentityServicesProvider.getSigninManager().signIn(
account, new SigninManager.SignInCallback() {
@Override
public void onSignInComplete() {
if (ChromeFeatureList.isEnabled(
ChromeFeatureList.SYNC_MANUAL_START_ANDROID)) {
mProfileSyncService.setFirstSetupComplete(
SyncFirstSetupCompleteSource.BASIC_FLOW);
}
}
@Override
public void onSignInAborted() {
Assert.fail("Sign-in was aborted");
}
});
// Outside of tests, URL-keyed anonymized data collection is enabled by sign-in UI.
UnifiedConsentServiceBridge.setUrlKeyedAnonymizedDataCollectionEnabled(true);
});
SyncTestUtil.waitForSyncActive();
SyncTestUtil.triggerSyncAndWaitForCompletion();
Assert.assertEquals(account, SigninTestUtil.getCurrentAccount());
signinAndEnableSyncInternal(account, true);
}
public void signOut() throws InterruptedException {
......@@ -341,4 +334,35 @@ public class SyncTestRule extends ChromeActivityTestRule<ChromeActivity> {
});
InstrumentationRegistry.getInstrumentation().waitForIdleSync();
}
private void signinAndEnableSyncInternal(final Account account, boolean setFirstSetupComplete) {
TestThreadUtils.runOnUiThreadBlocking(() -> {
IdentityServicesProvider.getSigninManager().signIn(
account, new SigninManager.SignInCallback() {
@Override
public void onSignInComplete() {
if (ChromeFeatureList.isEnabled(
ChromeFeatureList.SYNC_MANUAL_START_ANDROID)
&& setFirstSetupComplete) {
mProfileSyncService.setFirstSetupComplete(
SyncFirstSetupCompleteSource.BASIC_FLOW);
}
}
@Override
public void onSignInAborted() {
Assert.fail("Sign-in was aborted");
}
});
// Outside of tests, URL-keyed anonymized data collection is enabled by sign-in UI.
UnifiedConsentServiceBridge.setUrlKeyedAnonymizedDataCollectionEnabled(true);
});
if (setFirstSetupComplete) {
SyncTestUtil.waitForSyncActive();
SyncTestUtil.triggerSyncAndWaitForCompletion();
} else {
SyncTestUtil.waitForSyncTransportActive();
}
Assert.assertEquals(account, SigninTestUtil.getCurrentAccount());
}
}
......@@ -177,6 +177,14 @@ jboolean ProfileSyncServiceAndroid::IsEngineInitialized(
return sync_service_->IsEngineInitialized();
}
jboolean ProfileSyncServiceAndroid::IsTransportStateActive(
JNIEnv* env,
const JavaParamRef<jobject>&) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
return sync_service_->GetTransportState() ==
syncer::SyncService::TransportState::ACTIVE;
}
void ProfileSyncServiceAndroid::SetSetupInProgress(
JNIEnv* env,
const JavaParamRef<jobject>& obj,
......
......@@ -54,6 +54,9 @@ class ProfileSyncServiceAndroid : public syncer::SyncServiceObserver {
const base::android::JavaParamRef<jobject>& obj);
jboolean IsEngineInitialized(JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj);
jboolean IsTransportStateActive(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj);
void SetSetupInProgress(JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj,
jboolean in_progress);
......
......@@ -83,6 +83,20 @@ public final class SyncTestUtil {
}, TIMEOUT_MS, INTERVAL_MS);
}
/**
* Waits for sync machinery to become active.
*/
public static void waitForSyncTransportActive() {
CriteriaHelper.pollUiThread(
new Criteria("Timed out waiting for sync transport state to become active.") {
@Override
public boolean isSatisfied() {
return ProfileSyncService.get().isTransportStateActive();
}
},
TIMEOUT_MS, INTERVAL_MS);
}
/**
* Waits for sync's engine to be initialized.
*/
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment