Improve UI pref when sync/cancel account syncs.
The core of the change is in ManageAccountSettings#showSyncState(). New
code caches as much information as it can. And break out of loops as
early as possible.
Bug: 28575620
Test: make RunSettingsRoboTests
Change-Id: I076ce148e3d8db55f6cadfd9491f037f7a55a986
diff --git a/src/com/android/settings/AccountPreference.java b/src/com/android/settings/accounts/AccountPreference.java
similarity index 94%
rename from src/com/android/settings/AccountPreference.java
rename to src/com/android/settings/accounts/AccountPreference.java
index fe39244..7d613b0 100644
--- a/src/com/android/settings/AccountPreference.java
+++ b/src/com/android/settings/accounts/AccountPreference.java
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2008 The Android Open Source Project
+ * Copyright (C) 2016 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.
@@ -14,7 +14,7 @@
* limitations under the License.
*/
-package com.android.settings;
+package com.android.settings.accounts;
import android.accounts.Account;
import android.content.Context;
@@ -24,6 +24,8 @@
import android.util.Log;
import android.widget.ImageView;
+import com.android.settings.R;
+
import java.util.ArrayList;
/**
@@ -78,6 +80,10 @@
}
public void setSyncStatus(int status, boolean updateSummary) {
+ if (mStatus == status) {
+ Log.d(TAG, "Status is the same, not changing anything");
+ return;
+ }
mStatus = status;
if (!mShowTypeIcon && mSyncStatusIcon != null) {
mSyncStatusIcon.setImageResource(getSyncStatusIcon(status));
diff --git a/src/com/android/settings/accounts/AccountPreferenceBase.java b/src/com/android/settings/accounts/AccountPreferenceBase.java
index c6581ac..aa5c518 100644
--- a/src/com/android/settings/accounts/AccountPreferenceBase.java
+++ b/src/com/android/settings/accounts/AccountPreferenceBase.java
@@ -32,6 +32,7 @@
import android.os.UserManager;
import android.support.v7.preference.PreferenceScreen;
import android.text.format.DateFormat;
+import android.text.format.DateUtils;
import android.util.Log;
import com.android.settings.SettingsPreferenceFragment;
@@ -46,6 +47,7 @@
implements AuthenticatorHelper.OnAccountsUpdateListener {
protected static final String TAG = "AccountSettings";
+ protected static final boolean VERBOSE = Log.isLoggable(TAG, Log.VERBOSE);
public static final String AUTHORITIES_FILTER_KEY = "authorities";
public static final String ACCOUNT_TYPES_FILTER_KEY = "account_types";
diff --git a/src/com/android/settings/accounts/ManageAccountsSettings.java b/src/com/android/settings/accounts/ManageAccountsSettings.java
index abfa6a1..ce717e2 100644
--- a/src/com/android/settings/accounts/ManageAccountsSettings.java
+++ b/src/com/android/settings/accounts/ManageAccountsSettings.java
@@ -34,9 +34,11 @@
import android.graphics.drawable.Drawable;
import android.os.Bundle;
import android.os.UserHandle;
+import android.support.annotation.VisibleForTesting;
import android.support.v7.preference.Preference;
import android.support.v7.preference.Preference.OnPreferenceClickListener;
import android.support.v7.preference.PreferenceScreen;
+import android.util.ArraySet;
import android.util.Log;
import android.view.LayoutInflater;
import android.view.Menu;
@@ -47,7 +49,6 @@
import android.widget.TextView;
import com.android.internal.logging.nano.MetricsProto.MetricsEvent;
-import com.android.settings.AccountPreference;
import com.android.settings.R;
import com.android.settings.SettingsActivity;
import com.android.settings.Utils;
@@ -56,8 +57,8 @@
import java.util.ArrayList;
import java.util.Date;
-import java.util.HashSet;
import java.util.List;
+import java.util.Set;
import static android.content.Intent.EXTRA_USER;
@@ -74,7 +75,7 @@
"com.android.settings.accounts.LAUNCHING_LOCATION_SETTINGS";
private static final int MENU_SYNC_NOW_ID = Menu.FIRST;
- private static final int MENU_SYNC_CANCEL_ID = Menu.FIRST + 1;
+ private static final int MENU_SYNC_CANCEL_ID = Menu.FIRST + 1;
private static final int REQUEST_SHOW_SYNC_SETTINGS = 1;
@@ -87,6 +88,8 @@
// mFirstAccount is used for the injected preferences
private Account mFirstAccount;
+ protected Set<String> mUserFacingSyncAuthorities;
+
@Override
public int getMetricsCategory() {
return MetricsEvent.ACCOUNTS_MANAGE_ACCOUNTS;
@@ -131,7 +134,7 @@
final Activity activity = getActivity();
final View view = getView();
- mErrorInfoView = (TextView)view.findViewById(R.id.sync_settings_error_info);
+ mErrorInfoView = (TextView) view.findViewById(R.id.sync_settings_error_info);
mErrorInfoView.setVisibility(View.GONE);
mAuthorities = activity.getIntent().getStringArrayExtra(AUTHORITIES_FILTER_KEY);
@@ -188,8 +191,7 @@
@Override
public void onPrepareOptionsMenu(Menu menu) {
super.onPrepareOptionsMenu(menu);
- boolean syncActive = !ContentResolver.getCurrentSyncsAsUser(
- mUserHandle.getIdentifier()).isEmpty();
+ boolean syncActive = !getCurrentSyncs(mUserHandle.getIdentifier()).isEmpty();
menu.findItem(MENU_SYNC_NOW_ID).setVisible(!syncActive);
menu.findItem(MENU_SYNC_CANCEL_ID).setVisible(syncActive);
}
@@ -197,12 +199,12 @@
@Override
public boolean onOptionsItemSelected(MenuItem item) {
switch (item.getItemId()) {
- case MENU_SYNC_NOW_ID:
- requestOrCancelSyncForAccounts(true);
- return true;
- case MENU_SYNC_CANCEL_ID:
- requestOrCancelSyncForAccounts(false);
- return true;
+ case MENU_SYNC_NOW_ID:
+ requestOrCancelSyncForAccounts(true);
+ return true;
+ case MENU_SYNC_CANCEL_ID:
+ requestOrCancelSyncForAccounts(false);
+ return true;
}
return super.onOptionsItemSelected(item);
}
@@ -223,7 +225,7 @@
SyncAdapterType sa = syncAdapters[j];
if (syncAdapters[j].accountType.equals(mAccountType)
&& ContentResolver.getSyncAutomaticallyAsUser(account, sa.authority,
- userId)) {
+ userId)) {
if (sync) {
ContentResolver.requestSyncAsUser(account, sa.authority, userId,
extras);
@@ -238,47 +240,58 @@
@Override
protected void onSyncStateUpdated() {
- showSyncState();
- // Catch any delayed delivery of update messages
final Activity activity = getActivity();
- if (activity != null) {
- activity.invalidateOptionsMenu();
+ // Catch any delayed delivery of update messages
+ if (activity == null || activity.isFinishing()) {
+ return;
+ }
+ showSyncState();
+ activity.invalidateOptionsMenu();
+ }
+
+ private void tryInitUserFacingSyncAuthorities(int userId) {
+ if (mUserFacingSyncAuthorities != null) {
+ return;
+ }
+ mUserFacingSyncAuthorities = new ArraySet<>();
+
+ // only track userfacing sync adapters when deciding if account is synced or not
+ final SyncAdapterType[] syncAdapters = ContentResolver.getSyncAdapterTypesAsUser(userId);
+ for (int k = 0, n = syncAdapters.length; k < n; k++) {
+ final SyncAdapterType sa = syncAdapters[k];
+ if (sa.isUserVisible()) {
+ mUserFacingSyncAuthorities.add(sa.authority);
+ }
}
}
/**
* Shows the sync state of the accounts. Note: it must be called after the accounts have been
- * loaded, @see #showAccountsIfNeeded().
+ * loaded.
+ *
+ * @see {@link #showAccountsIfNeeded()}.
*/
- private void showSyncState() {
- // Catch any delayed delivery of update messages
- if (getActivity() == null || getActivity().isFinishing()) return;
-
+ @VisibleForTesting
+ void showSyncState() {
final int userId = mUserHandle.getIdentifier();
+ tryInitUserFacingSyncAuthorities(userId);
// iterate over all the preferences, setting the state properly for each
- List<SyncInfo> currentSyncs = ContentResolver.getCurrentSyncsAsUser(userId);
+ final List<SyncInfo> currentSyncs = getCurrentSyncs(userId);
boolean anySyncFailed = false; // true if sync on any account failed
Date date = new Date();
- // only track userfacing sync adapters when deciding if account is synced or not
- final SyncAdapterType[] syncAdapters = ContentResolver.getSyncAdapterTypesAsUser(userId);
- HashSet<String> userFacing = new HashSet<String>();
- for (int k = 0, n = syncAdapters.length; k < n; k++) {
- final SyncAdapterType sa = syncAdapters[k];
- if (sa.isUserVisible()) {
- userFacing.add(sa.authority);
- }
- }
- for (int i = 0, count = getPreferenceScreen().getPreferenceCount(); i < count; i++) {
- Preference pref = getPreferenceScreen().getPreference(i);
- if (! (pref instanceof AccountPreference)) {
+ final PreferenceScreen screen = getPreferenceScreen();
+ final int prefCount = screen.getPreferenceCount();
+ for (int i = 0; i < prefCount; i++) {
+ Preference pref = screen.getPreference(i);
+ if (!(pref instanceof AccountPreference)) {
continue;
}
- AccountPreference accountPref = (AccountPreference) pref;
- Account account = accountPref.getAccount();
+ final AccountPreference accountPref = (AccountPreference) pref;
+ final Account account = accountPref.getAccount();
int syncCount = 0;
long lastSuccessTime = 0;
boolean syncIsFailing = false;
@@ -286,28 +299,33 @@
boolean syncingNow = false;
if (authorities != null) {
for (String authority : authorities) {
- SyncStatusInfo status = ContentResolver.getSyncStatusAsUser(account, authority,
- userId);
+ SyncStatusInfo status = getSyncStatusInfo(account, authority, userId);
boolean syncEnabled = isSyncEnabled(userId, account, authority);
- boolean authorityIsPending = ContentResolver.isSyncPending(account, authority);
boolean activelySyncing = isSyncing(currentSyncs, account, authority);
boolean lastSyncFailed = status != null
&& syncEnabled
&& status.lastFailureTime != 0
&& status.getLastFailureMesgAsInt(0)
- != ContentResolver.SYNC_ERROR_SYNC_ALREADY_IN_PROGRESS;
- if (lastSyncFailed && !activelySyncing && !authorityIsPending) {
+ != ContentResolver.SYNC_ERROR_SYNC_ALREADY_IN_PROGRESS;
+ if (lastSyncFailed && !activelySyncing
+ && !ContentResolver.isSyncPending(account, authority)) {
syncIsFailing = true;
anySyncFailed = true;
+ break;
}
- syncingNow |= activelySyncing;
+
if (status != null && lastSuccessTime < status.lastSuccessTime) {
lastSuccessTime = status.lastSuccessTime;
}
- syncCount += syncEnabled && userFacing.contains(authority) ? 1 : 0;
+ syncCount += syncEnabled && mUserFacingSyncAuthorities.contains(authority)
+ ? 1 : 0;
+ syncingNow |= activelySyncing;
+ if (syncingNow) {
+ break;
+ }
}
} else {
- if (Log.isLoggable(TAG, Log.VERBOSE)) {
+ if (VERBOSE) {
Log.v(TAG, "no syncadapters found for " + account);
}
}
@@ -332,14 +350,14 @@
accountPref.setSyncStatus(AccountPreference.SYNC_DISABLED, true);
}
}
-
- mErrorInfoView.setVisibility(anySyncFailed ? View.VISIBLE : View.GONE);
+ if (mErrorInfoView != null) {
+ mErrorInfoView.setVisibility(anySyncFailed ? View.VISIBLE : View.GONE);
+ }
}
-
private boolean isSyncing(List<SyncInfo> currentSyncs, Account account, String authority) {
final int count = currentSyncs.size();
- for (int i = 0; i < count; i++) {
+ for (int i = 0; i < count; i++) {
SyncInfo syncInfo = currentSyncs.get(i);
if (syncInfo.account.equals(account) && syncInfo.authority.equals(authority)) {
return true;
@@ -348,7 +366,8 @@
return false;
}
- private boolean isSyncEnabled(int userId, Account account, String authority) {
+ @VisibleForTesting
+ protected boolean isSyncEnabled(int userId, Account account, String authority) {
return ContentResolver.getSyncAutomaticallyAsUser(account, authority, userId)
&& ContentResolver.getMasterSyncAutomaticallyAsUser(userId)
&& (ContentResolver.getIsSyncableAsUser(account, authority, userId) > 0);
@@ -436,7 +455,7 @@
*/
private void updatePreferenceIntents(PreferenceScreen prefs) {
final PackageManager pm = getActivity().getPackageManager();
- for (int i = 0; i < prefs.getPreferenceCount();) {
+ for (int i = 0; i < prefs.getPreferenceCount(); ) {
Preference pref = prefs.getPreference(i);
Intent intent = pref.getIntent();
if (intent != null) {
@@ -486,8 +505,8 @@
} else {
Log.e(TAG,
"Refusing to launch authenticator intent because"
- + "it exploits Settings permissions: "
- + prefIntent);
+ + "it exploits Settings permissions: "
+ + prefIntent);
}
return true;
}
@@ -536,4 +555,14 @@
}
}
}
+
+ @VisibleForTesting
+ protected List<SyncInfo> getCurrentSyncs(int userId) {
+ return ContentResolver.getCurrentSyncsAsUser(userId);
+ }
+
+ @VisibleForTesting
+ protected SyncStatusInfo getSyncStatusInfo(Account account, String authority, int userId) {
+ return ContentResolver.getSyncStatusAsUser(account, authority, userId);
+ }
}
diff --git a/tests/robotests/src/com/android/settings/accounts/AccountPreferenceTest.java b/tests/robotests/src/com/android/settings/accounts/AccountPreferenceTest.java
new file mode 100644
index 0000000..eeca90b
--- /dev/null
+++ b/tests/robotests/src/com/android/settings/accounts/AccountPreferenceTest.java
@@ -0,0 +1,73 @@
+/*
+ * Copyright (C) 2016 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.settings.accounts;
+
+import android.accounts.Account;
+import android.content.Context;
+
+import com.android.settings.R;
+import com.android.settings.SettingsRobolectricTestRunner;
+import com.android.settings.TestConfig;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.annotation.Config;
+import org.robolectric.shadows.ShadowApplication;
+
+import java.util.ArrayList;
+
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
+
+@RunWith(SettingsRobolectricTestRunner.class)
+@Config(manifest = TestConfig.MANIFEST_PATH, sdk = TestConfig.SDK_VERSION)
+public class AccountPreferenceTest {
+
+ private Context mContext;
+ private Account mAccount;
+ private ArrayList<String> mAuthorities;
+ private AccountPreference mPreference;
+
+ @Before
+ public void setUp() {
+ mContext = ShadowApplication.getInstance().getApplicationContext();
+ mAccount = new Account("name", "type");
+ mAuthorities = new ArrayList<>();
+ mAuthorities.add("authority");
+
+ mPreference = spy(new AccountPreference(
+ mContext, mAccount, null /* icon */, mAuthorities, false /* showTypeIcon */));
+ }
+
+ @Test
+ public void setSyncStatus_differentStatus_shouldUpdate() {
+ mPreference.setSyncStatus(AccountPreference.SYNC_ERROR, true);
+ verify(mPreference).setSummary(R.string.sync_error);
+ }
+
+ @Test
+ public void setSyncStatus_sameStatus_shouldNotUpdate() {
+ // Set it once, should update summary
+ mPreference.setSyncStatus(AccountPreference.SYNC_ERROR, true);
+ verify(mPreference).setSummary(R.string.sync_error);
+
+ // Set it again, should not update summary
+ mPreference.setSyncStatus(AccountPreference.SYNC_ERROR, true);
+ verify(mPreference).setSummary(R.string.sync_error);
+ }
+}
diff --git a/tests/robotests/src/com/android/settings/accounts/ManageAccountsSettingsTest.java b/tests/robotests/src/com/android/settings/accounts/ManageAccountsSettingsTest.java
new file mode 100644
index 0000000..916e395
--- /dev/null
+++ b/tests/robotests/src/com/android/settings/accounts/ManageAccountsSettingsTest.java
@@ -0,0 +1,135 @@
+/*
+ * Copyright (C) 2016 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.settings.accounts;
+
+import android.accounts.Account;
+import android.content.SyncInfo;
+import android.content.SyncStatusInfo;
+import android.os.UserHandle;
+import android.support.v7.preference.PreferenceScreen;
+import android.util.ArraySet;
+
+import com.android.settings.SettingsRobolectricTestRunner;
+import com.android.settings.TestConfig;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Answers;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+import org.robolectric.annotation.Config;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.mockito.Matchers.anyInt;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+@RunWith(SettingsRobolectricTestRunner.class)
+@Config(manifest = TestConfig.MANIFEST_PATH, sdk = TestConfig.SDK_VERSION)
+public class ManageAccountsSettingsTest {
+
+ @Mock(answer = Answers.RETURNS_DEEP_STUBS)
+ private AccountPreference mAccountPref;
+ private Account mAccount;
+ private ArrayList<String> mAuthorities;
+ private TestFragment mSettings;
+
+ @Before
+ public void setUp() {
+ MockitoAnnotations.initMocks(this);
+ mAuthorities = new ArrayList<>();
+ mAuthorities.add("authority");
+ mAccount = new Account("name", "type");
+ when(mAccountPref.getAccount()).thenReturn(mAccount);
+ when(mAccountPref.getAuthorities()).thenReturn(mAuthorities);
+ mSettings = new TestFragment();
+ }
+
+ @Test
+ public void showSyncState_noAccountPrefs_shouldUpdateNothing() {
+ when(mAccountPref.getAuthorities()).thenReturn(null);
+ mSettings.showSyncState();
+ verify(mSettings.getPreferenceScreen(), never()).getPreference(anyInt());
+ }
+
+ @Test
+ public void showSyncState_syncInProgress_shouldUpdateInProgress() {
+ mSettings.mUserFacingSyncAuthorities.add(mAuthorities.get(0));
+ mSettings.mSyncInfos.add(new SyncInfo(0, mAccount, mAuthorities.get(0), 0));
+ mSettings.mSyncStatusInfo = new SyncStatusInfo(0);
+ when(mSettings.getPreferenceScreen().getPreferenceCount()).thenReturn(1);
+ when(mSettings.getPreferenceScreen().getPreference(0)).thenReturn(mAccountPref);
+
+ mSettings.showSyncState();
+
+ verify(mSettings.getPreferenceScreen()).getPreference(anyInt());
+ verify(mAccountPref).setSyncStatus(AccountPreference.SYNC_IN_PROGRESS, true);
+ }
+
+ @Test
+ public void showSyncState_noUserFacingSynclets_shouldUpdateToDisabled() {
+ mSettings.mSyncInfos.add(new SyncInfo(0, mAccount, mAuthorities.get(0), 0));
+ mSettings.mSyncStatusInfo = new SyncStatusInfo(0);
+ when(mSettings.getPreferenceScreen().getPreferenceCount()).thenReturn(1);
+ when(mSettings.getPreferenceScreen().getPreference(0)).thenReturn(mAccountPref);
+
+ mSettings.showSyncState();
+
+ verify(mSettings.getPreferenceScreen()).getPreference(anyInt());
+ verify(mAccountPref).setSyncStatus(AccountPreference.SYNC_DISABLED, true);
+ }
+
+ public static class TestFragment extends ManageAccountsSettings {
+
+ private PreferenceScreen mScreen;
+ private List<SyncInfo> mSyncInfos;
+ private SyncStatusInfo mSyncStatusInfo;
+
+ public TestFragment() {
+ mUserHandle = mock(UserHandle.class);
+ mScreen = mock(PreferenceScreen.class);
+ mUserFacingSyncAuthorities = new ArraySet<>();
+ mSyncInfos = new ArrayList<>();
+ }
+
+ @Override
+ public PreferenceScreen getPreferenceScreen() {
+ return mScreen;
+ }
+
+ @Override
+ protected boolean isSyncEnabled(int userId, Account account, String authority) {
+ return true;
+ }
+
+ @Override
+ protected List<SyncInfo> getCurrentSyncs(int userId) {
+ return mSyncInfos;
+ }
+
+ @Override
+ protected SyncStatusInfo getSyncStatusInfo(Account account, String authority, int userId) {
+ return mSyncStatusInfo;
+ }
+ }
+
+}
\ No newline at end of file