From d9015233f50724294bb408f1c56715a581dc4bed Mon Sep 17 00:00:00 2001 From: Sandeep Siddhartha Date: Thu, 9 Oct 2014 16:41:25 -0700 Subject: [PATCH] Set up a sync preference and policy for syncing [2] - Adds a preference for enabling sync, which controls the sync behavior - Make the ProductionFlags depend on appropriate flags to guarantee that we don't mess things when flipping some flags - Preferences now control the "syncable" property of the provider thereby controlling the policy and when this entry shows up in system settings. Bug: 17464069 Change-Id: I1d58351188518c1ae9f1f9e147b5ea15d32a3427 --- .../latin/define/ProductionFlags.java | 2 +- .../latin/sync/BeanstalkManager.java | 55 ------ java/res/values/strings.xml | 8 +- java/res/xml/prefs_screen_accounts.xml | 13 +- .../android/inputmethod/latin/LatinIME.java | 3 - .../settings/AccountsSettingsFragment.java | 176 +++++++++++------- .../settings/LocalSettingsConstants.java | 6 +- .../AccountsSettingsFragmentTests.java | 67 +++++++ 8 files changed, 199 insertions(+), 131 deletions(-) delete mode 100644 java-overridable/src/com/android/inputmethod/latin/sync/BeanstalkManager.java diff --git a/java-overridable/src/com/android/inputmethod/latin/define/ProductionFlags.java b/java-overridable/src/com/android/inputmethod/latin/define/ProductionFlags.java index 10fc612e7..99b958952 100644 --- a/java-overridable/src/com/android/inputmethod/latin/define/ProductionFlags.java +++ b/java-overridable/src/com/android/inputmethod/latin/define/ProductionFlags.java @@ -46,5 +46,5 @@ public final class ProductionFlags { /** * When {@code true}, personal dictionary sync feature is ready to be enabled. */ - public static final boolean ENABLE_PERSONAL_DICTIONARY_SYNC = false; + public static final boolean ENABLE_PERSONAL_DICTIONARY_SYNC = ENABLE_ACCOUNT_SIGN_IN && false; } diff --git a/java-overridable/src/com/android/inputmethod/latin/sync/BeanstalkManager.java b/java-overridable/src/com/android/inputmethod/latin/sync/BeanstalkManager.java deleted file mode 100644 index 2242d9244..000000000 --- a/java-overridable/src/com/android/inputmethod/latin/sync/BeanstalkManager.java +++ /dev/null @@ -1,55 +0,0 @@ -/* - * Copyright (C) 2014 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.android.inputmethod.latin.sync; - -import android.content.Context; - -import javax.annotation.Nonnull; -import javax.annotation.concurrent.GuardedBy; - -public class BeanstalkManager { - private static final Object sLock = new Object(); - - @GuardedBy("sLock") - private static BeanstalkManager sInstance; - - /** - * @return the singleton instance of {@link BeanstalkManager}. - */ - @Nonnull - public static BeanstalkManager getInstance(Context context) { - synchronized(sLock) { - if (sInstance == null) { - sInstance = new BeanstalkManager(context.getApplicationContext()); - } - } - return sInstance; - } - - private BeanstalkManager(final Context context) { - // Intentional private constructor for singleton. - } - - public void onCreate() { - } - - public void requestSync() { - } - - public void onDestroy() { - } -} \ No newline at end of file diff --git a/java/res/values/strings.xml b/java/res/values/strings.xml index 6aea637c5..ed05e916f 100644 --- a/java/res/values/strings.xml +++ b/java/res/values/strings.xml @@ -56,9 +56,11 @@ Enable split keyboard - Sync Now - Sync your personal dictionary - Select an account to enable sync + + Enable sync + Sync your personal dictionary across devices + Select an account to enable sync + [DEBUG] Sync Now Switch to other input methods diff --git a/java/res/xml/prefs_screen_accounts.xml b/java/res/xml/prefs_screen_accounts.xml index 003a37ff7..41642bf08 100644 --- a/java/res/xml/prefs_screen_accounts.xml +++ b/java/res/xml/prefs_screen_accounts.xml @@ -28,7 +28,15 @@ android:title="@string/switch_accounts" android:summary="@string/no_accounts_selected" /> - + + + + + android:title="@string/sync_now_title" + android:dependency="pref_enable_cloud_sync" /> diff --git a/java/src/com/android/inputmethod/latin/LatinIME.java b/java/src/com/android/inputmethod/latin/LatinIME.java index 743b570ac..3b995f9d9 100644 --- a/java/src/com/android/inputmethod/latin/LatinIME.java +++ b/java/src/com/android/inputmethod/latin/LatinIME.java @@ -87,7 +87,6 @@ import com.android.inputmethod.latin.settings.SettingsActivity; import com.android.inputmethod.latin.settings.SettingsValues; import com.android.inputmethod.latin.suggestions.SuggestionStripView; import com.android.inputmethod.latin.suggestions.SuggestionStripViewAccessor; -import com.android.inputmethod.latin.sync.BeanstalkManager; import com.android.inputmethod.latin.touchinputconsumer.GestureConsumer; import com.android.inputmethod.latin.utils.ApplicationUtils; import com.android.inputmethod.latin.utils.CapsModeUtils; @@ -561,7 +560,6 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen AudioAndHapticFeedbackManager.init(this); AccessibilityUtils.init(this); mStatsUtilsManager.onCreate(this /* context */); - BeanstalkManager.getInstance(this /* context */).onCreate(); super.onCreate(); mHandler.onCreate(); @@ -709,7 +707,6 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen unregisterReceiver(mDictionaryPackInstallReceiver); unregisterReceiver(mDictionaryDumpBroadcastReceiver); mStatsUtilsManager.onDestroy(); - BeanstalkManager.getInstance(this /* context */).onDestroy(); super.onDestroy(); } diff --git a/java/src/com/android/inputmethod/latin/settings/AccountsSettingsFragment.java b/java/src/com/android/inputmethod/latin/settings/AccountsSettingsFragment.java index fa716457c..c382426c0 100644 --- a/java/src/com/android/inputmethod/latin/settings/AccountsSettingsFragment.java +++ b/java/src/com/android/inputmethod/latin/settings/AccountsSettingsFragment.java @@ -16,7 +16,12 @@ package com.android.inputmethod.latin.settings; +import static com.android.inputmethod.latin.settings.LocalSettingsConstants.PREF_ACCOUNT_NAME; +import static com.android.inputmethod.latin.settings.LocalSettingsConstants.PREF_ENABLE_CLOUD_SYNC; + +import android.accounts.Account; import android.app.AlertDialog; +import android.content.ContentResolver; import android.content.Context; import android.content.DialogInterface; import android.content.SharedPreferences; @@ -33,7 +38,6 @@ import com.android.inputmethod.latin.R; import com.android.inputmethod.latin.SubtypeSwitcher; import com.android.inputmethod.latin.accounts.LoginAccountUtils; import com.android.inputmethod.latin.define.ProductionFlags; -import com.android.inputmethod.latin.sync.BeanstalkManager; import javax.annotation.Nullable; @@ -41,19 +45,18 @@ import javax.annotation.Nullable; * "Accounts & Privacy" settings sub screen. * * This settings sub screen handles the following preferences: - *
  • Account selection/management for IME - *
  • TODO: Sync preferences - *
  • TODO: Privacy preferences - *
  • Sync now + *
  • Account selection/management for IME
    • + *
  • Sync preferences
    • + *
  • Privacy preferences
    • */ public final class AccountsSettingsFragment extends SubScreenFragment { - static final String PREF_ACCCOUNT_SWITCHER = "account_switcher"; - static final String PREF_SYNC_NOW = "pref_beanstalk"; + private static final String PREF_SYNC_NOW = "pref_beanstalk"; - private final DialogInterface.OnClickListener mAccountSelectedListener = - new AccountSelectedListener(); - private final DialogInterface.OnClickListener mAccountSignedOutListener = - new AccountSignedOutListener(); + @UsedForTesting static final String AUTHORITY = "com.android.inputmethod.latin.provider"; + static final String PREF_ACCCOUNT_SWITCHER = "account_switcher"; + + private final DialogInterface.OnClickListener mAccountChangedListener = + new AccountChangedListener(); private final Preference.OnPreferenceClickListener mSyncNowListener = new SyncNowListener(); @Override @@ -81,47 +84,55 @@ public final class AccountsSettingsFragment extends SubScreenFragment { removePreference(Settings.PREF_ENABLE_METRICS_LOGGING); } + if (!ProductionFlags.ENABLE_ACCOUNT_SIGN_IN) { + removePreference(PREF_ACCCOUNT_SWITCHER); + removePreference(PREF_ENABLE_CLOUD_SYNC); + removePreference(PREF_SYNC_NOW); + } if (!ProductionFlags.ENABLE_PERSONAL_DICTIONARY_SYNC) { + removePreference(PREF_ENABLE_CLOUD_SYNC); removePreference(PREF_SYNC_NOW); } else { final Preference syncNowPreference = findPreference(PREF_SYNC_NOW); - if (syncNowPreference != null) { - syncNowPreference.setOnPreferenceClickListener(mSyncNowListener); - } + syncNowPreference.setOnPreferenceClickListener(mSyncNowListener); } } @Override public void onResume() { super.onResume(); - refreshUi(); + refreshAccountAndDependentPreferences(getCurrentlySelectedAccount()); } @Override public void onSharedPreferenceChanged(final SharedPreferences prefs, final String key) { - // TODO: Look at the preference that changed before refreshing the view. - refreshUi(); + if (TextUtils.equals(key, PREF_ACCOUNT_NAME)) { + refreshAccountAndDependentPreferences( + prefs.getString(PREF_ACCOUNT_NAME, null)); + } else if (TextUtils.equals(key, PREF_ENABLE_CLOUD_SYNC)) { + final boolean syncEnabled = prefs.getBoolean(PREF_ENABLE_CLOUD_SYNC, false); + updateSyncPolicy(syncEnabled, LoginAccountUtils.getCurrentAccount(getActivity())); + } } - private void refreshUi() { - refreshAccountSelection(); - refreshSyncNow(); - } - - private void refreshAccountSelection() { + private void refreshAccountAndDependentPreferences(@Nullable final String currentAccount) { if (!ProductionFlags.ENABLE_ACCOUNT_SIGN_IN) { return; } - final String currentAccount = getCurrentlySelectedAccount(); final Preference accountSwitcher = findPreference(PREF_ACCCOUNT_SWITCHER); if (currentAccount == null) { // No account is currently selected. accountSwitcher.setSummary(getString(R.string.no_accounts_selected)); + // Disable the sync preference UI. + disableSyncPreference(); } else { // Set the currently selected account. accountSwitcher.setSummary(getString(R.string.account_selected, currentAccount)); + // Enable the sync preference UI. + enableSyncPreference(); } + // Set up onClick listener for the account picker preference. final Context context = getActivity(); final String[] accountsForLogin = LoginAccountUtils.getAccountsForLogin(context); accountSwitcher.setOnPreferenceClickListener(new OnPreferenceClickListener() { @@ -129,39 +140,64 @@ public final class AccountsSettingsFragment extends SubScreenFragment { public boolean onPreferenceClick(Preference preference) { if (accountsForLogin.length == 0) { // TODO: Handle account addition. - Toast.makeText(getActivity(), - getString(R.string.account_select_cancel), Toast.LENGTH_SHORT).show(); + Toast.makeText(getActivity(), getString(R.string.account_select_cancel), + Toast.LENGTH_SHORT).show(); } else { createAccountPicker(accountsForLogin, currentAccount).show(); } return true; } }); - - // TODO: Depending on the account selection, enable/disable preferences that - // depend on an account. } /** - * Refreshes the "Sync Now" feature + * Enables the Sync preference UI and updates its summary. */ - private void refreshSyncNow() { + private void enableSyncPreference() { if (!ProductionFlags.ENABLE_PERSONAL_DICTIONARY_SYNC) { return; } - final Preference syncNowPreference = findPreference(PREF_SYNC_NOW); - if (syncNowPreference == null) { + final Preference syncPreference = findPreference(PREF_ENABLE_CLOUD_SYNC); + syncPreference.setEnabled(true); + syncPreference.setSummary(R.string.cloud_sync_summary); + } + + /** + * Disables the Sync preference UI and updates its summary to indicate + * the fact that an account needs to be selected for sync. + */ + private void disableSyncPreference() { + if (!ProductionFlags.ENABLE_PERSONAL_DICTIONARY_SYNC) { return; } - final String currentAccount = getCurrentlySelectedAccount(); - if (currentAccount == null) { - syncNowPreference.setEnabled(false); - syncNowPreference.setSummary(R.string.sync_now_summary_disabled_signed_out); + final Preference syncPreference = findPreference(PREF_ENABLE_CLOUD_SYNC); + syncPreference.setEnabled(false); + syncPreference.setSummary(R.string.cloud_sync_summary_disabled_signed_out); + } + + /** + * Given a non-null accountToUse, this method looks at the enabled value to either + * set or unset the syncable property of the sync authority. + * If the account is null, this method is a no-op currently, but we may want + * to perform some cleanup in the future. + */ + @UsedForTesting + void updateSyncPolicy(boolean enabled, Account accountToUse) { + if (!ProductionFlags.ENABLE_PERSONAL_DICTIONARY_SYNC) { + return; + } + + if (accountToUse != null) { + final int syncable = enabled ? 1 : 0; + ContentResolver.setIsSyncable(accountToUse, AUTHORITY, syncable); + // TODO: Also add a periodic sync here. + // See ContentResolver.addPeriodicSync } else { - syncNowPreference.setEnabled(true); - syncNowPreference.setSummary(R.string.sync_now_summary); + // Without an account, we cannot really set the sync to off. + // Hopefully the account sign-out listener would have taken care of that for us. + // But cases such as clear data are still not handled cleanly. } } @@ -170,6 +206,10 @@ public final class AccountsSettingsFragment extends SubScreenFragment { return getSharedPreferences().getString(LocalSettingsConstants.PREF_ACCOUNT_NAME, null); } + private boolean isSyncEnabled() { + return getSharedPreferences().getBoolean(PREF_ENABLE_CLOUD_SYNC, false); + } + /** * Creates an account picker dialog showing the given accounts in a list and selecting * the selected account by default. @@ -200,51 +240,55 @@ public final class AccountsSettingsFragment extends SubScreenFragment { final AlertDialog.Builder builder = new AlertDialog.Builder(getActivity()) .setTitle(R.string.account_select_title) .setSingleChoiceItems(accounts, index, null) - .setPositiveButton(R.string.account_select_ok, mAccountSelectedListener) + .setPositiveButton(R.string.account_select_ok, mAccountChangedListener) .setNegativeButton(R.string.account_select_cancel, null); if (isSignedIn) { - builder.setNeutralButton(R.string.account_select_sign_out, mAccountSignedOutListener); + builder.setNeutralButton(R.string.account_select_sign_out, mAccountChangedListener); } return builder.create(); } /** - * Listener for an account being selected from the picker. - * Persists the account to shared preferences. + * Listener for a account selection changes from the picker. + * Persists/removes the account to/from shared preferences and sets up sync if required. */ - class AccountSelectedListener implements DialogInterface.OnClickListener { + class AccountChangedListener implements DialogInterface.OnClickListener { @Override public void onClick(DialogInterface dialog, int which) { - final ListView lv = ((AlertDialog)dialog).getListView(); - final Object selectedItem = lv.getItemAtPosition(lv.getCheckedItemPosition()); - getSharedPreferences() - .edit() - .putString(LocalSettingsConstants.PREF_ACCOUNT_NAME, (String) selectedItem) - .apply(); + switch (which) { + case DialogInterface.BUTTON_POSITIVE: // Signed in + final ListView lv = ((AlertDialog)dialog).getListView(); + final Object selectedItem = lv.getItemAtPosition(lv.getCheckedItemPosition()); + getSharedPreferences() + .edit() + .putString(PREF_ACCOUNT_NAME, (String) selectedItem) + .apply(); + // Attempt starting sync for the new account if sync was + // previously enabled. + // If not, stop it. + updateSyncPolicy(isSyncEnabled(), + LoginAccountUtils.getCurrentAccount(getActivity())); + break; + case DialogInterface.BUTTON_NEUTRAL: // Signed out + // Stop sync for the account that's being signed out of. + updateSyncPolicy(false, LoginAccountUtils.getCurrentAccount(getActivity())); + getSharedPreferences() + .edit() + .remove(PREF_ACCOUNT_NAME) + .apply(); + break; + } } } /** - * Listener for sign-out being initiated from from the picker. - * Removed the account from shared preferences. - */ - class AccountSignedOutListener implements DialogInterface.OnClickListener { - @Override - public void onClick(DialogInterface dialog, int which) { - getSharedPreferences() - .edit() - .remove(LocalSettingsConstants.PREF_ACCOUNT_NAME) - .apply(); - } - } - - /** - * Listener that initates the process of sync in the background. + * Listener that initiates the process of sync in the background. */ class SyncNowListener implements Preference.OnPreferenceClickListener { @Override public boolean onPreferenceClick(final Preference preference) { - BeanstalkManager.getInstance(getActivity() /* context */).requestSync(); + ContentResolver.requestSync( + LoginAccountUtils.getCurrentAccount(getActivity()), AUTHORITY, Bundle.EMPTY); return true; } } diff --git a/java/src/com/android/inputmethod/latin/settings/LocalSettingsConstants.java b/java/src/com/android/inputmethod/latin/settings/LocalSettingsConstants.java index 71d6065e0..f4f2e03c7 100644 --- a/java/src/com/android/inputmethod/latin/settings/LocalSettingsConstants.java +++ b/java/src/com/android/inputmethod/latin/settings/LocalSettingsConstants.java @@ -27,6 +27,9 @@ public class LocalSettingsConstants { // Preference key for the current account. // Do not restore. public static final String PREF_ACCOUNT_NAME = "pref_account_name"; + // Preference key for enabling cloud sync feature. + // Do not restore. + public static final String PREF_ENABLE_CLOUD_SYNC = "pref_enable_cloud_sync"; // List of preference keys to skip from being restored by backup agent. // These preferences are tied to a device and hence should not be restored. @@ -36,6 +39,7 @@ public class LocalSettingsConstants { // shared preferences which makes it non-trivial to move these out to // a different shared preferences file. public static final String[] PREFS_TO_SKIP_RESTORING = new String[] { - PREF_ACCOUNT_NAME + PREF_ACCOUNT_NAME, + PREF_ENABLE_CLOUD_SYNC }; } diff --git a/tests/src/com/android/inputmethod/latin/settings/AccountsSettingsFragmentTests.java b/tests/src/com/android/inputmethod/latin/settings/AccountsSettingsFragmentTests.java index 2ef8b548f..a0b2c2296 100644 --- a/tests/src/com/android/inputmethod/latin/settings/AccountsSettingsFragmentTests.java +++ b/tests/src/com/android/inputmethod/latin/settings/AccountsSettingsFragmentTests.java @@ -16,14 +16,20 @@ package com.android.inputmethod.latin.settings; +import static com.android.inputmethod.latin.settings.AccountsSettingsFragment.AUTHORITY; + +import android.accounts.Account; import android.app.AlertDialog; import android.app.Dialog; +import android.content.ContentResolver; import android.content.Intent; import android.test.ActivityInstrumentationTestCase2; import android.test.suitebuilder.annotation.MediumTest; import android.view.View; import android.widget.ListView; +import com.android.inputmethod.latin.define.ProductionFlags; + import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -32,6 +38,7 @@ public class AccountsSettingsFragmentTests extends ActivityInstrumentationTestCase2 { private static final String FRAG_NAME = AccountsSettingsFragment.class.getName(); private static final long TEST_TIMEOUT_MILLIS = 5000; + private static final Account TEST_ACCOUNT = new Account("account-for-test", "account-type"); private AlertDialog mDialog; @@ -47,6 +54,13 @@ public class AccountsSettingsFragmentTests setActivityIntent(intent); } + @Override + protected void tearDown() throws Exception { + super.tearDown(); + // reset the syncable state to unknown + ContentResolver.setIsSyncable(TEST_ACCOUNT, AUTHORITY, -1); + } + public void testEmptyAccounts() { final AccountsSettingsFragment fragment = (AccountsSettingsFragment) getActivity().mFragment; @@ -129,4 +143,57 @@ public class AccountsSettingsFragmentTests assertEquals(View.VISIBLE, mDialog.getButton(Dialog.BUTTON_NEGATIVE).getVisibility()); assertEquals(View.VISIBLE, mDialog.getButton(Dialog.BUTTON_POSITIVE).getVisibility()); } + + public void testUpdateSyncPolicy_enable() { + // This test is a no-op when ENABLE_PERSONAL_DICTIONARY_SYNC is not enabled + if (!ProductionFlags.ENABLE_PERSONAL_DICTIONARY_SYNC) { + return; + } + // Should be unknown by default. + assertTrue(ContentResolver.getIsSyncable(TEST_ACCOUNT, AUTHORITY) < 0); + + final AccountsSettingsFragment fragment = + (AccountsSettingsFragment) getActivity().mFragment; + fragment.updateSyncPolicy(true, TEST_ACCOUNT); + + // Should be syncable now. + assertEquals(1, ContentResolver.getIsSyncable(TEST_ACCOUNT, AUTHORITY)); + } + + public void testUpdateSyncPolicy_disable() { + // This test is a no-op when ENABLE_PERSONAL_DICTIONARY_SYNC is not enabled + if (!ProductionFlags.ENABLE_PERSONAL_DICTIONARY_SYNC) { + return; + } + // Should be unknown by default. + assertTrue(ContentResolver.getIsSyncable(TEST_ACCOUNT, AUTHORITY) < 0); + + final AccountsSettingsFragment fragment = + (AccountsSettingsFragment) getActivity().mFragment; + fragment.updateSyncPolicy(false, TEST_ACCOUNT); + + // Should not be syncable now. + assertEquals(0, ContentResolver.getIsSyncable(TEST_ACCOUNT, AUTHORITY)); + } + + public void testUpdateSyncPolicy_enableDisable() { + // This test is a no-op when ENABLE_PERSONAL_DICTIONARY_SYNC is not enabled + if (!ProductionFlags.ENABLE_PERSONAL_DICTIONARY_SYNC) { + return; + } + // Should be unknown by default. + assertTrue(ContentResolver.getIsSyncable(TEST_ACCOUNT, AUTHORITY) < 0); + + final AccountsSettingsFragment fragment = + (AccountsSettingsFragment) getActivity().mFragment; + fragment.updateSyncPolicy(true, TEST_ACCOUNT); + + // Should be syncable now. + assertEquals(1, ContentResolver.getIsSyncable(TEST_ACCOUNT, AUTHORITY)); + + fragment.updateSyncPolicy(false, TEST_ACCOUNT); + + // Should not be syncable now. + assertEquals(0, ContentResolver.getIsSyncable(TEST_ACCOUNT, AUTHORITY)); + } }