Only broadcast LOGIN_ACCOUNTS_CHANGED when changes occur.

Bug: 30014605
Change-Id: I451a795494a9dcbfa5e6afa76d8716911e5dc3a5

1 files changed, 81 insertions, 61 deletions

diff --git a/services/core/java/com/android/server/accounts/AccountManagerService.java b/services/core/java/com/android/server/accounts/AccountManagerService.java
index f7bd04b08ede..577cada36c3e 100644
--- a/services/core/java/com/android/server/accounts/AccountManagerService.java
+++ b/services/core/java/com/android/server/accounts/AccountManagerService.java
@@ -1225,11 +1225,13 @@ public class AccountManagerService
             } finally {
                 db.endTransaction();
             }
-            sendAccountsChangedBroadcast(accounts.userId);
         }
         if (getUserManager().getUserInfo(accounts.userId).canHaveProfile()) {
             addAccountToLinkedRestrictedUsers(account, accounts.userId);
         }
+
+        // Only send LOGIN_ACCOUNTS_CHANGED when the database changed.
+        sendAccountsChangedBroadcast(accounts.userId);
         return true;
     }
 
@@ -1412,7 +1414,6 @@ public class AccountManagerService
         synchronized (accounts.cacheLock) {
             final SQLiteDatabase db = accounts.openHelper.getWritableDatabaseUserIsUnlocked();
             db.beginTransaction();
-            boolean isSuccessful = false;
             Account renamedAccount = new Account(newName, accountToRename.type);
             try {
                 final long accountId = getAccountIdLocked(db, accountToRename);
@@ -1425,54 +1426,51 @@ public class AccountManagerService
                     values.put(ACCOUNTS_PREVIOUS_NAME, accountToRename.name);
                     db.update(TABLE_ACCOUNTS, values, ACCOUNTS_ID + "=?", argsAccountId);
                     db.setTransactionSuccessful();
-                    isSuccessful = true;
                     logRecord(db, DebugDbHelper.ACTION_ACCOUNT_RENAME, TABLE_ACCOUNTS, accountId,
                             accounts);
                 }
             } finally {
                 db.endTransaction();
-                if (isSuccessful) {
-                    /*
-                     * Database transaction was successful. Clean up cached
-                     * data associated with the account in the user profile.
-                     */
-                    insertAccountIntoCacheLocked(accounts, renamedAccount);
-                    /*
-                     * Extract the data and token caches before removing the
-                     * old account to preserve the user data associated with
-                     * the account.
-                     */
-                    HashMap<String, String> tmpData = accounts.userDataCache.get(accountToRename);
-                    HashMap<String, String> tmpTokens = accounts.authTokenCache.get(accountToRename);
-                    removeAccountFromCacheLocked(accounts, accountToRename);
-                    /*
-                     * Update the cached data associated with the renamed
-                     * account.
-                     */
-                    accounts.userDataCache.put(renamedAccount, tmpData);
-                    accounts.authTokenCache.put(renamedAccount, tmpTokens);
-                    accounts.previousNameCache.put(
-                          renamedAccount,
-                          new AtomicReference<String>(accountToRename.name));
-                    resultAccount = renamedAccount;
-
-                    int parentUserId = accounts.userId;
-                    if (canHaveProfile(parentUserId)) {
-                        /*
-                         * Owner or system user account was renamed, rename the account for
-                         * those users with which the account was shared.
-                         */
-                        List<UserInfo> users = getUserManager().getUsers(true);
-                        for (UserInfo user : users) {
-                            if (user.isRestricted()
-                                    && (user.restrictedProfileParentId == parentUserId)) {
-                                renameSharedAccountAsUser(accountToRename, newName, user.id);
-                            }
-                        }
+            }
+            /*
+             * Database transaction was successful. Clean up cached
+             * data associated with the account in the user profile.
+             */
+            insertAccountIntoCacheLocked(accounts, renamedAccount);
+            /*
+             * Extract the data and token caches before removing the
+             * old account to preserve the user data associated with
+             * the account.
+             */
+            HashMap<String, String> tmpData = accounts.userDataCache.get(accountToRename);
+            HashMap<String, String> tmpTokens = accounts.authTokenCache.get(accountToRename);
+            removeAccountFromCacheLocked(accounts, accountToRename);
+            /*
+             * Update the cached data associated with the renamed
+             * account.
+             */
+            accounts.userDataCache.put(renamedAccount, tmpData);
+            accounts.authTokenCache.put(renamedAccount, tmpTokens);
+            accounts.previousNameCache.put(
+                    renamedAccount,
+                    new AtomicReference<String>(accountToRename.name));
+            resultAccount = renamedAccount;
+
+            int parentUserId = accounts.userId;
+            if (canHaveProfile(parentUserId)) {
+                /*
+                 * Owner or system user account was renamed, rename the account for
+                 * those users with which the account was shared.
+                 */
+                List<UserInfo> users = getUserManager().getUsers(true);
+                for (UserInfo user : users) {
+                    if (user.isRestricted()
+                            && (user.restrictedProfileParentId == parentUserId)) {
+                        renameSharedAccountAsUser(accountToRename, newName, user.id);
                     }
-                    sendAccountsChangedBroadcast(accounts.userId);
                 }
             }
+            sendAccountsChangedBroadcast(accounts.userId);
         }
         return resultAccount;
     }
