Merge "Fix a race condition in which an Attachment might be wrongly deleted"

This commit is contained in:
Marc Blank 2011-05-11 18:03:26 -07:00 committed by Android (Google) Code Review
commit ffeb7de284
3 changed files with 20 additions and 8 deletions

View File

@ -1963,16 +1963,20 @@ public abstract class EmailContent {
}; };
// All attachments with an empty URI, regardless of mailbox // All attachments with an empty URI, regardless of mailbox
public static final String EMPTY_URI_SELECTION = public static final String PRECACHE_SELECTION =
AttachmentColumns.CONTENT_URI + " isnull AND " + Attachment.FLAGS + "=0"; AttachmentColumns.CONTENT_URI + " isnull AND " + Attachment.FLAGS + "=0";
// Attachments with an empty URI that are in an inbox // Attachments with an empty URI that are in an inbox
public static final String EMPTY_URI_INBOX_SELECTION = public static final String PRECACHE_INBOX_SELECTION =
EMPTY_URI_SELECTION + " AND " + AttachmentColumns.MESSAGE_KEY + " IN (" PRECACHE_SELECTION + " AND " + AttachmentColumns.MESSAGE_KEY + " IN ("
+ "SELECT " + MessageColumns.ID + " FROM " + Message.TABLE_NAME + "SELECT " + MessageColumns.ID + " FROM " + Message.TABLE_NAME
+ " WHERE " + Message.INBOX_SELECTION + " WHERE " + Message.INBOX_SELECTION
+ ")"; + ")";
// Bits used in mFlags // Bits used in mFlags
// WARNING: AttachmentDownloadService relies on the fact that ALL of the flags below
// disqualify attachments for precaching. If you add a flag that does NOT disqualify an
// attachment for precaching, you MUST change the PRECACHE_SELECTION definition above
// Instruct Rfc822Output to 1) not use Content-Disposition and 2) use multipart/alternative // Instruct Rfc822Output to 1) not use Content-Disposition and 2) use multipart/alternative
// with this attachment. This is only valid if there is one and only one attachment and // with this attachment. This is only valid if there is one and only one attachment and
// that attachment has this flag set // that attachment has this flag set

View File

@ -342,7 +342,7 @@ public class AttachmentDownloadService extends Service implements Runnable {
Uri lookupUri = EmailContent.uriWithLimit(Attachment.CONTENT_URI, Uri lookupUri = EmailContent.uriWithLimit(Attachment.CONTENT_URI,
MAX_ATTACHMENTS_TO_CHECK); MAX_ATTACHMENTS_TO_CHECK);
Cursor c = mContext.getContentResolver().query(lookupUri, AttachmentInfo.PROJECTION, Cursor c = mContext.getContentResolver().query(lookupUri, AttachmentInfo.PROJECTION,
EmailContent.Attachment.EMPTY_URI_INBOX_SELECTION, EmailContent.Attachment.PRECACHE_INBOX_SELECTION,
null, Attachment.RECORD_ID + " DESC"); null, Attachment.RECORD_ID + " DESC");
File cacheDir = mContext.getCacheDir(); File cacheDir = mContext.getCacheDir();
try { try {
@ -481,6 +481,7 @@ public class AttachmentDownloadService extends Service implements Runnable {
mWatchdogPendingIntent = PendingIntent.getBroadcast(context, 0, alarmIntent, 0); mWatchdogPendingIntent = PendingIntent.getBroadcast(context, 0, alarmIntent, 0);
mAlarmManager = (AlarmManager)context.getSystemService(Context.ALARM_SERVICE); mAlarmManager = (AlarmManager)context.getSystemService(Context.ALARM_SERVICE);
} }
private void cancelDownload(DownloadRequest req) { private void cancelDownload(DownloadRequest req) {
mDownloadsInProgress.remove(req.attachmentId); mDownloadsInProgress.remove(req.attachmentId);
req.inProgress = false; req.inProgress = false;
@ -572,8 +573,15 @@ public class AttachmentDownloadService extends Service implements Runnable {
} }
} }
if (statusCode == EmailServiceStatus.MESSAGE_NOT_FOUND) { if (statusCode == EmailServiceStatus.MESSAGE_NOT_FOUND) {
// If there's no associated message, delete the attachment Message msg = Message.restoreMessageWithId(mContext, attachment.mMessageKey);
EmailContent.delete(mContext, Attachment.CONTENT_URI, attachment.mId); 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
kick();
return;
}
} else if (!deleted) { } else if (!deleted) {
// Clear the download flags, since we're done for now. Note that this happens // 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 // only for non-recoverable errors. When these occur for forwarded mail, we can

View File

@ -235,7 +235,7 @@ public class AttachmentProviderTests extends ProviderTestCase2<AttachmentProvide
// count all attachments with an empty URI, regardless of mailbox location // count all attachments with an empty URI, regardless of mailbox location
c = mMockContext.getContentResolver().query( c = mMockContext.getContentResolver().query(
Attachment.CONTENT_URI, AttachmentInfo.PROJECTION, Attachment.CONTENT_URI, AttachmentInfo.PROJECTION,
EmailContent.Attachment.EMPTY_URI_SELECTION, EmailContent.Attachment.PRECACHE_SELECTION,
null, Attachment.RECORD_ID + " DESC"); null, Attachment.RECORD_ID + " DESC");
assertEquals(9, c.getCount()); assertEquals(9, c.getCount());
} finally { } finally {
@ -246,7 +246,7 @@ public class AttachmentProviderTests extends ProviderTestCase2<AttachmentProvide
// count all attachments with an empty URI, only in an inbox // count all attachments with an empty URI, only in an inbox
c = mMockContext.getContentResolver().query( c = mMockContext.getContentResolver().query(
Attachment.CONTENT_URI, AttachmentInfo.PROJECTION, Attachment.CONTENT_URI, AttachmentInfo.PROJECTION,
EmailContent.Attachment.EMPTY_URI_INBOX_SELECTION, EmailContent.Attachment.PRECACHE_INBOX_SELECTION,
null, Attachment.RECORD_ID + " DESC"); null, Attachment.RECORD_ID + " DESC");
assertEquals(4, c.getCount()); assertEquals(4, c.getCount());
} finally { } finally {