Reduce/eliminate UI thread access in MailService
* Make most calls to AccountBackupRestore return immediately w/o DB access * Move most workers in MailService into async runnables * Remove account restore / null check from ACTION_SEND_PENDING_EMAIL * Strengthened unit test on Mailbox.findMailboxOfType() because after removing the account check (above), sendPendingMessages depends on findMailboxOfType() returning -1 on a missing account. * Clean up a bunch of warnings (no longer use deprecated Config.LOGD) Bug: 3133763 (and probably others) Change-Id: Id39707bca7a8ebf5000f84d542013411ff0f422e
This commit is contained in:
parent
9f7e3982ad
commit
6ebaa90847
|
@ -36,6 +36,10 @@ import android.util.Log;
|
|||
*/
|
||||
public class AccountBackupRestore {
|
||||
|
||||
// This reduces the need for expensive checks to just the first time only/
|
||||
// synchronize on AccountBackupRestore.class
|
||||
private static boolean sBackupsChecked = false;
|
||||
|
||||
/**
|
||||
* Backup accounts. Can be called from UI thread (does work in a new thread)
|
||||
*/
|
||||
|
@ -56,7 +60,10 @@ public class AccountBackupRestore {
|
|||
* Restore accounts if needed. This is blocking, and should only be called in specific
|
||||
* startup/entry points.
|
||||
*/
|
||||
public static void restoreAccountsIfNeeded(final Context context) {
|
||||
public static synchronized void restoreAccountsIfNeeded(final Context context) {
|
||||
// Quick exit if possible
|
||||
if (sBackupsChecked) return;
|
||||
|
||||
// Don't log here; This is called often.
|
||||
boolean restored = doRestoreAccounts(context, Preferences.getPreferences(context), false);
|
||||
if (restored) {
|
||||
|
@ -68,6 +75,7 @@ public class AccountBackupRestore {
|
|||
Email.setServicesEnabled(context);
|
||||
ExchangeUtils.startExchangeService(context);
|
||||
}
|
||||
sBackupsChecked = true;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
@ -24,11 +24,11 @@ import com.android.email.SecurityPolicy;
|
|||
import com.android.email.Utility;
|
||||
import com.android.email.mail.MessagingException;
|
||||
import com.android.email.provider.EmailContent;
|
||||
import com.android.email.provider.EmailProvider;
|
||||
import com.android.email.provider.EmailContent.Account;
|
||||
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 android.accounts.AccountManager;
|
||||
import android.accounts.AccountManagerCallback;
|
||||
|
@ -52,7 +52,6 @@ import android.os.Bundle;
|
|||
import android.os.Handler;
|
||||
import android.os.IBinder;
|
||||
import android.os.SystemClock;
|
||||
import android.util.Config;
|
||||
import android.util.Log;
|
||||
|
||||
import java.io.IOException;
|
||||
|
@ -63,6 +62,9 @@ import java.util.concurrent.atomic.AtomicInteger;
|
|||
|
||||
/**
|
||||
* Background service for refreshing non-push email accounts.
|
||||
*
|
||||
* TODO: Convert to IntentService to move *all* work off the UI thread, serialize work, and avoid
|
||||
* possible problems with out-of-order startId processing.
|
||||
*/
|
||||
public class MailService extends Service {
|
||||
/** DO NOT CHECK IN "TRUE" */
|
||||
|
@ -181,7 +183,6 @@ public class MailService extends Service {
|
|||
*
|
||||
* @param context a context
|
||||
* @param accountId the id of the account that is reporting new messages
|
||||
* @param newCount the number of new messages
|
||||
*/
|
||||
public static void actionNotifyNewMessages(Context context, long accountId) {
|
||||
Intent i = new Intent(ACTION_NOTIFY_MAIL);
|
||||
|
@ -195,7 +196,7 @@ public class MailService extends Service {
|
|||
}
|
||||
|
||||
@Override
|
||||
public int onStartCommand(Intent intent, int flags, int startId) {
|
||||
public int onStartCommand(final Intent intent, int flags, final int startId) {
|
||||
super.onStartCommand(intent, flags, startId);
|
||||
|
||||
// Save the service away (for unit tests)
|
||||
|
@ -212,38 +213,43 @@ public class MailService extends Service {
|
|||
mAccountsUpdatedListener.onAccountsUpdated(null);
|
||||
|
||||
// TODO this needs to be passed through the controller and back to us
|
||||
this.mStartId = startId;
|
||||
mStartId = startId;
|
||||
String action = intent.getAction();
|
||||
final long accountId = intent.getLongExtra(EXTRA_ACCOUNT, -1);
|
||||
|
||||
mController = Controller.getInstance(this);
|
||||
mController.addResultCallback(mControllerCallback);
|
||||
mContentResolver = getContentResolver();
|
||||
mContext = this;
|
||||
|
||||
final AlarmManager alarmManager = (AlarmManager) getSystemService(Context.ALARM_SERVICE);
|
||||
|
||||
if (ACTION_CHECK_MAIL.equals(action)) {
|
||||
// DB access required to satisfy this intent, so offload from UI thread
|
||||
Utility.runAsync(new Runnable() {
|
||||
@Override
|
||||
public void run() {
|
||||
// If we have the data, restore the last-sync-times for each account
|
||||
// These are cached in the wakeup intent in case the process was killed.
|
||||
restoreSyncReports(intent);
|
||||
|
||||
// Sync a specific account if given
|
||||
AlarmManager alarmManager = (AlarmManager)getSystemService(Context.ALARM_SERVICE);
|
||||
long checkAccountId = intent.getLongExtra(EXTRA_ACCOUNT, -1);
|
||||
if (Config.LOGD && Email.DEBUG) {
|
||||
Log.d(LOG_TAG, "action: check mail for id=" + checkAccountId);
|
||||
if (Email.DEBUG) {
|
||||
Log.d(LOG_TAG, "action: check mail for id=" + accountId);
|
||||
}
|
||||
if (checkAccountId >= 0) {
|
||||
setWatchdog(checkAccountId, alarmManager);
|
||||
if (accountId >= 0) {
|
||||
setWatchdog(accountId, alarmManager);
|
||||
}
|
||||
|
||||
// Start sync if account is given and background data is enabled and the account itself
|
||||
// has sync enabled
|
||||
// Start sync if account is given && bg data enabled && account has sync enabled
|
||||
boolean syncStarted = false;
|
||||
if (checkAccountId != -1 && isBackgroundDataEnabled()) {
|
||||
if (accountId != -1 && isBackgroundDataEnabled()) {
|
||||
synchronized(mSyncReports) {
|
||||
for (AccountSyncReport report: mSyncReports.values()) {
|
||||
if (report.accountId == checkAccountId) {
|
||||
if (report.accountId == accountId) {
|
||||
if (report.syncEnabled) {
|
||||
syncStarted = syncOneAccount(mController, checkAccountId, startId);
|
||||
syncStarted = syncOneAccount(mController, accountId,
|
||||
startId);
|
||||
}
|
||||
break;
|
||||
}
|
||||
|
@ -254,56 +260,64 @@ public class MailService extends Service {
|
|||
// Reschedule if we didn't start sync.
|
||||
if (!syncStarted) {
|
||||
// Prevent runaway on the current account by pretending it updated
|
||||
if (checkAccountId != -1) {
|
||||
updateAccountReport(checkAccountId, 0);
|
||||
if (accountId != -1) {
|
||||
updateAccountReport(accountId, 0);
|
||||
}
|
||||
// Find next account to sync, and reschedule
|
||||
reschedule(alarmManager);
|
||||
// Stop the service, unless actually syncing (which will stop the service)
|
||||
stopSelf(startId);
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
else if (ACTION_CANCEL.equals(action)) {
|
||||
if (Config.LOGD && Email.DEBUG) {
|
||||
if (Email.DEBUG) {
|
||||
Log.d(LOG_TAG, "action: cancel");
|
||||
}
|
||||
cancel();
|
||||
stopSelf(startId);
|
||||
}
|
||||
else if (ACTION_SEND_PENDING_MAIL.equals(action)) {
|
||||
if (Config.LOGD && Email.DEBUG) {
|
||||
if (Email.DEBUG) {
|
||||
Log.d(LOG_TAG, "action: send pending mail");
|
||||
}
|
||||
final long accountId = intent.getLongExtra(EXTRA_ACCOUNT, -1);
|
||||
EmailContent.Account account =
|
||||
EmailContent.Account.restoreAccountWithId(this, accountId);
|
||||
if (account != null) {
|
||||
Utility.runAsync(new Runnable() {
|
||||
public void run() {
|
||||
mController.sendPendingMessages(accountId);
|
||||
}
|
||||
});
|
||||
}
|
||||
stopSelf(startId);
|
||||
}
|
||||
else if (ACTION_RESCHEDULE.equals(action)) {
|
||||
if (Config.LOGD && Email.DEBUG) {
|
||||
if (Email.DEBUG) {
|
||||
Log.d(LOG_TAG, "action: reschedule");
|
||||
}
|
||||
final NotificationController nc = NotificationController.getInstance(this);
|
||||
// DB access required to satisfy this intent, so offload from UI thread
|
||||
Utility.runAsync(new Runnable() {
|
||||
@Override
|
||||
public void run() {
|
||||
// Clear all notifications, in case account list has changed.
|
||||
//
|
||||
// TODO Clear notifications for non-existing accounts. Now that we have separate
|
||||
// notification for each account, NotificationController should be able to do that.
|
||||
NotificationController.getInstance(this).cancelNewMessageNotification(-1);
|
||||
// TODO Clear notifications for non-existing accounts. Now that we have
|
||||
// separate notifications for each account, NotificationController should be
|
||||
// able to do that.
|
||||
nc.cancelNewMessageNotification(-1);
|
||||
|
||||
// When called externally, we refresh the sync reports table to pick up
|
||||
// any changes in the account list or account settings
|
||||
refreshSyncReports();
|
||||
// Finally, scan for the next needing update, and set an alarm for it
|
||||
AlarmManager alarmManager = (AlarmManager)getSystemService(Context.ALARM_SERVICE);
|
||||
reschedule(alarmManager);
|
||||
stopSelf(startId);
|
||||
}
|
||||
});
|
||||
} else if (ACTION_NOTIFY_MAIL.equals(action)) {
|
||||
long accountId = intent.getLongExtra(EXTRA_ACCOUNT, -1);
|
||||
// DB access required to satisfy this intent, so offload from UI thread
|
||||
Utility.runAsync(new Runnable() {
|
||||
@Override
|
||||
public void run() {
|
||||
// Get the current new message count
|
||||
Cursor c = mContentResolver.query(
|
||||
ContentUris.withAppendedId(Account.CONTENT_URI, accountId),
|
||||
|
@ -312,23 +326,20 @@ public class MailService extends Service {
|
|||
try {
|
||||
if (c.moveToFirst()) {
|
||||
newMessageCount = c.getInt(0);
|
||||
} else {
|
||||
// If the account no longer exists, set to -1 (which is handled below)
|
||||
accountId = -1;
|
||||
updateAccountReport(accountId, newMessageCount);
|
||||
notifyNewMessages(accountId);
|
||||
}
|
||||
} finally {
|
||||
c.close();
|
||||
}
|
||||
if (Config.LOGD && Email.DEBUG) {
|
||||
if (Email.DEBUG) {
|
||||
Log.d(LOG_TAG, "notify accountId=" + Long.toString(accountId)
|
||||
+ " count=" + newMessageCount);
|
||||
}
|
||||
if (accountId != -1) {
|
||||
updateAccountReport(accountId, newMessageCount);
|
||||
notifyNewMessages(accountId);
|
||||
}
|
||||
stopSelf(startId);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
// Returning START_NOT_STICKY means that if a mail check is killed (e.g. due to memory
|
||||
// pressure, there will be no explicit restart. This is OK; Note that we set a watchdog
|
||||
|
@ -434,12 +445,12 @@ public class MailService extends Service {
|
|||
|
||||
if (nextAccount == null) {
|
||||
alarmMgr.cancel(pi);
|
||||
if (Config.LOGD && Email.DEBUG) {
|
||||
if (Email.DEBUG) {
|
||||
Log.d(LOG_TAG, "reschedule: alarm cancel - no account to check");
|
||||
}
|
||||
} else {
|
||||
alarmMgr.set(AlarmManager.ELAPSED_REALTIME_WAKEUP, nextCheckTime, pi);
|
||||
if (Config.LOGD && Email.DEBUG) {
|
||||
if (Email.DEBUG) {
|
||||
Log.d(LOG_TAG, "reschedule: alarm set at " + nextCheckTime
|
||||
+ " for " + nextAccount);
|
||||
}
|
||||
|
@ -646,7 +657,7 @@ public class MailService extends Service {
|
|||
if (newCount != -1) {
|
||||
report.unseenMessageCount = newCount;
|
||||
}
|
||||
if (Config.LOGD && Email.DEBUG) {
|
||||
if (Email.DEBUG) {
|
||||
Log.d(LOG_TAG, "update account " + report.toString());
|
||||
}
|
||||
return report;
|
||||
|
|
|
@ -1803,6 +1803,22 @@ public class ProviderTests extends ProviderTestCase2<EmailProvider> {
|
|||
Mailbox.findMailboxOfType(context, acct1.mId, Mailbox.TYPE_CONTACTS));
|
||||
assertEquals(acct2Contacts.mId,
|
||||
Mailbox.findMailboxOfType(context, acct2.mId, Mailbox.TYPE_CONTACTS));
|
||||
|
||||
// Check that nonexistent mailboxes are not returned
|
||||
assertEquals(Mailbox.NO_MAILBOX,
|
||||
Mailbox.findMailboxOfType(context, acct1.mId, Mailbox.TYPE_DRAFTS));
|
||||
assertEquals(Mailbox.NO_MAILBOX,
|
||||
Mailbox.findMailboxOfType(context, acct1.mId, Mailbox.TYPE_OUTBOX));
|
||||
|
||||
// delete account 1 and confirm no mailboxes are returned
|
||||
context.getContentResolver().delete(
|
||||
ContentUris.withAppendedId(Account.CONTENT_URI, acct1.mId), null, null);
|
||||
assertEquals(Mailbox.NO_MAILBOX,
|
||||
Mailbox.findMailboxOfType(context, acct1.mId, Mailbox.TYPE_INBOX));
|
||||
assertEquals(Mailbox.NO_MAILBOX,
|
||||
Mailbox.findMailboxOfType(context, acct1.mId, Mailbox.TYPE_CALENDAR));
|
||||
assertEquals(Mailbox.NO_MAILBOX,
|
||||
Mailbox.findMailboxOfType(context, acct1.mId, Mailbox.TYPE_CONTACTS));
|
||||
}
|
||||
|
||||
public void testRestoreMailboxOfType() {
|
||||
|
|
Loading…
Reference in New Issue