@@ -1653,7 +1651,7 @@ public class AccountManagerService
     }
 
     private boolean removeAccountInternal(UserAccounts accounts, Account account, int callingUid) {
-        int deleted;
+        boolean isChanged = false;
         boolean userUnlocked = isLocalUnlockedUser(accounts.userId);
         if (!userUnlocked) {
             Slog.i(TAG, "Removing account " + account + " while user "+ accounts.userId
@@ -1663,25 +1661,38 @@ public class AccountManagerService
             final SQLiteDatabase db = userUnlocked
                     ? accounts.openHelper.getWritableDatabaseUserIsUnlocked()
                     : accounts.openHelper.getWritableDatabase();
-            final long accountId = getAccountIdLocked(db, account);
             db.beginTransaction();
+            // Set to a dummy value, this will only be used if the database
+            // transaction succeeds.
+            long accountId = -1;
             try {
-                deleted = db.delete(TABLE_ACCOUNTS, ACCOUNTS_NAME + "=? AND " + ACCOUNTS_TYPE
-                                + "=?", new String[]{account.name, account.type});
-                if (userUnlocked) {
-                    // Delete from CE table
-                    deleted = db.delete(CE_TABLE_ACCOUNTS, ACCOUNTS_NAME + "=? AND " + ACCOUNTS_TYPE
-                            + "=?", new String[]{account.name, account.type});
+                accountId = getAccountIdLocked(db, account);
+                if (accountId >= 0) {
+                    db.delete(
+                            TABLE_ACCOUNTS,
+                            ACCOUNTS_NAME + "=? AND " + ACCOUNTS_TYPE + "=?",
+                            new String[]{ account.name, account.type });
+                    if (userUnlocked) {
+                        // Delete from CE table
+                        db.delete(
+                                CE_TABLE_ACCOUNTS,
+                                ACCOUNTS_NAME + "=? AND " + ACCOUNTS_TYPE + "=?",
+                                new String[]{ account.name, account.type });
+                    }
+                    db.setTransactionSuccessful();
+                    isChanged = true;
                 }
-                db.setTransactionSuccessful();
             } finally {
                 db.endTransaction();
             }
-            removeAccountFromCacheLocked(accounts, account);
-            sendAccountsChangedBroadcast(accounts.userId);
-            String action = userUnlocked ? DebugDbHelper.ACTION_ACCOUNT_REMOVE
-                    : DebugDbHelper.ACTION_ACCOUNT_REMOVE_DE;
-            logRecord(db, action, TABLE_ACCOUNTS, accountId, accounts);
+            if (isChanged) {
+                removeAccountFromCacheLocked(accounts, account);
+                // Only broadcast LOGIN_ACCOUNTS_CHANGED if a change occured.
+                sendAccountsChangedBroadcast(accounts.userId);
+                String action = userUnlocked ? DebugDbHelper.ACTION_ACCOUNT_REMOVE
+                        : DebugDbHelper.ACTION_ACCOUNT_REMOVE_DE;
+                logRecord(db, action, TABLE_ACCOUNTS, accountId, accounts);
+            }
         }
         long id = Binder.clearCallingIdentity();
         try {
@@ -1698,7 +1709,7 @@ public class AccountManagerService
         } finally {
             Binder.restoreCallingIdentity(id);
         }
-        return (deleted > 0);
+        return isChanged;
     }
 
     @Override
@@ -1922,6 +1933,7 @@ public class AccountManagerService
         if (account == null) {
             return;
         }
+        boolean isChanged = false;
         synchronized (accounts.cacheLock) {
             final SQLiteDatabase db = accounts.openHelper.getWritableDatabaseUserIsUnlocked();
             db.beginTransaction();
@@ -1931,12 +1943,17 @@ public class AccountManagerService
                 final long accountId = getAccountIdLocked(db, account);
                 if (accountId >= 0) {
                     final String[] argsAccountId = {String.valueOf(accountId)};
-                    db.update(CE_TABLE_ACCOUNTS, values, ACCOUNTS_ID + "=?", argsAccountId);
-                    db.delete(CE_TABLE_AUTHTOKENS, AUTHTOKENS_ACCOUNTS_ID + "=?", argsAccountId);
+                    db.update(
+                            CE_TABLE_ACCOUNTS, values, ACCOUNTS_ID + "=?", argsAccountId);
+                    db.delete(
+                            CE_TABLE_AUTHTOKENS, AUTHTOKENS_ACCOUNTS_ID + "=?", argsAccountId);
                     accounts.authTokenCache.remove(account);
                     accounts.accountTokenCaches.remove(account);
                     db.setTransactionSuccessful();
-
+                    // If there is an account whose password will be updated and the database
+                    // transactions succeed, then we say that a change has occured. Even if the
+                    // new password is the same as the old and there were no authtokens to delete.
+                    isChanged = true;
                     String action = (password == null || password.length() == 0) ?
                             DebugDbHelper.ACTION_CLEAR_PASSWORD
                             : DebugDbHelper.ACTION_SET_PASSWORD;
@@ -1944,8 +1961,11 @@ public class AccountManagerService
                 }
             } finally {
                 db.endTransaction();
+                if (isChanged) {
+                    // Send LOGIN_ACCOUNTS_CHANGED only if the something changed.
+                    sendAccountsChangedBroadcast(accounts.userId);
+                }
             }
-            sendAccountsChangedBroadcast(accounts.userId);
         }
     }