diff --git a/src/com/android/email/service/AttachmentService.java b/src/com/android/email/service/AttachmentService.java index 745a86e57..663bbe1ef 100644 --- a/src/com/android/email/service/AttachmentService.java +++ b/src/com/android/email/service/AttachmentService.java @@ -56,10 +56,8 @@ import java.io.PrintWriter; import java.util.Collection; import java.util.Comparator; import java.util.HashMap; -import java.util.Iterator; import java.util.PriorityQueue; import java.util.Queue; -import java.util.TreeSet; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentLinkedQueue; @@ -81,15 +79,15 @@ public class AttachmentService extends Service implements Runnable { // Try to download an attachment in the background this many times before giving up private static final int MAX_DOWNLOAD_RETRIES = 5; - /* package */ static final int PRIORITY_NONE = -1; + static final int PRIORITY_NONE = -1; // High priority is for user requests - /* package */ static final int PRIORITY_FOREGROUND = 0; - /* package */ static final int PRIORITY_HIGHEST = PRIORITY_FOREGROUND; + static final int PRIORITY_FOREGROUND = 0; + static final int PRIORITY_HIGHEST = PRIORITY_FOREGROUND; // Normal priority is for forwarded downloads in outgoing mail - /* package */ static final int PRIORITY_SEND_MAIL = 1; + static final int PRIORITY_SEND_MAIL = 1; // Low priority will be used for opportunistic downloads - /* package */ static final int PRIORITY_BACKGROUND = 2; - /* package */ static final int PRIORITY_LOWEST = PRIORITY_BACKGROUND; + static final int PRIORITY_BACKGROUND = 2; + static final int PRIORITY_LOWEST = PRIORITY_BACKGROUND; // Minimum free storage in order to perform prefetch (25% of total memory) private static final float PREFETCH_MINIMUM_STORAGE_AVAILABLE = 0.25F; @@ -106,20 +104,20 @@ public class AttachmentService extends Service implements Runnable { private static final String EXTRA_ATTACHMENT = "com.android.email.AttachmentService.attachment"; - // This callback is invoked by the various service backends to give us download progress + // This callback is invoked by the various service implementations to give us download progress // since those modules are responsible for the actual download. private final ServiceCallback mServiceCallback = new ServiceCallback(); // sRunningService is only set in the UI thread; it's visibility elsewhere is guaranteed // by the use of "volatile" - /*package*/ static volatile AttachmentService sRunningService = null; + static volatile AttachmentService sRunningService = null; // Signify that we are being shut down & destroyed. private volatile boolean mStop = false; - /*package*/ Context mContext; - /*package*/ EmailConnectivityManager mConnectivityManager; - /*package*/ final AttachmentWatchdog mWatchdog = new AttachmentWatchdog(); + Context mContext; + EmailConnectivityManager mConnectivityManager; + final AttachmentWatchdog mWatchdog = new AttachmentWatchdog(); private final Object mLock = new Object(); @@ -127,22 +125,18 @@ public class AttachmentService extends Service implements Runnable { // NOTE: This map is not kept current in terms of deletions (i.e. it stores the last calculated // amount plus the size of any new attachments loaded). If and when we reach the per-account // limit, we recalculate the actual usage - /*package*/ final ConcurrentHashMap mAttachmentStorageMap = - new ConcurrentHashMap(); + final ConcurrentHashMap mAttachmentStorageMap = new ConcurrentHashMap(); // A map of attachment ids to the number of failed attempts to download the attachment // NOTE: We do not want to persist this. This allows us to retry background downloading // if any transient network errors are fixed & and the app is restarted - /* package */ final ConcurrentHashMap mAttachmentFailureMap = - new ConcurrentHashMap(); + final ConcurrentHashMap mAttachmentFailureMap = new ConcurrentHashMap(); // Keeps tracks of downloads in progress based on an attachment ID to DownloadRequest mapping. - /*package*/ final ConcurrentHashMap mDownloadsInProgress = + final ConcurrentHashMap mDownloadsInProgress = new ConcurrentHashMap(); - // TODO: Remove in favor of the DownloadQueue when we are ready to move over (soon). - /*package*/ final DownloadSet mDownloadSet = new DownloadSet(new DownloadComparator()); - /*package*/ final DownloadQueue mDownloadQueue = new DownloadQueue(); + final DownloadQueue mDownloadQueue = new DownloadQueue(); /** * This class is used to contain the details and state of a particular request to download @@ -150,8 +144,7 @@ public class AttachmentService extends Service implements Runnable { * or in the in-progress map used to keep track of downloads that are currently happening * in the system */ -// public static class DownloadRequest { - /*package*/ static class DownloadRequest { + static class DownloadRequest { // Details of the request. final int mPriority; final long mTime; @@ -227,29 +220,6 @@ public class AttachmentService extends Service implements Runnable { } } - /** - * Comparator class for the download set; we first compare by priority. Requests with equal - * priority are compared by the time the request was created (older requests come first) - * TODO: Move this into the DownloadQueue as a private static class when we finally remove - * the DownloadSet class from the implementation. - */ - private static class DownloadComparator implements Comparator { - @Override - public int compare(DownloadRequest req1, DownloadRequest req2) { - int res; - if (req1.mPriority != req2.mPriority) { - res = (req1.mPriority < req2.mPriority) ? -1 : 1; - } else { - if (req1.mTime == req2.mTime) { - res = 0; - } else { - res = (req1.mTime < req2.mTime) ? -1 : 1; - } - } - return res; - } - } - /** * This class is used to organize the various download requests that are pending. * We need a class that allows us to prioritize a collection of {@link DownloadRequest} objects @@ -259,20 +229,40 @@ public class AttachmentService extends Service implements Runnable { * as we can avoid pesky ConcurrentModificationException when one thread has the iterator * and another thread modifies the collection. */ - /*package*/ static class DownloadQueue { + static class DownloadQueue { private final int DEFAULT_SIZE = 10; // For synchronization private final Object mLock = new Object(); + /** + * Comparator class for the download set; we first compare by priority. Requests with equal + * priority are compared by the time the request was created (older requests come first) + */ + private static class DownloadComparator implements Comparator { + @Override + public int compare(DownloadRequest req1, DownloadRequest req2) { + int res; + if (req1.mPriority != req2.mPriority) { + res = (req1.mPriority < req2.mPriority) ? -1 : 1; + } else { + if (req1.mTime == req2.mTime) { + res = 0; + } else { + res = (req1.mTime < req2.mTime) ? -1 : 1; + } + } + return res; + } + } + // For prioritization of DownloadRequests. - /*package*/ final PriorityQueue mRequestQueue = + final PriorityQueue mRequestQueue = new PriorityQueue(DEFAULT_SIZE, new DownloadComparator()); // Secondary collection to quickly find objects w/o the help of an iterator. // This class should be kept in lock step with the priority queue. - /*package*/ final ConcurrentHashMap mRequestMap = - new ConcurrentHashMap(); + final ConcurrentHashMap mRequestMap = new ConcurrentHashMap(); /** * This function will add the request to our collections if it does not already @@ -280,7 +270,7 @@ public class AttachmentService extends Service implements Runnable { * @param request The {@link DownloadRequest} that should be added to our queue * @return true if it was added (or already exists), false otherwise */ - public synchronized boolean addRequest(final DownloadRequest request) + public boolean addRequest(final DownloadRequest request) throws NullPointerException { // It is key to keep the map and queue in lock step if (request == null) { @@ -312,7 +302,7 @@ public class AttachmentService extends Service implements Runnable { * @return true if it was removed or the request was invalid (meaning that the request * is not in our queue), false otherwise. */ - public synchronized boolean removeRequest(final DownloadRequest request) { + public boolean removeRequest(final DownloadRequest request) { if (request == null) { // If it is invalid, its not in the queue. return true; @@ -332,7 +322,7 @@ public class AttachmentService extends Service implements Runnable { * Return the next request from our queue. * @return The next {@link DownloadRequest} object or null if the queue is empty */ - public synchronized DownloadRequest getNextRequest() { + public DownloadRequest getNextRequest() { // It is key to keep the map and queue in lock step final DownloadRequest returnRequest; synchronized (mLock) { @@ -354,15 +344,21 @@ public class AttachmentService extends Service implements Runnable { if (requestId < 0) { return null; } - return mRequestMap.get(requestId); + synchronized (mLock) { + return mRequestMap.get(requestId); + } } public int getSize() { - return mRequestMap.size(); + synchronized (mLock) { + return mRequestMap.size(); + } } public boolean isEmpty() { - return mRequestMap.isEmpty(); + synchronized (mLock) { + return mRequestMap.isEmpty(); + } } } @@ -420,7 +416,7 @@ public class AttachmentService extends Service implements Runnable { * Watchdog for downloads; we use this in case we are hanging on a download, which might * have failed silently (the connection dropped, for example) */ - /*package*/ void watchdogAlarm(final AttachmentService service, final int callbackTimeout) { + void watchdogAlarm(final AttachmentService service, final int callbackTimeout) { final long now = System.currentTimeMillis(); // We want to iterate on each of the downloads that are currently in progress and // cancel the ones that seem to be taking too long. @@ -433,6 +429,7 @@ public class AttachmentService extends Service implements Runnable { LogUtils.d(LOG_TAG, "== Download of " + req.mAttachmentId + " timed out"); } service.cancelDownload(req); + // TODO: Should we also mark the attachment as failed at this point in time? } } // Check whether we can start new downloads... @@ -448,486 +445,444 @@ public class AttachmentService extends Service implements Runnable { } } - /** - * Temporary function implemented as a transition between DownloadSet to DownloadQueue. - * Will be property implemented and documented in a subsequent CL. - * @param req The {@link DownloadRequest} to be cancelled. - */ - /*package*/ void cancelDownload(final DownloadRequest req) { - mDownloadSet.cancelDownload(req); - return; - } - - /** - * Temporary function implemented as a transition between DownloadSet to DownloadQueue - */ - /*package*/ void processQueue() { - mDownloadSet.processQueue(); - return; - } - - /*package*/ boolean isConnected() { + boolean isConnected() { if (mConnectivityManager != null) { return mConnectivityManager.hasConnectivity(); } return false; } - /*package*/ Collection getInProgressDownloads() { + Collection getInProgressDownloads() { return mDownloadsInProgress.values(); } - /*package*/ boolean areDownloadsInProgress() { + boolean areDownloadsInProgress() { return !mDownloadsInProgress.isEmpty(); } /** - * The DownloadSet is a TreeSet sorted by priority class (e.g. low, high, etc.) and the - * time of the request. Higher priority requests - * are always processed first; among equals, the oldest request is processed first. The - * priority key represents this ordering. Note: All methods that change the attachment map are - * synchronized on the map itself + * Set the bits in the provider to mark this download as failed. + * @param att The attachment that failed to download. */ - /*package*/ class DownloadSet extends TreeSet { - private static final long serialVersionUID = 1L; + void markAttachmentAsFailed(final Attachment att) { + final ContentValues cv = new ContentValues(); + final int flags = Attachment.FLAG_DOWNLOAD_FORWARD | Attachment.FLAG_DOWNLOAD_USER_REQUEST; + cv.put(AttachmentColumns.FLAGS, att.mFlags &= ~flags); + cv.put(AttachmentColumns.UI_STATE, AttachmentState.FAILED); + att.update(mContext, cv); + } - /*package*/ DownloadSet(Comparator comparator) { - super(comparator); - } - - private void markAttachmentAsFailed(final Attachment att) { - final ContentValues cv = new ContentValues(); - final int flags = Attachment.FLAG_DOWNLOAD_FORWARD | Attachment.FLAG_DOWNLOAD_USER_REQUEST; - cv.put(AttachmentColumns.FLAGS, att.mFlags &= ~flags); - cv.put(AttachmentColumns.UI_STATE, AttachmentState.FAILED); - att.update(mContext, cv); - } - - /** - * onChange is called by the AttachmentReceiver upon receipt of a valid notification from - * EmailProvider that an attachment has been inserted or modified. It's not strictly - * necessary that we detect a deleted attachment, as the code always checks for the - * existence of an attachment before acting on it. - */ - public synchronized void onChange(Context context, Attachment att) { - DownloadRequest req = findDownloadRequest(att.mId); - long priority = getPriority(att); - if (priority == PRIORITY_NONE) { - if (LogUtils.isLoggable(LOG_TAG, LogUtils.DEBUG)) { - LogUtils.d(LOG_TAG, "== Attachment changed: " + att.mId); - } - // In this case, there is no download priority for this attachment - if (req != null) { - // If it exists in the map, remove it - // NOTE: We don't yet support deleting downloads in progress - if (LogUtils.isLoggable(LOG_TAG, LogUtils.DEBUG)) { - LogUtils.d(LOG_TAG, "== Attachment " + att.mId + " was in queue, removing"); - } - remove(req); - } - } else { - // Ignore changes that occur during download - if (mDownloadsInProgress.containsKey(att.mId)) return; - // If this is new, add the request to the queue - if (req == null) { - req = new DownloadRequest(context, att); - final AttachmentInfo attachInfo = new AttachmentInfo(context, att); - if (!attachInfo.isEligibleForDownload()) { - // We can't download this file due to policy, depending on what type - // of request we received, we handle the response differently. - if (((att.mFlags & Attachment.FLAG_DOWNLOAD_USER_REQUEST) != 0) || - ((att.mFlags & Attachment.FLAG_POLICY_DISALLOWS_DOWNLOAD) != 0)) { - // There are a couple of situations where we will not even allow this - // request to go in the queue because we can already process it as a - // failure. - // 1. The user explictly wants to download this attachment from the - // email view but they should not be able to...either because there is - // no app to view it or because its been marked as a policy violation. - // 2. The user is forwarding an email and the attachment has been - // marked as a policy violation. If the attachment is non viewable - // that is OK for forwarding a message so we'll let it pass through - markAttachmentAsFailed(att); - return; - } - // If we get this far it a forward of an attachment that is only - // ineligible because we can't view it or process it. Not because we - // can't download it for policy reasons. Let's let this go through because - // the final recipient of this forward email might be able to process it. - } - add(req); - } - // If the request already existed, we'll update the priority (so that the time is - // up-to-date); otherwise, we create a new request - if (LogUtils.isLoggable(LOG_TAG, LogUtils.DEBUG)) { - LogUtils.d(LOG_TAG, "== Download queued for attachment " + att.mId + ", class " + - req.mPriority + ", priority time " + req.mTime); - } - } - // Process the queue if we're in a wait - kick(); - } - - /** - * Find a queued DownloadRequest, given the attachment's id - * @param id the id of the attachment - * @return the DownloadRequest for that attachment (or null, if none) - */ - /*package*/ synchronized DownloadRequest findDownloadRequest(long id) { - Iterator iterator = iterator(); - while(iterator.hasNext()) { - DownloadRequest req = iterator.next(); - if (req.mAttachmentId == id) { - return req; - } - } - return null; - } - - @Override - public synchronized boolean isEmpty() { - return super.isEmpty() && mDownloadsInProgress.isEmpty(); - } - - /** - * Run through the AttachmentMap and find DownloadRequests that can be executed, enforcing - * the limit on maximum downloads - */ - /*package*/ synchronized void processQueue() { + /** + * onChange is called by the AttachmentReceiver upon receipt of a valid notification from + * EmailProvider that an attachment has been inserted or modified. It's not strictly + * necessary that we detect a deleted attachment, as the code always checks for the + * existence of an attachment before acting on it. + */ + public synchronized void onChange(final Context context, final Attachment att) { + DownloadRequest req = mDownloadQueue.findRequestById(att.mId); + final long priority = getPriority(att); + if (priority == PRIORITY_NONE) { if (LogUtils.isLoggable(LOG_TAG, LogUtils.DEBUG)) { - LogUtils.d(LOG_TAG, "== Checking attachment queue, " + mDownloadSet.size() - + " entries"); + LogUtils.d(LOG_TAG, "== Attachment changed: " + att.mId); } - Iterator iterator = mDownloadSet.descendingIterator(); - // First, start up any required downloads, in priority order - while (iterator.hasNext() && - (mDownloadsInProgress.size() < MAX_SIMULTANEOUS_DOWNLOADS)) { - DownloadRequest req = iterator.next(); - // Enforce per-account limit here - if (downloadsForAccount(req.mAccountId) >= MAX_SIMULTANEOUS_DOWNLOADS_PER_ACCOUNT) { - if (LogUtils.isLoggable(LOG_TAG, LogUtils.DEBUG)) { - LogUtils.d(LOG_TAG, "== Skip #" + req.mAttachmentId + "; maxed for acct #" + - req.mAccountId); - } - continue; - } else if (Attachment.restoreAttachmentWithId(mContext, req.mAttachmentId) == null) { - continue; - } - if (!req.mInProgress) { - final long currentTime = SystemClock.elapsedRealtime(); - if (req.mRetryCount > 0 && req.mRetryStartTime > currentTime) { - LogUtils.d(LOG_TAG, "== waiting to retry attachment %d", req.mAttachmentId); - mWatchdog.setWatchdogAlarm(mContext, CONNECTION_ERROR_RETRY_MILLIS, - CALLBACK_TIMEOUT); - continue; - } - // TODO: We try to gate ineligible downloads from entering the queue but its - // always possible that they made it in here regardless in the future. In a - // perfect world, we would make it bullet proof with a check for eligibility - // here instead/also. - mDownloadSet.tryStartDownload(req); - } - } - - // Don't prefetch if background downloading is disallowed - EmailConnectivityManager ecm = mConnectivityManager; - if (ecm == null) return; - if (!ecm.isAutoSyncAllowed()) return; - // Don't prefetch unless we're on a WiFi network - if (ecm.getActiveNetworkType() != ConnectivityManager.TYPE_WIFI) { - return; - } - // Then, try opportunistic download of appropriate attachments - int backgroundDownloads = MAX_SIMULTANEOUS_DOWNLOADS - mDownloadsInProgress.size(); - // Always leave one slot for user requested download - if (backgroundDownloads > (MAX_SIMULTANEOUS_DOWNLOADS - 1)) { - // We'll load up the newest 25 attachments that aren't loaded or queued - Uri lookupUri = EmailContent.uriWithLimit(Attachment.CONTENT_URI, - MAX_ATTACHMENTS_TO_CHECK); - Cursor c = mContext.getContentResolver().query(lookupUri, - Attachment.CONTENT_PROJECTION, - EmailContent.Attachment.PRECACHE_INBOX_SELECTION, - null, AttachmentColumns._ID + " DESC"); - File cacheDir = mContext.getCacheDir(); - try { - while (c.moveToNext()) { - Attachment att = new Attachment(); - att.restore(c); - Account account = Account.restoreAccountWithId(mContext, att.mAccountKey); - if (account == null) { - // Clean up this orphaned attachment; there's no point in keeping it - // around; then try to find another one - EmailContent.delete(mContext, Attachment.CONTENT_URI, att.mId); - } else { - // Check that the attachment meets system requirements for download - AttachmentInfo info = new AttachmentInfo(mContext, att); - if (info.isEligibleForDownload()) { - // Either the account must be able to prefetch or this must be - // an inline attachment - if (att.mContentId != null || - (canPrefetchForAccount(account, cacheDir))) { - Integer tryCount; - tryCount = mAttachmentFailureMap.get(att.mId); - if (tryCount != null && tryCount > MAX_DOWNLOAD_RETRIES) { - // move onto the next attachment - continue; - } - // Start this download and we're done - DownloadRequest req = new DownloadRequest(mContext, att); - mDownloadSet.tryStartDownload(req); - break; - } - } else { - // If this attachment was ineligible for download - // because of policy related issues, its flags would be set to - // FLAG_POLICY_DISALLOWS_DOWNLOAD and would not show up in the - // query results. We are most likely here for other reasons such - // as the inability to view the attachment. In that case, let's just - // skip it for now. - LogUtils.e(LOG_TAG, "== skip attachment %d, it is ineligible", att.mId); - } - } - } - } finally { - c.close(); - } - } - } - - /** - * Count the number of running downloads in progress for this account - * @param accountId the id of the account - * @return the count of running downloads - */ - /*package*/ synchronized int downloadsForAccount(long accountId) { - int count = 0; - for (DownloadRequest req: mDownloadsInProgress.values()) { - if (req.mAccountId == accountId) { - count++; - } - } - return count; - } - - /** - * Attempt to execute the DownloadRequest, enforcing the maximum downloads per account - * parameter - * @param req the DownloadRequest - * @return whether or not the download was started - */ - /*package*/ synchronized boolean tryStartDownload(DownloadRequest req) { - EmailServiceProxy service = EmailServiceUtils.getServiceForAccount( - AttachmentService.this, req.mAccountId); - - // Do not download the same attachment multiple times - boolean alreadyInProgress = mDownloadsInProgress.get(req.mAttachmentId) != null; - if (alreadyInProgress) return false; - - try { - if (LogUtils.isLoggable(LOG_TAG, LogUtils.DEBUG)) { - LogUtils.d(LOG_TAG, ">> Starting download for attachment #" + req.mAttachmentId); - } - startDownload(service, req); - } catch (RemoteException e) { - // TODO: Consider whether we need to do more in this case... - // For now, fix up our data to reflect the failure - cancelDownload(req); - } - return true; - } - - private synchronized DownloadRequest getDownloadInProgress(long attachmentId) { - return mDownloadsInProgress.get(attachmentId); - } - - - /** - * Do the work of starting an attachment download using the EmailService interface, and - * set our watchdog alarm - * - * @param service the service handling the download - * @param req the DownloadRequest - * @throws RemoteException - */ - private void startDownload(EmailServiceProxy service, DownloadRequest req) - throws RemoteException { - req.mStartTime = System.currentTimeMillis(); - req.mInProgress = true; - mDownloadsInProgress.put(req.mAttachmentId, req); - service.loadAttachment(mServiceCallback, req.mAccountId, req.mAttachmentId, - req.mPriority != PRIORITY_FOREGROUND); - mWatchdog.setWatchdogAlarm(mContext); - } - - /*package*/ synchronized void cancelDownload(DownloadRequest req) { - LogUtils.d(LOG_TAG, "cancelDownload #%d", req.mAttachmentId); - req.mInProgress = false; - mDownloadsInProgress.remove(req.mAttachmentId); - // Remove the download from our queue, and then decide whether or not to add it back. - remove(req); - req.mRetryCount++; - if (req.mRetryCount > CONNECTION_ERROR_MAX_RETRIES) { - LogUtils.d(LOG_TAG, "too many failures, giving up"); - } else { - LogUtils.d(LOG_TAG, "moving to end of queue, will retry"); - // The time field of DownloadRequest is final, because it's unsafe to change it - // as long as the DownloadRequest is in the DownloadSet. It's needed for the - // comparator, so changing time would make the request unfindable. - // Instead, we'll create a new DownloadRequest with an updated time. - // This will sort at the end of the set. - req = new DownloadRequest(req, SystemClock.elapsedRealtime()); - add(req); - } - } - - /** - * Called when a download is finished; we get notified of this via our EmailServiceCallback - * @param attachmentId the id of the attachment whose download is finished - * @param statusCode the EmailServiceStatus code returned by the Service - */ - /*package*/ synchronized void endDownload(long attachmentId, int statusCode) { - // Say we're no longer downloading this - mDownloadsInProgress.remove(attachmentId); - - // TODO: This code is conservative and treats connection issues as failures. - // Since we have no mechanism to throttle reconnection attempts, it makes - // sense to be cautious here. Once logic is in place to prevent connecting - // in a tight loop, we can exclude counting connection issues as "failures". - - // Update the attachment failure list if needed - Integer downloadCount; - downloadCount = mAttachmentFailureMap.remove(attachmentId); - if (statusCode != EmailServiceStatus.SUCCESS) { - if (downloadCount == null) { - downloadCount = 0; - } - downloadCount += 1; - mAttachmentFailureMap.put(attachmentId, downloadCount); - } - - DownloadRequest req = mDownloadSet.findDownloadRequest(attachmentId); - if (statusCode == EmailServiceStatus.CONNECTION_ERROR) { - // If this needs to be retried, just process the queue again - if (req != null) { - req.mRetryCount++; - if (req.mRetryCount > CONNECTION_ERROR_MAX_RETRIES) { - LogUtils.d(LOG_TAG, "Connection Error #%d, giving up", attachmentId); - remove(req); - } else if (req.mRetryCount > CONNECTION_ERROR_DELAY_THRESHOLD) { - // TODO: I'm not sure this is a great retry/backoff policy, but we're - // afraid of changing behavior too much in case something relies upon it. - // So now, for the first five errors, we'll retry immediately. For the next - // five tries, we'll add a ten second delay between each. After that, we'll - // give up. - LogUtils.d(LOG_TAG, "ConnectionError #%d, retried %d times, adding delay", - attachmentId, req.mRetryCount); - req.mInProgress = false; - req.mRetryStartTime = SystemClock.elapsedRealtime() + - CONNECTION_ERROR_RETRY_MILLIS; - mWatchdog.setWatchdogAlarm(mContext, CONNECTION_ERROR_RETRY_MILLIS, - CALLBACK_TIMEOUT); - } else { - LogUtils.d(LOG_TAG, "ConnectionError #%d, retried %d times, adding delay", - attachmentId, req.mRetryCount); - req.mInProgress = false; - req.mRetryStartTime = 0; - kick(); - } - } - return; - } - - // If the request is still in the queue, remove it + // In this case, there is no download priority for this attachment if (req != null) { - remove(req); + // If it exists in the map, remove it + // NOTE: We don't yet support deleting downloads in progress + if (LogUtils.isLoggable(LOG_TAG, LogUtils.DEBUG)) { + LogUtils.d(LOG_TAG, "== Attachment " + att.mId + " was in queue, removing"); + } + mDownloadQueue.removeRequest(req); } - if (LogUtils.isLoggable(LOG_TAG, LogUtils.DEBUG)) { - long secs = 0; - if (req != null) { - secs = (System.currentTimeMillis() - req.mTime) / 1000; - } - String status = (statusCode == EmailServiceStatus.SUCCESS) ? "Success" : - "Error " + statusCode; - LogUtils.d(LOG_TAG, "<< Download finished for attachment #" + attachmentId + "; " + secs - + " seconds from request, status: " + status); - } - - Attachment attachment = Attachment.restoreAttachmentWithId(mContext, attachmentId); - if (attachment != null) { - long accountId = attachment.mAccountKey; - // Update our attachment storage for this account - Long currentStorage = mAttachmentStorageMap.get(accountId); - if (currentStorage == null) { - currentStorage = 0L; - } - mAttachmentStorageMap.put(accountId, currentStorage + attachment.mSize); - boolean deleted = false; - if ((attachment.mFlags & Attachment.FLAG_DOWNLOAD_FORWARD) != 0) { - if (statusCode == EmailServiceStatus.ATTACHMENT_NOT_FOUND) { - // If this is a forwarding download, and the attachment doesn't exist (or - // can't be downloaded) delete it from the outgoing message, lest that - // message never get sent - EmailContent.delete(mContext, Attachment.CONTENT_URI, attachment.mId); - // TODO: Talk to UX about whether this is even worth doing - NotificationController nc = NotificationController.getInstance(mContext); - nc.showDownloadForwardFailedNotification(attachment); - deleted = true; - } - // If we're an attachment on forwarded mail, and if we're not still blocked, - // try to send pending mail now (as mediated by MailService) - if ((req != null) && - !Utility.hasUnloadedAttachments(mContext, attachment.mMessageKey)) { - if (LogUtils.isLoggable(LOG_TAG, LogUtils.DEBUG)) { - LogUtils.d(LOG_TAG, "== Downloads finished for outgoing msg #" - + req.mMessageId); - } - EmailServiceProxy service = EmailServiceUtils.getServiceForAccount( - mContext, accountId); - try { - service.sendMail(accountId); - } catch (RemoteException e) { - // We tried - } - } - } - if (statusCode == EmailServiceStatus.MESSAGE_NOT_FOUND) { - Message msg = Message.restoreMessageWithId(mContext, attachment.mMessageKey); - if (msg == null) { - // If there's no associated message, delete the attachment - EmailContent.delete(mContext, Attachment.CONTENT_URI, attachment.mId); - } else { - // If there really is a message, retry - // TODO: How will this get retried? It's still marked as inProgress? - kick(); + } else { + // Ignore changes that occur during download + if (mDownloadsInProgress.containsKey(att.mId)) return; + // If this is new, add the request to the queue + if (req == null) { + req = new DownloadRequest(context, att); + final AttachmentInfo attachInfo = new AttachmentInfo(context, att); + if (!attachInfo.isEligibleForDownload()) { + // We can't download this file due to policy, depending on what type + // of request we received, we handle the response differently. + if (((att.mFlags & Attachment.FLAG_DOWNLOAD_USER_REQUEST) != 0) || + ((att.mFlags & Attachment.FLAG_POLICY_DISALLOWS_DOWNLOAD) != 0)) { + // There are a couple of situations where we will not even allow this + // request to go in the queue because we can already process it as a + // failure. + // 1. The user explictly wants to download this attachment from the + // email view but they should not be able to...either because there is + // no app to view it or because its been marked as a policy violation. + // 2. The user is forwarding an email and the attachment has been + // marked as a policy violation. If the attachment is non viewable + // that is OK for forwarding a message so we'll let it pass through + markAttachmentAsFailed(att); return; } - } else if (!deleted) { - // Clear the download flags, since we're done for now. Note that this happens - // only for non-recoverable errors. When these occur for forwarded mail, we can - // ignore it and continue; otherwise, it was either 1) a user request, in which - // case the user can retry manually or 2) an opportunistic download, in which - // case the download wasn't critical - ContentValues cv = new ContentValues(); - int flags = - Attachment.FLAG_DOWNLOAD_FORWARD | Attachment.FLAG_DOWNLOAD_USER_REQUEST; - cv.put(AttachmentColumns.FLAGS, attachment.mFlags &= ~flags); - cv.put(AttachmentColumns.UI_STATE, AttachmentState.SAVED); - attachment.update(mContext, cv); + // If we get this far it a forward of an attachment that is only + // ineligible because we can't view it or process it. Not because we + // can't download it for policy reasons. Let's let this go through because + // the final recipient of this forward email might be able to process it. + } + mDownloadQueue.addRequest(req); + } + // If the request already existed, we'll update the priority (so that the time is + // up-to-date); otherwise, we create a new request + if (LogUtils.isLoggable(LOG_TAG, LogUtils.DEBUG)) { + LogUtils.d(LOG_TAG, "== Download queued for attachment " + att.mId + ", class " + + req.mPriority + ", priority time " + req.mTime); + } + } + // Process the queue if we're in a wait + kick(); + } + + /** + * Run through the AttachmentMap and find DownloadRequests that can be executed, enforcing + * the limit on maximum downloads + */ + synchronized void processQueue() { + if (LogUtils.isLoggable(LOG_TAG, LogUtils.DEBUG)) { + LogUtils.d(LOG_TAG, "== Checking attachment queue, " + mDownloadQueue.getSize() + + " entries"); + } + + while (mDownloadsInProgress.size() < MAX_SIMULTANEOUS_DOWNLOADS) { + final DownloadRequest req = mDownloadQueue.getNextRequest(); + if (req == null) { + // No more queued requests? We are done for now. + break; + } + // Enforce per-account limit here + if (downloadsForAccount(req.mAccountId) >= MAX_SIMULTANEOUS_DOWNLOADS_PER_ACCOUNT) { + if (LogUtils.isLoggable(LOG_TAG, LogUtils.DEBUG)) { + LogUtils.d(LOG_TAG, "== Skip #" + req.mAttachmentId + "; maxed for acct #" + + req.mAccountId); + } + continue; + } else if (Attachment.restoreAttachmentWithId(mContext, req.mAttachmentId) == null) { + continue; + } + if (!req.mInProgress) { + final long currentTime = SystemClock.elapsedRealtime(); + if (req.mRetryCount > 0 && req.mRetryStartTime > currentTime) { + LogUtils.d(LOG_TAG, "== waiting to retry attachment %d", req.mAttachmentId); + mWatchdog.setWatchdogAlarm(mContext, CONNECTION_ERROR_RETRY_MILLIS, + CALLBACK_TIMEOUT); + continue; + } + // TODO: We try to gate ineligible downloads from entering the queue but its + // always possible that they made it in here regardless in the future. In a + // perfect world, we would make it bullet proof with a check for eligibility + // here instead/also. + tryStartDownload(req); + } + } + + // Check our ability to be opportunistic regarding background downloads. + final EmailConnectivityManager ecm = mConnectivityManager; + if ((ecm == null) || !ecm.isAutoSyncAllowed() || + (ecm.getActiveNetworkType() != ConnectivityManager.TYPE_WIFI)) { + // Only prefetch if it if connectivity is available, prefetch is enabled + // and we are on WIFI + return; + } + + // Then, try opportunistic download of appropriate attachments + final int backgroundDownloads = MAX_SIMULTANEOUS_DOWNLOADS - mDownloadsInProgress.size(); + if ((backgroundDownloads + 1) >= MAX_SIMULTANEOUS_DOWNLOADS) { + // We want to leave one spot open for a user requested download that we haven't + // started processing yet. + return; + } + + // We'll load up the newest 25 attachments that aren't loaded or queued + // TODO: We are always looking for MAX_ATTACHMENTS_TO_CHECK, shouldn't this be + // backgroundDownloads instead? We should fix and test this. + final Uri lookupUri = EmailContent.uriWithLimit(Attachment.CONTENT_URI, + MAX_ATTACHMENTS_TO_CHECK); + final Cursor c = mContext.getContentResolver().query(lookupUri, + Attachment.CONTENT_PROJECTION, + EmailContent.Attachment.PRECACHE_INBOX_SELECTION, + null, AttachmentColumns._ID + " DESC"); + File cacheDir = mContext.getCacheDir(); + try { + while (c.moveToNext()) { + final Attachment att = new Attachment(); + att.restore(c); + final Account account = Account.restoreAccountWithId(mContext, att.mAccountKey); + if (account == null) { + // Clean up this orphaned attachment; there's no point in keeping it + // around; then try to find another one + EmailContent.delete(mContext, Attachment.CONTENT_URI, att.mId); + } else { + // Check that the attachment meets system requirements for download + // Note that there couple be policy that does not allow this attachment + // to be downloaded. + final AttachmentInfo info = new AttachmentInfo(mContext, att); + if (info.isEligibleForDownload()) { + // Either the account must be able to prefetch or this must be + // an inline attachment. + if (att.mContentId != null || + (canPrefetchForAccount(account, cacheDir))) { + final Integer tryCount = mAttachmentFailureMap.get(att.mId); + if (tryCount != null && tryCount > MAX_DOWNLOAD_RETRIES) { + // move onto the next attachment + continue; + } + // Start this download and we're done + final DownloadRequest req = new DownloadRequest(mContext, att); + tryStartDownload(req); + break; + } + } else { + // If this attachment was ineligible for download + // because of policy related issues, its flags would be set to + // FLAG_POLICY_DISALLOWS_DOWNLOAD and would not show up in the + // query results. We are most likely here for other reasons such + // as the inability to view the attachment. In that case, let's just + // skip it for now. + LogUtils.e(LOG_TAG, "== skip attachment %d, it is ineligible", att.mId); + } } } - // Process the queue - kick(); + } finally { + c.close(); } } + /** + * Count the number of running downloads in progress for this account + * @param accountId the id of the account + * @return the count of running downloads + */ + synchronized int downloadsForAccount(final long accountId) { + int count = 0; + for (final DownloadRequest req: mDownloadsInProgress.values()) { + if (req.mAccountId == accountId) { + count++; + } + } + return count; + } + + /** + * Attempt to execute the DownloadRequest, enforcing the maximum downloads per account + * parameter + * @param req the DownloadRequest + * @return whether or not the download was started + */ + synchronized boolean tryStartDownload(final DownloadRequest req) { + final EmailServiceProxy service = EmailServiceUtils.getServiceForAccount( + AttachmentService.this, req.mAccountId); + + // Do not download the same attachment multiple times + boolean alreadyInProgress = mDownloadsInProgress.get(req.mAttachmentId) != null; + if (alreadyInProgress) return false; + + try { + if (LogUtils.isLoggable(LOG_TAG, LogUtils.DEBUG)) { + LogUtils.d(LOG_TAG, ">> Starting download for attachment #" + req.mAttachmentId); + } + startDownload(service, req); + } catch (RemoteException e) { + // TODO: Consider whether we need to do more in this case... + // For now, fix up our data to reflect the failure + cancelDownload(req); + } + return true; + } + + /** + * Do the work of starting an attachment download using the EmailService interface, and + * set our watchdog alarm + * + * @param service the service handling the download + * @param req the DownloadRequest + * @throws RemoteException + */ + private void startDownload(final EmailServiceProxy service, final DownloadRequest req) + throws RemoteException { + req.mStartTime = System.currentTimeMillis(); + req.mInProgress = true; + mDownloadsInProgress.put(req.mAttachmentId, req); + service.loadAttachment(mServiceCallback, req.mAccountId, req.mAttachmentId, + req.mPriority != PRIORITY_FOREGROUND); + mWatchdog.setWatchdogAlarm(mContext); + } + + synchronized void cancelDownload(final DownloadRequest req) { + LogUtils.d(LOG_TAG, "cancelDownload #%d", req.mAttachmentId); + req.mInProgress = false; + mDownloadsInProgress.remove(req.mAttachmentId); + // Remove the download from our queue, and then decide whether or not to add it back. + mDownloadQueue.removeRequest(req); + req.mRetryCount++; + if (req.mRetryCount > CONNECTION_ERROR_MAX_RETRIES) { + LogUtils.d(LOG_TAG, "too many failures, giving up"); + } else { + LogUtils.d(LOG_TAG, "moving to end of queue, will retry"); + // The time field of DownloadRequest is final, because it's unsafe to change it + // as long as the DownloadRequest is in the DownloadSet. It's needed for the + // comparator, so changing time would make the request unfindable. + // Instead, we'll create a new DownloadRequest with an updated time. + // This will sort at the end of the set. + final DownloadRequest newReq = new DownloadRequest(req, SystemClock.elapsedRealtime()); + mDownloadQueue.addRequest(newReq); + } + } + + /** + * Called when a download is finished; we get notified of this via our EmailServiceCallback + * @param attachmentId the id of the attachment whose download is finished + * @param statusCode the EmailServiceStatus code returned by the Service + */ + synchronized void endDownload(final long attachmentId, final int statusCode) { + // Say we're no longer downloading this + mDownloadsInProgress.remove(attachmentId); + + // TODO: This code is conservative and treats connection issues as failures. + // Since we have no mechanism to throttle reconnection attempts, it makes + // sense to be cautious here. Once logic is in place to prevent connecting + // in a tight loop, we can exclude counting connection issues as "failures". + + // Update the attachment failure list if needed + Integer downloadCount; + downloadCount = mAttachmentFailureMap.remove(attachmentId); + if (statusCode != EmailServiceStatus.SUCCESS) { + if (downloadCount == null) { + downloadCount = 0; + } + downloadCount += 1; + mAttachmentFailureMap.put(attachmentId, downloadCount); + } + + final DownloadRequest req = mDownloadQueue.findRequestById(attachmentId); + if (statusCode == EmailServiceStatus.CONNECTION_ERROR) { + // If this needs to be retried, just process the queue again + if (req != null) { + req.mRetryCount++; + if (req.mRetryCount > CONNECTION_ERROR_MAX_RETRIES) { + // We are done, we maxed out our total number of tries. + LogUtils.d(LOG_TAG, "Connection Error #%d, giving up", attachmentId); + mDownloadQueue.removeRequest(req); + // Note that we are not doing anything with the attachment right now + // We will annotate it later in this function if needed. + } else if (req.mRetryCount > CONNECTION_ERROR_DELAY_THRESHOLD) { + // TODO: I'm not sure this is a great retry/backoff policy, but we're + // afraid of changing behavior too much in case something relies upon it. + // So now, for the first five errors, we'll retry immediately. For the next + // five tries, we'll add a ten second delay between each. After that, we'll + // give up. + LogUtils.d(LOG_TAG, "ConnectionError #%d, retried %d times, adding delay", + attachmentId, req.mRetryCount); + req.mInProgress = false; + req.mRetryStartTime = SystemClock.elapsedRealtime() + + CONNECTION_ERROR_RETRY_MILLIS; + mWatchdog.setWatchdogAlarm(mContext, CONNECTION_ERROR_RETRY_MILLIS, + CALLBACK_TIMEOUT); + } else { + LogUtils.d(LOG_TAG, "ConnectionError #%d, retried %d times, adding delay", + attachmentId, req.mRetryCount); + req.mInProgress = false; + req.mRetryStartTime = 0; + kick(); + } + } + return; + } + + // If the request is still in the queue, remove it + if (req != null) { + mDownloadQueue.removeRequest(req); + } + + if (LogUtils.isLoggable(LOG_TAG, LogUtils.DEBUG)) { + long secs = 0; + if (req != null) { + secs = (System.currentTimeMillis() - req.mTime) / 1000; + } + final String status = (statusCode == EmailServiceStatus.SUCCESS) ? "Success" : + "Error " + statusCode; + LogUtils.d(LOG_TAG, "<< Download finished for attachment #" + attachmentId + "; " + secs + + " seconds from request, status: " + status); + } + + final Attachment attachment = Attachment.restoreAttachmentWithId(mContext, attachmentId); + if (attachment != null) { + final long accountId = attachment.mAccountKey; + // Update our attachment storage for this account + Long currentStorage = mAttachmentStorageMap.get(accountId); + if (currentStorage == null) { + currentStorage = 0L; + } + mAttachmentStorageMap.put(accountId, currentStorage + attachment.mSize); + boolean deleted = false; + if ((attachment.mFlags & Attachment.FLAG_DOWNLOAD_FORWARD) != 0) { + if (statusCode == EmailServiceStatus.ATTACHMENT_NOT_FOUND) { + // If this is a forwarding download, and the attachment doesn't exist (or + // can't be downloaded) delete it from the outgoing message, lest that + // message never get sent + EmailContent.delete(mContext, Attachment.CONTENT_URI, attachment.mId); + // TODO: Talk to UX about whether this is even worth doing + NotificationController nc = NotificationController.getInstance(mContext); + nc.showDownloadForwardFailedNotification(attachment); + deleted = true; + } + // If we're an attachment on forwarded mail, and if we're not still blocked, + // try to send pending mail now (as mediated by MailService) + if ((req != null) && + !Utility.hasUnloadedAttachments(mContext, attachment.mMessageKey)) { + if (LogUtils.isLoggable(LOG_TAG, LogUtils.DEBUG)) { + LogUtils.d(LOG_TAG, "== Downloads finished for outgoing msg #" + + req.mMessageId); + } + EmailServiceProxy service = EmailServiceUtils.getServiceForAccount( + mContext, accountId); + try { + service.sendMail(accountId); + } catch (RemoteException e) { + // We tried + } + } + } + if (statusCode == EmailServiceStatus.MESSAGE_NOT_FOUND) { + Message msg = Message.restoreMessageWithId(mContext, attachment.mMessageKey); + if (msg == null) { + // If there's no associated message, delete the attachment + EmailContent.delete(mContext, Attachment.CONTENT_URI, attachment.mId); + } else { + // If there really is a message, retry + // TODO: How will this get retried? It's still marked as inProgress? + kick(); + return; + } + } else if (!deleted) { + // Clear the download flags, since we're done for now. Note that this happens + // only for non-recoverable errors. When these occur for forwarded mail, we can + // ignore it and continue; otherwise, it was either 1) a user request, in which + // case the user can retry manually or 2) an opportunistic download, in which + // case the download wasn't critical + ContentValues cv = new ContentValues(); + int flags = + Attachment.FLAG_DOWNLOAD_FORWARD | Attachment.FLAG_DOWNLOAD_USER_REQUEST; + cv.put(AttachmentColumns.FLAGS, attachment.mFlags &= ~flags); + cv.put(AttachmentColumns.UI_STATE, AttachmentState.SAVED); + attachment.update(mContext, cv); + } + } + // Process the queue + kick(); + } + /** * Calculate the download priority of an Attachment. A priority of zero means that the * attachment is not marked for download. * @param att the Attachment * @return the priority key of the Attachment */ - private static int getPriority(Attachment att) { + private static int getPriority(final Attachment att) { int priorityClass = PRIORITY_NONE; - int flags = att.mFlags; + final int flags = att.mFlags; if ((flags & Attachment.FLAG_DOWNLOAD_FORWARD) != 0) { priorityClass = PRIORITY_SEND_MAIL; } else if ((flags & Attachment.FLAG_DOWNLOAD_USER_REQUEST) != 0) { @@ -947,15 +902,15 @@ public class AttachmentService extends Service implements Runnable { * come from either Controller (IMAP) or ExchangeService (EAS). Note that we only implement the * single callback that's defined by the EmailServiceCallback interface. */ - private class ServiceCallback extends IEmailServiceCallback.Stub { + class ServiceCallback extends IEmailServiceCallback.Stub { @Override - public void loadAttachmentStatus(long messageId, long attachmentId, int statusCode, - int progress) { + public void loadAttachmentStatus(final long messageId, final long attachmentId, + final int statusCode, final int progress) { // Record status and progress - DownloadRequest req = mDownloadSet.getDownloadInProgress(attachmentId); + final DownloadRequest req = mDownloadsInProgress.get(attachmentId); if (req != null) { if (LogUtils.isLoggable(LOG_TAG, LogUtils.DEBUG)) { - String code; + final String code; switch(statusCode) { case EmailServiceStatus.SUCCESS: code = "Success"; break; case EmailServiceStatus.IN_PROGRESS: code = "In progress"; break; @@ -970,88 +925,32 @@ public class AttachmentService extends Service implements Runnable { req.mLastStatusCode = statusCode; req.mLastProgress = progress; req.mLastCallbackTime = System.currentTimeMillis(); - Attachment attachment = Attachment.restoreAttachmentWithId(mContext, attachmentId); + final Attachment attachment = + Attachment.restoreAttachmentWithId(mContext, attachmentId); if (attachment != null && statusCode == EmailServiceStatus.IN_PROGRESS) { - ContentValues values = new ContentValues(); + final ContentValues values = new ContentValues(); values.put(AttachmentColumns.UI_DOWNLOADED_SIZE, attachment.mSize * progress / 100); // Update UIProvider with updated download size // Individual services will set contentUri and state when finished attachment.update(mContext, values); } - } - switch (statusCode) { - case EmailServiceStatus.IN_PROGRESS: - break; - default: - mDownloadSet.endDownload(attachmentId, statusCode); - break; + switch (statusCode) { + case EmailServiceStatus.IN_PROGRESS: + break; + default: + endDownload(attachmentId, statusCode); + break; + } + } else { + // The only way that we can get a callback from the service implementation for + // an attachment that doesn't exist is if it was cancelled due to the + // AttachmentWatchdog. This is a valid scenario and the Watchdog should have already + // marked this attachment as failed/cancelled. } } } - /*package*/ void onChange(Attachment att) { - mDownloadSet.onChange(this, att); - } - - /*package*/ boolean isQueued(long attachmentId) { - return mDownloadSet.findDownloadRequest(attachmentId) != null; - } - - /*package*/ int getSize() { - return mDownloadSet.size(); - } - - /*package*/ boolean dequeue(long attachmentId) { - DownloadRequest req = mDownloadSet.findDownloadRequest(attachmentId); - if (req != null) { - if (LogUtils.isLoggable(LOG_TAG, LogUtils.DEBUG)) { - LogUtils.d(LOG_TAG, "Dequeued attachmentId: " + attachmentId); - } - mDownloadSet.remove(req); - return true; - } - return false; - } - - /** - * Ask the service for the number of items in the download queue - * @return the number of items queued for download - */ - public static int getQueueSize() { - AttachmentService service = sRunningService; - if (service != null) { - return service.getSize(); - } - return 0; - } - - /** - * Ask the service whether a particular attachment is queued for download - * @param attachmentId the id of the Attachment (as stored by EmailProvider) - * @return whether or not the attachment is queued for download - */ - public static boolean isAttachmentQueued(long attachmentId) { - AttachmentService service = sRunningService; - if (service != null) { - return service.isQueued(attachmentId); - } - return false; - } - - /** - * Ask the service to remove an attachment from the download queue - * @param attachmentId the id of the Attachment (as stored by EmailProvider) - * @return whether or not the attachment was removed from the queue - */ - public static boolean cancelQueuedAttachment(long attachmentId) { - AttachmentService service = sRunningService; - if (service != null) { - return service.dequeue(attachmentId); - } - return false; - } - // The queue entries here are entries of the form {id, flags}, with the values passed in to // attachmentChanged() private static final Queue sAttachmentChangedQueue = @@ -1059,7 +958,9 @@ public class AttachmentService extends Service implements Runnable { private static AsyncTask sAttachmentChangedTask; /** - * Called directly by EmailProvider whenever an attachment is inserted or changed + * Called directly by EmailProvider whenever an attachment is inserted or changed. Since this + * call is being invoked on the UI thread, we need to make sure that the downloads are + * happening in the background. * @param context the caller's context * @param id the attachment's id * @param flags the new flags for the attachment @@ -1067,7 +968,6 @@ public class AttachmentService extends Service implements Runnable { public static void attachmentChanged(final Context context, final long id, final int flags) { synchronized (sAttachmentChangedQueue) { sAttachmentChangedQueue.add(new long[]{id, flags}); - if (sAttachmentChangedTask == null) { sAttachmentChangedTask = new AsyncTask() { @Override @@ -1092,6 +992,8 @@ public class AttachmentService extends Service implements Runnable { final Intent intent = new Intent(context, AttachmentService.class); intent.putExtra(EXTRA_ATTACHMENT, attachment); + // This is result in a call to AttachmentService.onStartCommand() + // which will queue the attachment in its internal prioritized queue. context.startService(intent); } } @@ -1101,31 +1003,34 @@ public class AttachmentService extends Service implements Runnable { } /** - * Determine whether an attachment can be prefetched for the given account + * Determine whether an attachment can be prefetched for the given account based on + * total download size restrictions tied to the account. * @return true if download is allowed, false otherwise */ - public boolean canPrefetchForAccount(Account account, File dir) { + public boolean canPrefetchForAccount(final Account account, final File dir) { // Check account, just in case if (account == null) return false; + // First, check preference and quickly return if prefetch isn't allowed if ((account.mFlags & Account.FLAGS_BACKGROUND_ATTACHMENTS) == 0) return false; - long totalStorage = dir.getTotalSpace(); - long usableStorage = dir.getUsableSpace(); - long minAvailable = (long)(totalStorage * PREFETCH_MINIMUM_STORAGE_AVAILABLE); + final long totalStorage = dir.getTotalSpace(); + final long usableStorage = dir.getUsableSpace(); + final long minAvailable = (long)(totalStorage * PREFETCH_MINIMUM_STORAGE_AVAILABLE); // If there's not enough overall storage available, stop now - if (usableStorage < minAvailable) { - return false; - } + if (usableStorage < minAvailable) return false; - int numberOfAccounts = mAccountManagerStub.getNumberOfAccounts(); - long perAccountMaxStorage = + final int numberOfAccounts = mAccountManagerStub.getNumberOfAccounts(); + // Calculate an even per-account storage although it would make a lot of sense to not + // do this as you may assign more storage to your corporate account rather than a personal + // account. + final long perAccountMaxStorage = (long)(totalStorage * PREFETCH_MAXIMUM_ATTACHMENT_STORAGE / numberOfAccounts); // Retrieve our idea of currently used attachment storage; since we don't track deletions, // this number is the "worst case". If the number is greater than what's allowed per - // account, we walk the directory to determine the actual number + // account, we walk the directory to determine the actual number. Long accountStorage = mAttachmentStorageMap.get(account.mId); if (accountStorage == null || (accountStorage > perAccountMaxStorage)) { // Calculate the exact figure for attachment storage for this account @@ -1136,22 +1041,24 @@ public class AttachmentService extends Service implements Runnable { accountStorage += file.length(); } } - // Cache the value + // Cache the value. No locking here since this is a concurrent collection object. mAttachmentStorageMap.put(account.mId, accountStorage); } // Return true if we're using less than the maximum per account - if (accountStorage < perAccountMaxStorage) { - return true; - } else { + if (accountStorage >= perAccountMaxStorage) { if (LogUtils.isLoggable(LOG_TAG, LogUtils.DEBUG)) { - LogUtils.d(LOG_TAG, ">> Prefetch not allowed for account " + account.mId + "; used " + - accountStorage + ", limit " + perAccountMaxStorage); + LogUtils.d(LOG_TAG, ">> Prefetch not allowed for account " + account.mId + + "; used " + accountStorage + ", limit " + perAccountMaxStorage); } return false; } + return true; } + /** + * The main routine for our AttachmentService service thread. + */ @Override public void run() { // These fields are only used within the service thread @@ -1160,24 +1067,24 @@ public class AttachmentService extends Service implements Runnable { mAccountManagerStub = new AccountManagerStub(this); // Run through all attachments in the database that require download and add them to - // the queue - int mask = Attachment.FLAG_DOWNLOAD_FORWARD | Attachment.FLAG_DOWNLOAD_USER_REQUEST; - Cursor c = getContentResolver().query(Attachment.CONTENT_URI, + // the queue. This is the case where a previous AttachmentService may have been notified + // to stop before processing everything in its queue. + final int mask = Attachment.FLAG_DOWNLOAD_FORWARD | Attachment.FLAG_DOWNLOAD_USER_REQUEST; + final Cursor c = getContentResolver().query(Attachment.CONTENT_URI, EmailContent.ID_PROJECTION, "(" + AttachmentColumns.FLAGS + " & ?) != 0", new String[] {Integer.toString(mask)}, null); try { LogUtils.d(LOG_TAG, "Count: " + c.getCount()); while (c.moveToNext()) { - Attachment attachment = Attachment.restoreAttachmentWithId( + final Attachment attachment = Attachment.restoreAttachmentWithId( this, c.getLong(EmailContent.ID_PROJECTION_COLUMN)); if (attachment != null) { - mDownloadSet.onChange(this, attachment); + onChange(this, attachment); } } } catch (Exception e) { e.printStackTrace(); - } - finally { + } finally { c.close(); } @@ -1191,10 +1098,11 @@ public class AttachmentService extends Service implements Runnable { } if (mStop) { // We might be bailing out here due to the service shutting down + LogUtils.d(LOG_TAG, "*** AttachmentService has been instructed to stop"); break; } - mDownloadSet.processQueue(); - if (mDownloadSet.isEmpty()) { + processQueue(); + if (mDownloadQueue.isEmpty()) { LogUtils.d(LOG_TAG, "*** All done; shutting down service"); stopSelf(); break; @@ -1217,32 +1125,37 @@ public class AttachmentService extends Service implements Runnable { } @Override - public int onStartCommand(Intent intent, int flags, int startId) { + public int onStartCommand(final Intent intent, final int flags, final int startId) { if (sRunningService == null) { sRunningService = this; } if (intent != null && intent.hasExtra(EXTRA_ATTACHMENT)) { Attachment att = intent.getParcelableExtra(EXTRA_ATTACHMENT); - onChange(att); + onChange(mContext, att); + } else { + LogUtils.wtf(LOG_TAG, "Received an invalid intent w/o EXTRA_ATTACHMENT"); } return Service.START_STICKY; } @Override public void onCreate() { - // Start up our service thread + // Start up our service thread. new Thread(this, "AttachmentService").start(); } + @Override - public IBinder onBind(Intent intent) { + public IBinder onBind(final Intent intent) { return null; } @Override public void onDestroy() { - // Mark this instance of the service as stopped + // Mark this instance of the service as stopped. Our main loop for the AttachmentService + // checks for this flag along with the AttachmentWatchdog. mStop = true; if (sRunningService != null) { + // Kick it awake to get it to realize that we are stopping. kick(); sRunningService = null; } @@ -1255,27 +1168,31 @@ public class AttachmentService extends Service implements Runnable { // For Debugging. @Override - public void dump(FileDescriptor fd, PrintWriter pw, String[] args) { + public void dump(final FileDescriptor fd, final PrintWriter pw, final String[] args) { pw.println("AttachmentService"); - long time = System.currentTimeMillis(); - synchronized(mDownloadSet) { - pw.println(" Queue, " + mDownloadSet.size() + " entries"); - Iterator iterator = mDownloadSet.descendingIterator(); - // First, start up any required downloads, in priority order - while (iterator.hasNext()) { - DownloadRequest req = iterator.next(); + final long time = System.currentTimeMillis(); + synchronized(mDownloadQueue) { + pw.println(" Queue, " + mDownloadQueue.getSize() + " entries"); + // If you iterate over the queue either via iterator or collection, they are not + // returned in any particular order. With all things being equal its better to go with + // a collection to avoid any potential ConcurrentModificationExceptions. + // If we really want this sorted, we can sort it manually since performance isn't a big + // concern with this debug method. + for (final DownloadRequest req : mDownloadQueue.mRequestMap.values()) { pw.println(" Account: " + req.mAccountId + ", Attachment: " + req.mAttachmentId); pw.println(" Priority: " + req.mPriority + ", Time: " + req.mTime + (req.mInProgress ? " [In progress]" : "")); - Attachment att = Attachment.restoreAttachmentWithId(this, req.mAttachmentId); + final Attachment att = Attachment.restoreAttachmentWithId(this, req.mAttachmentId); if (att == null) { pw.println(" Attachment not in database?"); } else if (att.mFileName != null) { - String fileName = att.mFileName; - String suffix = "[none]"; - int lastDot = fileName.lastIndexOf('.'); + final String fileName = att.mFileName; + final String suffix; + final int lastDot = fileName.lastIndexOf('.'); if (lastDot >= 0) { suffix = fileName.substring(lastDot); + } else { + suffix = "[none]"; } pw.print(" Suffix: " + suffix); if (att.getContentUri() != null) { @@ -1305,10 +1222,10 @@ public class AttachmentService extends Service implements Runnable { } // For Testing - /*package*/ AccountManagerStub mAccountManagerStub; + AccountManagerStub mAccountManagerStub; private final HashMap mAccountServiceMap = new HashMap(); - /*package*/ void addServiceIntentForTest(long accountId, Intent intent) { + void addServiceIntentForTest(final long accountId, final Intent intent) { mAccountServiceMap.put(accountId, intent); } @@ -1316,11 +1233,11 @@ public class AttachmentService extends Service implements Runnable { * We only use the getAccounts() call from AccountManager, so this class wraps that call and * allows us to build a mock account manager stub in the unit tests */ - /*package*/ static class AccountManagerStub { + static class AccountManagerStub { private int mNumberOfAccounts; private final AccountManager mAccountManager; - AccountManagerStub(Context context) { + AccountManagerStub(final Context context) { if (context != null) { mAccountManager = AccountManager.get(context); } else { @@ -1328,7 +1245,7 @@ public class AttachmentService extends Service implements Runnable { } } - /*package*/ int getNumberOfAccounts() { + int getNumberOfAccounts() { if (mAccountManager != null) { return mAccountManager.getAccounts().length; } else { @@ -1336,7 +1253,7 @@ public class AttachmentService extends Service implements Runnable { } } - /*package*/ void setNumberOfAccounts(int numberOfAccounts) { + void setNumberOfAccounts(final int numberOfAccounts) { mNumberOfAccounts = numberOfAccounts; } }