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

* This is a serious bug dating back to the first Honeycomb release
* It was possible that a newly created Message could not yet be
  committed to the database when the AttachmentDownloadService
  tries to download one of that message's attachments.
* ADS, when it sees that the message (apparently) doesn't
  exist, deletes the Attachment (it appears to be orphaned)
* The effect is that the user never sees one of the attachments
  in a message.
* This bug has been reported externally
* The fix is simply to check for the message's existence before
  deciding to delete it (this check will always work properly)

Bug: 4409692

Change-Id: I106ed2fe88d2435ad7a462fced5cb307c2559fd6

emailcommon/src/com/android/emailcommon/provider/EmailContent.java

@@ -1961,16 +1961,20 @@ public abstract class EmailContent {
         };
 
         // 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";
         // Attachments with an empty URI that are in an inbox
-        public static final String EMPTY_URI_INBOX_SELECTION =
-            EMPTY_URI_SELECTION + " AND " + AttachmentColumns.MESSAGE_KEY + " IN ("
+        public static final String PRECACHE_INBOX_SELECTION =
+            PRECACHE_SELECTION + " AND " + AttachmentColumns.MESSAGE_KEY + " IN ("
             +     "SELECT " + MessageColumns.ID + " FROM " + Message.TABLE_NAME
             +     " WHERE " + Message.INBOX_SELECTION
             +     ")";
 
         // 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
         // with this attachment.  This is only valid if there is one and only one attachment and
         // that attachment has this flag set

src/com/android/email/service/AttachmentDownloadService.java

@@ -342,7 +342,7 @@ public class AttachmentDownloadService extends Service implements Runnable {
                 Uri lookupUri = EmailContent.uriWithLimit(Attachment.CONTENT_URI,
                         MAX_ATTACHMENTS_TO_CHECK);
                 Cursor c = mContext.getContentResolver().query(lookupUri, AttachmentInfo.PROJECTION,
-                        EmailContent.Attachment.EMPTY_URI_INBOX_SELECTION,
+                        EmailContent.Attachment.PRECACHE_INBOX_SELECTION,
                         null, Attachment.RECORD_ID + " DESC");
                 File cacheDir = mContext.getCacheDir();
                 try {
@@ -481,6 +481,7 @@ public class AttachmentDownloadService extends Service implements Runnable {
             mWatchdogPendingIntent = PendingIntent.getBroadcast(context, 0, alarmIntent, 0);
             mAlarmManager = (AlarmManager)context.getSystemService(Context.ALARM_SERVICE);
         }
+
         private void cancelDownload(DownloadRequest req) {
             mDownloadsInProgress.remove(req.attachmentId);
             req.inProgress = false;
@@ -572,8 +573,15 @@ public class AttachmentDownloadService extends Service implements Runnable {
                     }
                 }
                 if (statusCode == EmailServiceStatus.MESSAGE_NOT_FOUND) {
-                    // If there's no associated message, delete the attachment
-                    EmailContent.delete(mContext, Attachment.CONTENT_URI, attachment.mId);
+                    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
+                        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

tests/src/com/android/email/provider/AttachmentProviderTests.java

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