From 2959a7e073c87e2fa5fab42ec543b352a91cf187 Mon Sep 17 00:00:00 2001 From: Andy Stadler Date: Thu, 23 Dec 2010 13:19:55 -0800 Subject: [PATCH] Fix ANRs from Email.setServicesEnabled() * Create sync & async versions * Rename all callsites so sync is very apparent * Fix callsites appropriately * Clean up interaction between reconciler and setServicesEnabled Bug: 3133770 Bug: 3134677 Change-Id: Iefbc7814d9aa390baea6345e450e2a4768bf0a9a --- .../android/email/AccountBackupRestore.java | 2 +- src/com/android/email/Controller.java | 2 +- src/com/android/email/Email.java | 26 ++++++++- .../email/activity/UpgradeAccounts.java | 2 +- .../setup/AccountSettingsFragment.java | 7 +-- .../activity/setup/AccountSetupOptions.java | 5 +- .../EmailBroadcastProcessorService.java | 2 +- src/com/android/exchange/ExchangeService.java | 57 ++++++++++--------- .../email/activity/MessageComposeTests.java | 2 +- 9 files changed, 64 insertions(+), 41 deletions(-) diff --git a/src/com/android/email/AccountBackupRestore.java b/src/com/android/email/AccountBackupRestore.java index a016fb2bc..c75834768 100644 --- a/src/com/android/email/AccountBackupRestore.java +++ b/src/com/android/email/AccountBackupRestore.java @@ -72,7 +72,7 @@ public class AccountBackupRestore { // update security profile SecurityPolicy.getInstance(context).updatePolicies(-1); // enable/disable other email services as necessary - Email.setServicesEnabled(context); + Email.setServicesEnabledSync(context); ExchangeUtils.startExchangeService(context); } sBackupsChecked = true; diff --git a/src/com/android/email/Controller.java b/src/com/android/email/Controller.java index 0866d12ca..a4db02369 100644 --- a/src/com/android/email/Controller.java +++ b/src/com/android/email/Controller.java @@ -963,7 +963,7 @@ public class Controller { // Release or relax device administration, if relevant SecurityPolicy.getInstance(context).reducePolicies(); - Email.setServicesEnabled(context); + Email.setServicesEnabledSync(context); } catch (Exception e) { Log.w(Email.LOG_TAG, "Exception while deleting account", e); } finally { diff --git a/src/com/android/email/Email.java b/src/com/android/email/Email.java index 24e64d7fe..c051e74df 100644 --- a/src/com/android/email/Email.java +++ b/src/com/android/email/Email.java @@ -175,12 +175,32 @@ public class Email extends Application { return sTempDirectory; } + /** + * Asynchronous version of {@link #setServicesEnabledSync(Context)}. Use when calling from + * UI thread (or lifecycle entry points.) + * + * @param context + */ + public static void setServicesEnabledAsync(final Context context) { + Utility.runAsync(new Runnable() { + @Override + public void run() { + setServicesEnabledSync(context); + } + }); + } + /** * Called throughout the application when the number of accounts has changed. This method * enables or disables the Compose activity, the boot receiver and the service based on - * whether any accounts are configured. Returns true if there are any accounts configured. + * whether any accounts are configured. + * + * Blocking call - do not call from UI/lifecycle threads. + * + * @param context + * @return true if there are any accounts configured. */ - public static boolean setServicesEnabled(Context context) { + public static boolean setServicesEnabledSync(Context context) { Cursor c = null; try { c = context.getContentResolver().query( @@ -197,7 +217,7 @@ public class Email extends Application { } } - public static void setServicesEnabled(Context context, boolean enabled) { + private static void setServicesEnabled(Context context, boolean enabled) { PackageManager pm = context.getPackageManager(); if (!enabled && pm.getComponentEnabledSetting( new ComponentName(context, MailService.class)) == diff --git a/src/com/android/email/activity/UpgradeAccounts.java b/src/com/android/email/activity/UpgradeAccounts.java index d30e96169..366b07863 100644 --- a/src/com/android/email/activity/UpgradeAccounts.java +++ b/src/com/android/email/activity/UpgradeAccounts.java @@ -410,7 +410,7 @@ public class UpgradeAccounts extends ListActivity implements OnClickListener { } // Step 4: Enable app-wide features such as composer, and start mail service(s) - Email.setServicesEnabled(mContext); + Email.setServicesEnabledSync(mContext); } return null; diff --git a/src/com/android/email/activity/setup/AccountSettingsFragment.java b/src/com/android/email/activity/setup/AccountSettingsFragment.java index e2381f5a8..7a9e39c26 100644 --- a/src/com/android/email/activity/setup/AccountSettingsFragment.java +++ b/src/com/android/email/activity/setup/AccountSettingsFragment.java @@ -620,12 +620,7 @@ public class AccountSettingsFragment extends PreferenceFragment { mAccount.update(mContext, cv); // Run the remaining changes off-thread - Utility.runAsync(new Runnable() { - @Override - public void run() { - Email.setServicesEnabled(mContext); - } - }); + Email.setServicesEnabledAsync(mContext); } /** diff --git a/src/com/android/email/activity/setup/AccountSetupOptions.java b/src/com/android/email/activity/setup/AccountSetupOptions.java index 4532c58bf..75b2b0eb4 100644 --- a/src/com/android/email/activity/setup/AccountSetupOptions.java +++ b/src/com/android/email/activity/setup/AccountSetupOptions.java @@ -223,12 +223,15 @@ public class AccountSetupOptions extends AccountSetupActivity implements OnClick saveAccountAndFinish(); } + /** + * STOPSHIP most of this work needs to be async + */ private void saveAccountAndFinish() { // Clear the incomplete/security hold flag now Account account = SetupData.getAccount(); account.mFlags &= ~(Account.FLAGS_INCOMPLETE | Account.FLAGS_SECURITY_HOLD); AccountSettingsUtils.commitSettings(this, account); - Email.setServicesEnabled(this); + Email.setServicesEnabledSync(this); AccountSetupNames.actionSetNames(this); // Start up ExchangeService (if it isn't already running) ExchangeUtils.startExchangeService(this); diff --git a/src/com/android/email/service/EmailBroadcastProcessorService.java b/src/com/android/email/service/EmailBroadcastProcessorService.java index 38d5b1709..0101cc922 100644 --- a/src/com/android/email/service/EmailBroadcastProcessorService.java +++ b/src/com/android/email/service/EmailBroadcastProcessorService.java @@ -87,7 +87,7 @@ public class EmailBroadcastProcessorService extends IntentService { } private void enableComponentsIfNecessary() { - if (Email.setServicesEnabled(this)) { + if (Email.setServicesEnabledSync(this)) { // At least one account exists. // TODO probably we should check if it's a POP/IMAP account. MailService.actionReschedule(this); diff --git a/src/com/android/exchange/ExchangeService.java b/src/com/android/exchange/ExchangeService.java index de7594b14..f58e0b467 100644 --- a/src/com/android/exchange/ExchangeService.java +++ b/src/com/android/exchange/ExchangeService.java @@ -1046,36 +1046,34 @@ public class ExchangeService extends Service implements Runnable { */ public class EasAccountsUpdatedListener implements OnAccountsUpdateListener { public void onAccountsUpdated(android.accounts.Account[] accounts) { - ExchangeService exchangeService = INSTANCE; + final ExchangeService exchangeService = INSTANCE; if (exchangeService != null) { - exchangeService.runAccountReconciler(); + Utility.runAsync(new Runnable() { + @Override + public void run() { + exchangeService.runAccountReconcilerSync(exchangeService); + } + }); } } } /** - * Non-blocking call to run the account reconciler. - * Launches a worker thread, so it may be called from UI thread. + * Blocking call to the account reconciler */ - private void runAccountReconciler() { - final ExchangeService exchangeService = this; - new Thread() { - @Override - public void run() { - android.accounts.Account[] accountMgrList = AccountManager.get(exchangeService) - .getAccountsByType(Email.EXCHANGE_ACCOUNT_MANAGER_TYPE); - synchronized (mAccountList) { - // Make sure we have an up-to-date sAccountList. If not (for example, if the - // service has been destroyed), we would be reconciling against an empty account - // list, which would cause the deletion of all of our accounts - if (mAccountObserver != null) { - mAccountObserver.onAccountChanged(); - MailService.reconcileAccountsWithAccountManager(exchangeService, - mAccountList, accountMgrList, false, mResolver); - } - } + private void runAccountReconcilerSync(Context context) { + android.accounts.Account[] accountMgrList = AccountManager.get(context) + .getAccountsByType(Email.EXCHANGE_ACCOUNT_MANAGER_TYPE); + synchronized (mAccountList) { + // Make sure we have an up-to-date sAccountList. If not (for example, if the + // service has been destroyed), we would be reconciling against an empty account + // list, which would cause the deletion of all of our accounts + if (mAccountObserver != null) { + mAccountObserver.onAccountChanged(); + MailService.reconcileAccountsWithAccountManager(context, + mAccountList, accountMgrList, false, mResolver); } - }.start(); + } } public static void log(String str) { @@ -1752,7 +1750,6 @@ public class ExchangeService extends Service implements Runnable { @Override public void onCreate() { synchronized (sSyncLock) { - Email.setServicesEnabled(this); alwaysLog("!!! EAS ExchangeService, onCreate"); if (sStop) { return; @@ -1765,9 +1762,17 @@ public class ExchangeService extends Service implements Runnable { throw new RuntimeException(e); } } - // Run the reconciler and clean up any mismatched accounts - if we weren't running when - // accounts were deleted, it won't have been called. - runAccountReconciler(); + // Finally, run some setup activities off the UI thread + Utility.runAsync(new Runnable() { + @Override + public void run() { + // Run the reconciler and clean up any mismatched accounts - if we weren't + // running when accounts were deleted, it won't have been called. + runAccountReconcilerSync(ExchangeService.this); + // Update other services depending on final account configuration + Email.setServicesEnabledSync(ExchangeService.this); + } + }); } } diff --git a/tests/src/com/android/email/activity/MessageComposeTests.java b/tests/src/com/android/email/activity/MessageComposeTests.java index 76ad46c5d..6a71d7609 100644 --- a/tests/src/com/android/email/activity/MessageComposeTests.java +++ b/tests/src/com/android/email/activity/MessageComposeTests.java @@ -137,7 +137,7 @@ public class MessageComposeTests } Account account = Account.restoreAccountWithId(mContext, accountId); mSignature = account.getSignature(); - Email.setServicesEnabled(mContext); + Email.setServicesEnabledSync(mContext); Intent intent = new Intent(Intent.ACTION_VIEW); setActivityIntent(intent);