From c50b6f685ba26a38b8110f6ca0115b12f84d643c Mon Sep 17 00:00:00 2001 From: Makoto Onuki Date: Fri, 14 Jan 2011 12:00:17 -0800 Subject: [PATCH] Use broadcast to detect system account update. Instead of AccountsUpdatedListener. Bug 3211657 Change-Id: I1e60092fc06fe80b9914ff7264c24fcbfa950745 --- AndroidManifest.xml | 12 +- src/com/android/email/SingleRunningTask.java | 63 ++++++++ src/com/android/email/activity/Welcome.java | 32 +--- .../EmailBroadcastProcessorService.java | 12 ++ .../android/email/service/MailService.java | 80 +++------- src/com/android/exchange/ExchangeService.java | 33 ---- .../android/email/SingleRunningTaskTest.java | 150 ++++++++++++++++++ .../email/service/MailServiceTests.java | 18 --- 8 files changed, 260 insertions(+), 140 deletions(-) create mode 100644 src/com/android/email/SingleRunningTask.java create mode 100644 tests/src/com/android/email/SingleRunningTaskTest.java diff --git a/AndroidManifest.xml b/AndroidManifest.xml index dd2bef470..3f2f5699e 100644 --- a/AndroidManifest.xml +++ b/AndroidManifest.xml @@ -296,7 +296,7 @@ android:name="com.android.exchange.MailboxAlarmReceiver"/> - @@ -310,6 +310,8 @@ android:name="android.intent.action.DEVICE_STORAGE_LOW" /> + @@ -365,7 +367,7 @@ android:enabled="true" > - - @@ -505,10 +507,10 @@ - - diff --git a/src/com/android/email/SingleRunningTask.java b/src/com/android/email/SingleRunningTask.java new file mode 100644 index 000000000..f0215faf7 --- /dev/null +++ b/src/com/android/email/SingleRunningTask.java @@ -0,0 +1,63 @@ +/* + * Copyright (C) 2011 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.email; + +import android.util.Log; + +import java.util.concurrent.atomic.AtomicBoolean; + +/** + * Base class for a task that runs at most one instance at any given moment. + * + * Call {@link #run} to start the task. If the task is already running on another thread, it'll do + * nothing. + */ +public abstract class SingleRunningTask { + private final AtomicBoolean mIsRunning = new AtomicBoolean(false); + private final String mLogTaskName; + + public SingleRunningTask(String logTaskName) { + mLogTaskName = logTaskName; + } + + /** + * Calls {@link #runInternal} if it's not running already. + */ + public final void run(Param param) { + if (mIsRunning.compareAndSet(false, true)) { + Log.i(Email.LOG_TAG, mLogTaskName + ": start"); + try { + runInternal(param); + } finally { + Log.i(Email.LOG_TAG, mLogTaskName + ": done"); + mIsRunning.set(false); + } + } else { + // Already running -- do nothing. + Log.i(Email.LOG_TAG, mLogTaskName + ": already running"); + } + } + + /** + * The actual task must be implemented by subclasses. + */ + protected abstract void runInternal(Param param); + + /* package */ boolean isRunningForTest() { + return mIsRunning.get(); + } +} diff --git a/src/com/android/email/activity/Welcome.java b/src/com/android/email/activity/Welcome.java index 26577221b..fbca3bcc2 100644 --- a/src/com/android/email/activity/Welcome.java +++ b/src/com/android/email/activity/Welcome.java @@ -19,15 +19,12 @@ package com.android.email.activity; import com.android.email.AccountBackupRestore; import com.android.email.Email; import com.android.email.ExchangeUtils; -import com.android.email.Utility; import com.android.email.activity.setup.AccountSetupBasics; import com.android.email.provider.EmailContent; import com.android.email.provider.EmailContent.Account; import com.android.email.provider.EmailContent.Mailbox; import com.android.email.service.MailService; -import android.accounts.AccountManager; -import android.accounts.OnAccountsUpdateListener; import android.app.Activity; import android.content.Context; import android.content.Intent; @@ -80,7 +77,6 @@ public class Welcome extends Activity { */ private static final String EXTRA_DEBUG_PANE_MODE = "DEBUG_PANE_MODE"; - private AccountsUpdatedListener mAccountsUpdatedListener; private Handler mHandler = new Handler(); /** @@ -178,17 +174,10 @@ public class Welcome extends Activity { // Because the app could be reloaded (for debugging, etc.), we need to make sure that // ExchangeService gets a chance to start. There is no harm to starting it if it has // already been started + // When the service starts, it reconciles EAS accounts. // TODO More completely separate ExchangeService from Email app ExchangeUtils.startExchangeService(this); - // TODO Move this listener code to a more central location - // Set up our observer for AccountManager - mAccountsUpdatedListener = new AccountsUpdatedListener(); - AccountManager.get(getApplication()).addOnAccountsUpdatedListener( - mAccountsUpdatedListener, mHandler, true); - // Run reconciliation to make sure we're up-to-date on account status - mAccountsUpdatedListener.onAccountsUpdated(null); - final long accountId = getIntent().getLongExtra(EXTRA_ACCOUNT_ID, -1); final long mailboxId = getIntent().getLongExtra(EXTRA_MAILBOX_ID, -1); final long messageId = getIntent().getLongExtra(EXTRA_MESSAGE_ID, -1); @@ -199,22 +188,6 @@ public class Welcome extends Activity { @Override public void onDestroy() { super.onDestroy(); - if (mAccountsUpdatedListener != null) { - AccountManager.get(this).removeOnAccountsUpdatedListener(mAccountsUpdatedListener); - } - } - - /** - * Reconcile accounts when accounts are added/removed from AccountManager - */ - public class AccountsUpdatedListener implements OnAccountsUpdateListener { - public void onAccountsUpdated(android.accounts.Account[] accounts) { - Utility.runAsync(new Runnable() { - public void run() { - MailService.reconcilePopImapAccounts(Welcome.this); - } - }); - } } /** @@ -249,6 +222,9 @@ public class Welcome extends Activity { @Override protected Void doInBackground(Void... params) { + // Reconcile POP/IMAP accounts. EAS accounts are taken care of by ExchangeService. + MailService.reconcilePopImapAccountsSync(mFromActivity); + final int numAccount = EmailContent.count(mFromActivity, EmailContent.Account.CONTENT_URI); if (numAccount == 0) { diff --git a/src/com/android/email/service/EmailBroadcastProcessorService.java b/src/com/android/email/service/EmailBroadcastProcessorService.java index 3710ee3b3..d6c086c1c 100644 --- a/src/com/android/email/service/EmailBroadcastProcessorService.java +++ b/src/com/android/email/service/EmailBroadcastProcessorService.java @@ -23,6 +23,7 @@ import com.android.email.SecurityPolicy; import com.android.email.VendorPolicyLoader; import com.android.email.activity.setup.AccountSettingsXL; +import android.accounts.AccountManager; import android.app.IntentService; import android.content.ComponentName; import android.content.Context; @@ -108,6 +109,8 @@ public class EmailBroadcastProcessorService extends IntentService { } else if (ACTION_SECRET_CODE.equals(broadcastAction) && SECRET_CODE_HOST_DEBUG_SCREEN.equals(broadcastIntent.getData().getHost())) { AccountSettingsXL.actionSettingsWithDebug(this); + } else if (AccountManager.LOGIN_ACCOUNTS_CHANGED_ACTION.equals(broadcastAction)) { + onSystemAccountChanged(); } } else if (ACTION_DEVICE_POLICY_ADMIN.equals(action)) { int message = intent.getIntExtra(EXTRA_DEVICE_POLICY_ADMIN, -1); @@ -169,4 +172,13 @@ public class EmailBroadcastProcessorService extends IntentService { : PackageManager.COMPONENT_ENABLED_STATE_DISABLED, PackageManager.DONT_KILL_APP); } + + private void onSystemAccountChanged() { + Log.i(Email.LOG_TAG, "System accouns updated."); + MailService.reconcilePopImapAccountsSync(this); + + // Let ExchangeService reconcile EAS accouts. + // The service will stops itself it there's no EAS accounts. + ExchangeUtils.startExchangeService(this); + } } diff --git a/src/com/android/email/service/MailService.java b/src/com/android/email/service/MailService.java index cbf0becb3..594d9181b 100644 --- a/src/com/android/email/service/MailService.java +++ b/src/com/android/email/service/MailService.java @@ -21,6 +21,7 @@ import com.android.email.Controller; import com.android.email.Email; import com.android.email.NotificationController; import com.android.email.SecurityPolicy; +import com.android.email.SingleRunningTask; import com.android.email.Utility; import com.android.email.mail.MessagingException; import com.android.email.provider.EmailContent; @@ -29,12 +30,12 @@ import com.android.email.provider.EmailContent.AccountColumns; import com.android.email.provider.EmailContent.HostAuth; import com.android.email.provider.EmailContent.Mailbox; import com.android.email.provider.EmailProvider; +import com.android.exchange.ExchangeService; import android.accounts.AccountManager; import android.accounts.AccountManagerCallback; import android.accounts.AccountManagerFuture; import android.accounts.AuthenticatorException; -import android.accounts.OnAccountsUpdateListener; import android.accounts.OperationCanceledException; import android.app.AlarmManager; import android.app.PendingIntent; @@ -47,7 +48,6 @@ import android.content.SyncStatusObserver; import android.database.Cursor; import android.net.ConnectivityManager; import android.net.Uri; -import android.os.AsyncTask; import android.os.Bundle; import android.os.Handler; import android.os.IBinder; @@ -59,7 +59,6 @@ import java.io.IOException; import java.util.ArrayList; import java.util.HashMap; import java.util.List; -import java.util.concurrent.atomic.AtomicInteger; /** * Background service for refreshing non-push email accounts. @@ -98,17 +97,12 @@ public class MailService extends Service { private static final String[] NEW_MESSAGE_COUNT_PROJECTION = new String[] {AccountColumns.NEW_MESSAGE_COUNT}; - // Keep track of the number of times we're calling the reconciler - // If the count is > 1, don't try to run another one (it means that one is running and one is - // already queued). - private static AtomicInteger sReconcilerCount = new AtomicInteger(0); private static MailService sMailService; /*package*/ Controller mController; private final Controller.Result mControllerCallback = new ControllerResults(); private ContentResolver mContentResolver; private Context mContext; - /*package*/ AccountsUpdatedListener mAccountsUpdatedListener; private Handler mHandler = new Handler(); private int mStartId; @@ -206,12 +200,12 @@ public class MailService extends Service { // Restore accounts, if it has not happened already AccountBackupRestore.restoreAccountsIfNeeded(this); - // Set up our observer for AccountManager - mAccountsUpdatedListener = new AccountsUpdatedListener(); - AccountManager.get(getApplication()).addOnAccountsUpdatedListener( - mAccountsUpdatedListener, mHandler, true); - // Run reconciliation to make sure we're up-to-date on account status - mAccountsUpdatedListener.onAccountsUpdated(null); + Utility.runAsync(new Runnable() { + @Override + public void run() { + reconcilePopImapAccountsSync(MailService.this); + } + }); // TODO this needs to be passed through the controller and back to us mStartId = startId; @@ -358,10 +352,6 @@ public class MailService extends Service { public void onDestroy() { super.onDestroy(); Controller.getInstance(getApplication()).removeResultCallback(mControllerCallback); - // Unregister our account listener - if (mAccountsUpdatedListener != null) { - AccountManager.get(this).removeOnAccountsUpdatedListener(mAccountsUpdatedListener); - } } private void cancel() { @@ -809,46 +799,24 @@ public class MailService extends Service { return providerAccounts; } - /** - * We synchronize this, since it can be called from multiple threads - */ - public static synchronized void reconcilePopImapAccounts(Context context) { - android.accounts.Account[] accountManagerAccounts = AccountManager.get(context) - .getAccountsByType(Email.POP_IMAP_ACCOUNT_MANAGER_TYPE); - ArrayList providerAccounts = getPopImapAccountList(context); - MailService.reconcileAccountsWithAccountManager(context, providerAccounts, - accountManagerAccounts, false, context.getContentResolver()); + private static final SingleRunningTask sReconcilePopImapAccountsSyncExecutor = + new SingleRunningTask("ReconcilePopImapAccountsSync") { + @Override + protected void runInternal(Context context) { + android.accounts.Account[] accountManagerAccounts = AccountManager.get(context) + .getAccountsByType(Email.POP_IMAP_ACCOUNT_MANAGER_TYPE); + ArrayList providerAccounts = getPopImapAccountList(context); + MailService.reconcileAccountsWithAccountManager(context, providerAccounts, + accountManagerAccounts, false, context.getContentResolver()); - } - - /** - * Reconcile accounts when accounts are added/removed from AccountManager; note that the - * required argument is ignored (we request those of our specific type within the method) - */ - public class AccountsUpdatedListener implements OnAccountsUpdateListener { - - public void onAccountsUpdated(android.accounts.Account[] accounts) { - // Only allow one to be queued at a time (and one running) - if (sReconcilerCount.getAndIncrement() > 1) { - sReconcilerCount.decrementAndGet(); - return; - } - new AccountReconcilerTask().execute(); - } - - private class AccountReconcilerTask extends AsyncTask { - @Override - protected Void doInBackground(Void... params) { - try { - reconcilePopImapAccounts(MailService.this); - } finally { - // Belt & suspenders; most likely, any RuntimeException in this code would - // kill the Email process - sReconcilerCount.decrementAndGet(); } - return null; - } - } + }; + + /** + * Reconcile POP/IMAP accounts. + */ + public static void reconcilePopImapAccountsSync(Context context) { + sReconcilePopImapAccountsSyncExecutor.run(context); } /** diff --git a/src/com/android/exchange/ExchangeService.java b/src/com/android/exchange/ExchangeService.java index e9e2af7e4..12768ffb5 100644 --- a/src/com/android/exchange/ExchangeService.java +++ b/src/com/android/exchange/ExchangeService.java @@ -51,7 +51,6 @@ import org.apache.http.params.BasicHttpParams; import org.apache.http.params.HttpParams; import android.accounts.AccountManager; -import android.accounts.OnAccountsUpdateListener; import android.app.AlarmManager; import android.app.PendingIntent; import android.app.Service; @@ -210,7 +209,6 @@ public class ExchangeService extends Service implements Runnable { private SyncedMessageObserver mSyncedMessageObserver; private EasSyncStatusObserver mSyncStatusObserver; private Object mStatusChangeListener; - private EasAccountsUpdatedListener mAccountsUpdatedListener; private HashMap mCalendarObservers = new HashMap(); @@ -1044,25 +1042,6 @@ public class ExchangeService extends Service implements Runnable { } } - /** - * The reconciler (which is called from this listener) can make blocking calls back into - * the account manager. So, in this callback we spin up a worker thread to call the - * reconciler. - */ - public class EasAccountsUpdatedListener implements OnAccountsUpdateListener { - public void onAccountsUpdated(android.accounts.Account[] accounts) { - final ExchangeService exchangeService = INSTANCE; - if (exchangeService != null) { - Utility.runAsync(new Runnable() { - @Override - public void run() { - exchangeService.runAccountReconcilerSync(exchangeService); - } - }); - } - } - } - /** * Blocking call to the account reconciler */ @@ -1872,11 +1851,6 @@ public class ExchangeService extends Service implements Runnable { ContentResolver.addStatusChangeListener( ContentResolver.SYNC_OBSERVER_TYPE_SETTINGS, mSyncStatusObserver); - // Set up our observer for AccountManager - mAccountsUpdatedListener = new EasAccountsUpdatedListener(); - AccountManager.get(getApplication()).addOnAccountsUpdatedListener( - mAccountsUpdatedListener, mHandler, true); - // Set up receivers for connectivity and background data setting mConnectivityReceiver = new ConnectivityReceiver(); registerReceiver(mConnectivityReceiver, new IntentFilter( @@ -1973,13 +1947,6 @@ public class ExchangeService extends Service implements Runnable { } unregisterCalendarObservers(); - // Remove account listener (registered with AccountManager) - if (mAccountsUpdatedListener != null) { - AccountManager.get(this).removeOnAccountsUpdatedListener( - mAccountsUpdatedListener); - mAccountsUpdatedListener = null; - } - // Remove the sync status change listener (and null out the observer) if (mStatusChangeListener != null) { ContentResolver.removeStatusChangeListener(mStatusChangeListener); diff --git a/tests/src/com/android/email/SingleRunningTaskTest.java b/tests/src/com/android/email/SingleRunningTaskTest.java new file mode 100644 index 000000000..652c07a60 --- /dev/null +++ b/tests/src/com/android/email/SingleRunningTaskTest.java @@ -0,0 +1,150 @@ +/* + * Copyright (C) 2011 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.email; + +import com.android.email.TestUtils.Condition; + +import java.util.concurrent.atomic.AtomicInteger; + +import junit.framework.TestCase; + +public class SingleRunningTaskTest extends TestCase { + + private static class NormalTask extends SingleRunningTask { + // # of times the task has actually run. + public final AtomicInteger mCalledCount = new AtomicInteger(0); + + // The task will be blocked if true + private volatile boolean mBlocked = false; + + public NormalTask() { + super("task"); + } + + public void block() { + mBlocked = true; + } + + public void unblock() { + mBlocked = false; + synchronized (this) { + notify(); + } + } + + @Override + protected void runInternal(Void param) { + mCalledCount.incrementAndGet(); + while (mBlocked) { + synchronized (this) { + try { + wait(); + } catch (InterruptedException ignore) { + } + } + } + } + } + + // Always throws exception + private static class FailTask extends SingleRunningTask { + public FailTask() { + super("task"); + } + + @Override + protected void runInternal(Void param) { + throw new RuntimeException("Intentional exception"); + } + } + + /** + * Run 3 tasks sequentially. + */ + public void testSequential() { + final NormalTask e = new NormalTask(); + + e.run(null); + e.run(null); + e.run(null); + + assertEquals(3, e.mCalledCount.get()); + } + + /** + * Run 2 tasks in parallel, and then another call. + */ + public void testParallel() { + final NormalTask e = new NormalTask(); + + // Block the first task + e.block(); + + // The call will be blocked, so run it on another thread. + Utility.runAsync(new Runnable() { + @Override + public void run() { + e.run(null); + } + }); + + // Wait until the task really starts. + TestUtils.waitUntil(new Condition() { + @Override + public boolean isMet() { + return e.mCalledCount.get() >= 1; + } + }, 10); + + // Now the task is running, blocked. + + // This call will just be ignored. + e.run(null); + + assertEquals(1, e.mCalledCount.get()); + + // Let the thread finish. + e.unblock(); + + // Wait until the task really finishes. + TestUtils.waitUntil(new Condition() { + @Override + public boolean isMet() { + return !e.isRunningForTest(); + } + }, 10); + + // Now this should not be ignored. + e.run(null); + + assertEquals(2, e.mCalledCount.get()); + } + + /** + * If a task throws, isRunning should become false. + */ + public void testException() { + final FailTask e = new FailTask(); + + try { + e.run(null); + fail("Didn't throw exception"); + } catch (RuntimeException expected) { + } + assertFalse(e.isRunningForTest()); + } +} diff --git a/tests/src/com/android/email/service/MailServiceTests.java b/tests/src/com/android/email/service/MailServiceTests.java index a2ebf770f..a8eddc25a 100644 --- a/tests/src/com/android/email/service/MailServiceTests.java +++ b/tests/src/com/android/email/service/MailServiceTests.java @@ -100,24 +100,6 @@ public class MailServiceTests extends AccountTestCase { } } - /** - * Call onAccountsUpdated as fast as we can from a single thread (simulating them all coming - * from the UI thread); it shouldn't crash - */ - public void testThrashOnAccountsUpdated() { - final MailService mailService = new MailService(); - mailService.mController = new TestController(mMockContext, getContext()); - assertNotNull(mailService); - final android.accounts.Account[] accounts = getExchangeAccounts(); - Utility.runAsync(new Runnable() { - @Override - public void run() { - for (int i = 0; i < 1000; i++) { - mailService.mAccountsUpdatedListener.onAccountsUpdated(accounts); - } - }}); - } - /** * Note, there is some inherent risk in this test, as it creates *real* accounts in the * system (it cannot use the mock context with the Account Manager).