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

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.support.v7.preference.PreferenceViewHolder;
 import android.util.Log;
 import android.widget.ImageView;
 
+import com.android.settings.R;
+
 import java.util.ArrayList;
 
 /**
@@ -78,6 +80,10 @@ public class AccountPreference extends Preference {
     }
 
     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));

src/com/android/settings/accounts/AccountPreferenceBase.java

@@ -32,6 +32,7 @@ import android.os.UserHandle;
 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 @@ abstract class AccountPreferenceBase extends SettingsPreferenceFragment
         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";

src/com/android/settings/accounts/ManageAccountsSettings.java

@@ -34,9 +34,11 @@ import android.content.pm.ResolveInfo;
 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.view.ViewGroup;
 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 com.android.settingslib.accounts.AuthenticatorHelper;
 
 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 @@ public class ManageAccountsSettings extends AccountPreferenceBase
             "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 @@ public class ManageAccountsSettings extends AccountPreferenceBase
     // 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 @@ public class ManageAccountsSettings extends AccountPreferenceBase
         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 @@ public class ManageAccountsSettings extends AccountPreferenceBase
     @Override
     public void onPrepareOptionsMenu(Menu menu) {
         super.onPrepareOptionsMenu(menu);
-        boolean syncActive = !ContentResolver.getCurrentSyncsAsUser(
+        boolean syncActive = !getCurrentSyncs(mUserHandle.getIdentifier()).isEmpty();
-                mUserHandle.getIdentifier()).isEmpty();
         menu.findItem(MENU_SYNC_NOW_ID).setVisible(!syncActive);
         menu.findItem(MENU_SYNC_CANCEL_ID).setVisible(syncActive);
     }
@@ -197,12 +199,12 @@ public class ManageAccountsSettings extends AccountPreferenceBase
     @Override
     public boolean onOptionsItemSelected(MenuItem item) {
         switch (item.getItemId()) {
-        case MENU_SYNC_NOW_ID:
+            case MENU_SYNC_NOW_ID:
-            requestOrCancelSyncForAccounts(true);
+                requestOrCancelSyncForAccounts(true);
-            return true;
+                return true;
-        case MENU_SYNC_CANCEL_ID:
+            case MENU_SYNC_CANCEL_ID:
-            requestOrCancelSyncForAccounts(false);
+                requestOrCancelSyncForAccounts(false);
-            return true;
+                return true;
         }
         return super.onOptionsItemSelected(item);
     }
@@ -223,7 +225,7 @@ public class ManageAccountsSettings extends AccountPreferenceBase
                     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 @@ public class ManageAccountsSettings extends AccountPreferenceBase
 
     @Override
     protected void onSyncStateUpdated() {
-        showSyncState();
-        // Catch any delayed delivery of update messages
         final Activity activity = getActivity();
-        if (activity != null) {
+        // Catch any delayed delivery of update messages
-            activity.invalidateOptionsMenu();
+        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() {
+    @VisibleForTesting
-        // Catch any delayed delivery of update messages
+    void showSyncState() {
-        if (getActivity() == null || getActivity().isFinishing()) return;
-
         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 PreferenceScreen screen = getPreferenceScreen();
-        final SyncAdapterType[] syncAdapters = ContentResolver.getSyncAdapterTypesAsUser(userId);
+        final int prefCount = screen.getPreferenceCount();
-        HashSet<String> userFacing = new HashSet<String>();
+        for (int i = 0; i < prefCount; i++) {
-        for (int k = 0, n = syncAdapters.length; k < n; k++) {
+            Preference pref = screen.getPreference(i);
-            final SyncAdapterType sa = syncAdapters[k];
+            if (!(pref instanceof AccountPreference)) {
-            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)) {
                 continue;
             }
 
-            AccountPreference accountPref = (AccountPreference) pref;
+            final AccountPreference accountPref = (AccountPreference) pref;
-            Account account = accountPref.getAccount();
+            final Account account = accountPref.getAccount();
             int syncCount = 0;
             long lastSuccessTime = 0;
             boolean syncIsFailing = false;
@@ -286,28 +299,33 @@ public class ManageAccountsSettings extends AccountPreferenceBase
             boolean syncingNow = false;
             if (authorities != null) {
                 for (String authority : authorities) {
-                    SyncStatusInfo status = ContentResolver.getSyncStatusAsUser(account, authority,
+                    SyncStatusInfo status = getSyncStatusInfo(account, authority, userId);
-                            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;
+                            != ContentResolver.SYNC_ERROR_SYNC_ALREADY_IN_PROGRESS;
-                    if (lastSyncFailed && !activelySyncing && !authorityIsPending) {
+                    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 @@ public class ManageAccountsSettings extends AccountPreferenceBase
                 accountPref.setSyncStatus(AccountPreference.SYNC_DISABLED, true);
             }
         }
-
+        if (mErrorInfoView != null) {
-        mErrorInfoView.setVisibility(anySyncFailed ? View.VISIBLE : View.GONE);
+            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 @@ public class ManageAccountsSettings extends AccountPreferenceBase
         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 @@ public class ManageAccountsSettings extends AccountPreferenceBase
      */
     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 @@ public class ManageAccountsSettings extends AccountPreferenceBase
                                 } else {
                                     Log.e(TAG,
                                             "Refusing to launch authenticator intent because"
-                                            + "it exploits Settings permissions: "
+                                                    + "it exploits Settings permissions: "
-                                            + prefIntent);
+                                                    + prefIntent);
                                 }
                                 return true;
                             }
@@ -536,4 +555,14 @@ public class ManageAccountsSettings extends AccountPreferenceBase
             }
         }
     }
+
+    @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);
+    }
 }

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);
+    }
+}

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;
+        }
+    }
+
+}