From d5acf0bbc00cbe9a2c075e1bd4825ccbd9851d8d Mon Sep 17 00:00:00 2001 From: Yu Ping Hu Date: Mon, 15 Jul 2013 14:44:54 -0700 Subject: [PATCH] Explicitly pass a callback when loading attachments. This is part of moving away from the explicit setCallback, which either has race conditions or is very noisy, or both. (Each IEmailService call that wants callbacks should just pass the callback explicitly.) I'm not yet changing how the services actually handle the call. Each protocol will need to fix this on their own. Bug: 9735207 Bug: 9842867 Change-Id: If8cf69ffe82f3544ace9e58b1db5a183f38d038a --- .../emailcommon/service/EmailServiceProxy.java | 11 ++++++----- .../android/emailcommon/service/IEmailService.aidl | 2 +- .../email/service/AttachmentDownloadService.java | 5 +++-- src/com/android/email/service/EmailServiceStub.java | 4 +++- src/com/android/email/service/EmailServiceUtils.java | 3 ++- src/com/android/email/service/Pop3Service.java | 4 +++- 6 files changed, 18 insertions(+), 11 deletions(-) diff --git a/emailcommon/src/com/android/emailcommon/service/EmailServiceProxy.java b/emailcommon/src/com/android/emailcommon/service/EmailServiceProxy.java index 7e5eb8b96..ecb440560 100644 --- a/emailcommon/src/com/android/emailcommon/service/EmailServiceProxy.java +++ b/emailcommon/src/com/android/emailcommon/service/EmailServiceProxy.java @@ -125,19 +125,21 @@ public class EmailServiceProxy extends ServiceProxy implements IEmailService { * loading has started and stopped and SHOULD send callbacks with progress information if * possible. * + * @param cb The {@link IEmailServiceCallback} to use for this operation. * @param attachmentId the id of the attachment record * @param background whether or not this request corresponds to a background action (i.e. * prefetch) vs a foreground action (user request) */ @Override - public void loadAttachment(final long attachmentId, final boolean background) + public void loadAttachment(final IEmailServiceCallback cb, final long attachmentId, + final boolean background) throws RemoteException { setTask(new ProxyTask() { @Override public void run() throws RemoteException { try { if (mCallback != null) mService.setCallback(mCallback); - mService.loadAttachment(attachmentId, background); + mService.loadAttachment(mCallback, attachmentId, background); } catch (RemoteException e) { try { // Try to send a callback (if set) @@ -181,7 +183,6 @@ public class EmailServiceProxy extends ServiceProxy implements IEmailService { * the sync was started via the startSync service call. * * @param mailboxId the id of the mailbox record - * @param userRequest whether or not the user specifically asked for the sync */ @Override public void stopSync(final long mailboxId) throws RemoteException { @@ -262,7 +263,7 @@ public class EmailServiceProxy extends ServiceProxy implements IEmailService { * Request that the service reload the folder list for the specified account. The service * MUST use the syncMailboxListStatus callback to indicate "starting" and "finished" * - * @param accoundId the id of the account whose folder list is to be updated + * @param accountId the id of the account whose folder list is to be updated */ @Override public void updateFolderList(final long accountId) throws RemoteException { @@ -479,7 +480,7 @@ public class EmailServiceProxy extends ServiceProxy implements IEmailService { /** * Request that the account be updated for this service; this call is synchronous * - * @param the email address of the account to be updated + * @param emailAddress the email address of the account to be updated */ @Override public void serviceUpdated(final String emailAddress) throws RemoteException { diff --git a/emailcommon/src/com/android/emailcommon/service/IEmailService.aidl b/emailcommon/src/com/android/emailcommon/service/IEmailService.aidl index 3595f64c3..cae906a6a 100644 --- a/emailcommon/src/com/android/emailcommon/service/IEmailService.aidl +++ b/emailcommon/src/com/android/emailcommon/service/IEmailService.aidl @@ -31,7 +31,7 @@ interface IEmailService { // TODO: loadMore appears to be unused; if so, delete it. oneway void loadMore(long messageId); - oneway void loadAttachment(long attachmentId, boolean background); + oneway void loadAttachment(IEmailServiceCallback cb, long attachmentId, boolean background); oneway void updateFolderList(long accountId); diff --git a/src/com/android/email/service/AttachmentDownloadService.java b/src/com/android/email/service/AttachmentDownloadService.java index 96387a59a..7d1a3c7c3 100644 --- a/src/com/android/email/service/AttachmentDownloadService.java +++ b/src/com/android/email/service/AttachmentDownloadService.java @@ -492,7 +492,7 @@ public class AttachmentDownloadService extends Service implements Runnable { * Do the work of starting an attachment download using the EmailService interface, and * set our watchdog alarm * - * @param serviceClass the service handling the download + * @param service the service handling the download * @param req the DownloadRequest * @throws RemoteException */ @@ -501,7 +501,8 @@ public class AttachmentDownloadService extends Service implements Runnable { req.startTime = System.currentTimeMillis(); req.inProgress = true; mDownloadsInProgress.put(req.attachmentId, req); - service.loadAttachment(req.attachmentId, req.priority != PRIORITY_FOREGROUND); + service.loadAttachment(mServiceCallback, req.attachmentId, + req.priority != PRIORITY_FOREGROUND); setWatchdogAlarm(); } diff --git a/src/com/android/email/service/EmailServiceStub.java b/src/com/android/email/service/EmailServiceStub.java index 2ac0808b6..97d6ff23a 100644 --- a/src/com/android/email/service/EmailServiceStub.java +++ b/src/com/android/email/service/EmailServiceStub.java @@ -206,8 +206,10 @@ public abstract class EmailServiceStub extends IEmailService.Stub implements IEm } } + // TODO: Switch from using setCallback to the explicitly passed callback. @Override - public void loadAttachment(long attachmentId, boolean background) throws RemoteException { + public void loadAttachment(final IEmailServiceCallback cb, final long attachmentId, + final boolean background) throws RemoteException { try { //1. Check if the attachment is already here and return early in that case Attachment attachment = diff --git a/src/com/android/email/service/EmailServiceUtils.java b/src/com/android/email/service/EmailServiceUtils.java index af6d06bbd..671c6b565 100644 --- a/src/com/android/email/service/EmailServiceUtils.java +++ b/src/com/android/email/service/EmailServiceUtils.java @@ -612,7 +612,8 @@ public class EmailServiceUtils { } @Override - public void loadAttachment(long attachmentId, boolean background) throws RemoteException { + public void loadAttachment(final IEmailServiceCallback cb, final long attachmentId, + final boolean background) throws RemoteException { } @Override diff --git a/src/com/android/email/service/Pop3Service.java b/src/com/android/email/service/Pop3Service.java index d7e753b4d..57ddf133f 100644 --- a/src/com/android/email/service/Pop3Service.java +++ b/src/com/android/email/service/Pop3Service.java @@ -95,8 +95,10 @@ public class Pop3Service extends Service { return AccountCapabilities.UNDO; } + // TODO: Switch from using the callback from setCallback to using the one passed here. @Override - public void loadAttachment(long attachmentId, boolean background) throws RemoteException { + public void loadAttachment(final IEmailServiceCallback callback, final long attachmentId, + final boolean background) throws RemoteException { Attachment att = Attachment.restoreAttachmentWithId(mContext, attachmentId); if (att == null || att.mUiState != AttachmentState.DOWNLOADING) return; long inboxId = Mailbox.findMailboxOfType(mContext, att.mAccountKey, Mailbox.TYPE_INBOX);