POP3 renovation

* Much, much faster
* Remove message length pass and lots of other useless code
* Create pseudo-attachment for long messages (click to download) that
  includes size (so user can determine whether it's worth it)
* Handle download of message via pseudo-attachment; real attachments
  are then created as necessary.

TODO: Add real UI with UX input (or modify existing to clean up the
loose ends)
TODO: Optimizations for loading the whole message
TODO: Get server delete working (isn't working currently anyway)

Change-Id: I31f3809fc5a2f9fd490d33cfed70d2930654e71d

emailcommon/src/com/android/emailcommon/internet/MimeMessage.java

@@ -67,6 +67,7 @@ public class MimeMessage extends Message {
     private Body mBody;
     protected int mSize;
     private boolean mInhibitLocalMessageId = false;
+    private boolean mComplete = true;
 
     // Shared random source for generating local message-id values
     private static final java.util.Random sRandom = new java.util.Random();
@@ -135,6 +136,7 @@ public class MimeMessage extends Message {
         MimeStreamParser parser = new MimeStreamParser();
         parser.setContentHandler(new MimeMessageBuilder());
         parser.parse(new EOLConvertingInputStream(in));
+        mComplete = !parser.getPrematureEof();
     }
 
     /**
@@ -202,6 +204,10 @@ public class MimeMessage extends Message {
         }
     }
 
+    public boolean isComplete() {
+        return mComplete;
+    }
+
     public String getMimeType() throws MessagingException {
         return MimeUtility.getHeaderParameter(getContentType(), null);
     }

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

@@ -779,6 +779,7 @@ public abstract class EmailContent {
         public static final int FLAG_LOADED_COMPLETE = 1;
         public static final int FLAG_LOADED_PARTIAL = 2;
         public static final int FLAG_LOADED_DELETED = 3;
+        public static final int FLAG_LOADED_UNKNOWN = 4;
 
         // Bits used in mFlags
         // The following three states are mutually exclusive, and indicate whether the message is an

res/xml/services.xml

@@ -52,7 +52,7 @@
         email:protocol="pop3"
         email:name="POP3"
         email:accountType="com.android.email"
-        email:serviceClass="com.android.email.service.ImapService"
+        email:serviceClass="com.android.email.service.Pop3Service"
         email:port="110"
         email:portSsl="995"
         email:syncIntervalStrings="@array/account_settings_check_frequency_entries"
@@ -67,7 +67,7 @@
         email:protocol="imap"
         email:name="IMAP"
         email:accountType="com.android.email"
-        email:serviceClass="com.android.email.service.Pop3Service"
+        email:serviceClass="com.android.email.service.ImapService"
         email:port="143"
         email:portSsl="993"
         email:syncIntervalStrings="@array/account_settings_check_frequency_entries"

src/com/android/email/LegacyConversions.java

@@ -185,7 +185,7 @@ public class LegacyConversions {
      * @param part a single attachment part from POP or IMAP
      * @throws IOException
      */
-    private static void addOneAttachment(Context context, EmailContent.Message localMessage,
+    public static void addOneAttachment(Context context, EmailContent.Message localMessage,
             Part part) throws MessagingException, IOException {
 
         Attachment localAttachment = new Attachment();

src/com/android/email/activity/setup/AccountSetupOptions.java

@@ -28,6 +28,7 @@ import android.content.Context;
 import android.content.DialogInterface;
 import android.content.Intent;
 import android.os.Bundle;
+import android.os.RemoteException;
 import android.util.Log;
 import android.view.View;
 import android.view.View.OnClickListener;
@@ -45,6 +46,7 @@ import com.android.email2.ui.MailActivityEmail;
 import com.android.emailcommon.Logging;
 import com.android.emailcommon.provider.Account;
 import com.android.emailcommon.provider.Policy;
+import com.android.emailcommon.service.EmailServiceProxy;
 import com.android.emailcommon.service.SyncWindow;
 import com.android.emailcommon.utility.Utility;
 
@@ -203,7 +205,7 @@ public class AccountSetupOptions extends AccountSetupActivity implements OnClick
         if (mNotifyView.isChecked()) {
             newFlags |= Account.FLAGS_NOTIFY_NEW_MAIL;
         }
-        if (mBackgroundAttachmentsView.isChecked()) {
+        if (mServiceInfo.offerAttachmentPreload && mBackgroundAttachmentsView.isChecked()) {
             newFlags |= Account.FLAGS_BACKGROUND_ATTACHMENTS;
         }
         account.setFlags(newFlags);
@@ -332,6 +334,16 @@ public class AccountSetupOptions extends AccountSetupActivity implements OnClick
             return;
         }
         saveAccountAndFinish();
+
+        // If we're local, update the folder list (to get our starting folders, e.g. Inbox)
+        EmailServiceProxy proxy = EmailServiceUtils.getServiceForAccount(this, null, account.mId);
+        if (!proxy.isRemote()) {
+            try {
+                proxy.updateFolderList(account.mId);
+            } catch (RemoteException e) {
+                // It's all good
+            }
+        }
     }
 
     /**

src/com/android/email/mail/Store.java

@@ -43,9 +43,9 @@ import java.util.HashMap;
 public abstract class Store {
     /**
      * A global suggestion to Store implementors on how much of the body
-     * should be returned on FetchProfile.Item.BODY_SANE requests.
+     * should be returned on FetchProfile.Item.BODY_SANE requests. We'll use 125k now.
      */
-    public static final int FETCH_BODY_SANE_SUGGESTED_SIZE = (50 * 1024);
+    public static final int FETCH_BODY_SANE_SUGGESTED_SIZE = (125 * 1024);
 
     @VisibleForTesting
     static final HashMap<HostAuth, Store> sStores = new HashMap<HostAuth, Store>();

src/com/android/email/mail/store/Pop3Store.java

@@ -18,11 +18,13 @@ package com.android.email.mail.store;
 
 import android.content.Context;
 import android.os.Bundle;
+import android.os.RemoteException;
 import android.util.Log;
 
 import com.android.email.R;
 import com.android.email.mail.Store;
 import com.android.email.mail.transport.MailTransport;
+import com.android.email.service.EmailServiceUtils;
 import com.android.email2.ui.MailActivityEmail;
 import com.android.emailcommon.Logging;
 import com.android.emailcommon.internet.MimeMessage;
@@ -47,7 +49,6 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.util.ArrayList;
 import java.util.HashMap;
-import java.util.HashSet;
 
 public class Pop3Store extends Store {
     // All flags defining debug or development code settings must be FALSE
@@ -60,31 +61,6 @@ public class Pop3Store extends Store {
     private static final String POP3_MAILBOX_NAME = "INBOX";
     private final HashMap<String, Folder> mFolders = new HashMap<String, Folder>();
 
-//    /**
-//     * Detected latency, used for usage scaling.
-//     * Usage scaling occurs when it is necessary to get information about
-//     * messages that could result in large data loads. This value allows
-//     * the code that loads this data to decide between using large downloads
-//     * (high latency) or multiple round trips (low latency) to accomplish
-//     * the same thing.
-//     * Default is Integer.MAX_VALUE implying massive latency so that the large
-//     * download method is used by default until latency data is collected.
-//     */
-//    private int mLatencyMs = Integer.MAX_VALUE;
-//
-//    /**
-//     * Detected throughput, used for usage scaling.
-//     * Usage scaling occurs when it is necessary to get information about
-//     * messages that could result in large data loads. This value allows
-//     * the code that loads this data to decide between using large downloads
-//     * (high latency) or multiple round trips (low latency) to accomplish
-//     * the same thing.
-//     * Default is Integer.MAX_VALUE implying massive bandwidth so that the
-//     * large download method is used by default until latency data is
-//     * collected.
-//     */
-//    private int mThroughputKbS = Integer.MAX_VALUE;
-
     /**
      * Static named constructor.
      */
@@ -154,7 +130,8 @@ public class Pop3Store extends Store {
         for (int type : DEFAULT_FOLDERS) {
             if (Mailbox.findMailboxOfType(mContext, mAccount.mId, type) == Mailbox.NO_MAILBOX) {
                 String name = getMailboxServerName(mContext, type);
-                Mailbox.newSystemMailbox(mAccount.mId, type, name).save(mContext);
+                mailbox = Mailbox.newSystemMailbox(mAccount.mId, type, name);
+                mailbox.save(mContext);
             }
         }
 
@@ -216,7 +193,7 @@ public class Pop3Store extends Store {
         return bundle;
     }
 
-    class Pop3Folder extends Folder {
+    public class Pop3Folder extends Folder {
         private final HashMap<String, Pop3Message> mUidToMsgMap
                 = new HashMap<String, Pop3Message>();
         private final HashMap<Integer, Pop3Message> mMsgNumToMsgMap
@@ -245,24 +222,22 @@ public class Pop3Store extends Store {
         public Bundle checkSettings() throws MessagingException {
             Bundle bundle = new Bundle();
             int result = MessagingException.NO_ERROR;
-            if (!mCapabilities.uidl) {
+            try {
-                try {
+                UidlParser parser = new UidlParser();
-                    UidlParser parser = new UidlParser();
+                executeSimpleCommand("UIDL");
-                    executeSimpleCommand("UIDL");
+                // drain the entire output, so additional communications don't get confused.
-                    // drain the entire output, so additional communications don't get confused.
+                String response;
-                    String response;
+                while ((response = mTransport.readLine()) != null) {
-                    while ((response = mTransport.readLine()) != null) {
+                    parser.parseMultiLine(response);
-                        parser.parseMultiLine(response);
+                    if (parser.mEndOfMessage) {
-                        if (parser.mEndOfMessage) {
+                        break;
-                            break;
-                        }
                     }
-                } catch (IOException ioe) {
-                    mTransport.close();
-                    result = MessagingException.IOERROR;
-                    bundle.putString(EmailServiceProxy.VALIDATE_BUNDLE_ERROR_MESSAGE,
-                            ioe.getMessage());
                 }
+            } catch (IOException ioe) {
+                mTransport.close();
+                result = MessagingException.IOERROR;
+                bundle.putString(EmailServiceProxy.VALIDATE_BUNDLE_ERROR_MESSAGE,
+                        ioe.getMessage());
             }
             bundle.putInt(EmailServiceProxy.VALIDATE_BUNDLE_RESULT_CODE, result);
             return bundle;
@@ -413,7 +388,7 @@ public class Pop3Store extends Store {
         }
 
         @Override
-        public Message[] getMessages(int start, int end, MessageRetrievalListener listener)
+        public Pop3Message[] getMessages(int start, int end, MessageRetrievalListener listener)
                 throws MessagingException {
             if (start < 1 || end < 1 || end < start) {
                 throw new MessagingException(String.format("Invalid message set %d %d",
@@ -432,11 +407,8 @@ public class Pop3Store extends Store {
             for (int msgNum = start; msgNum <= end; msgNum++) {
                 Pop3Message message = mMsgNumToMsgMap.get(msgNum);
                 messages.add(message);
-                if (listener != null) {
-                    listener.messageRetrieved(message);
-                }
             }
-            return messages.toArray(new Message[messages.size()]);
+            return messages.toArray(new Pop3Message[messages.size()]);
         }
 
         /**
@@ -497,39 +469,6 @@ public class Pop3Store extends Store {
             }
         }
 
-        private void indexUids(ArrayList<String> uids)
-                throws MessagingException, IOException {
-            HashSet<String> unindexedUids = new HashSet<String>();
-            for (String uid : uids) {
-                if (mUidToMsgMap.get(uid) == null) {
-                    unindexedUids.add(uid);
-                }
-            }
-            if (unindexedUids.size() == 0) {
-                return;
-            }
-            /*
-             * If we are missing uids in the cache the only sure way to
-             * get them is to do a full UIDL list. A possible optimization
-             * would be trying UIDL for the latest X messages and praying.
-             */
-            UidlParser parser = new UidlParser();
-            String response = executeSimpleCommand("UIDL");
-            while ((response = mTransport.readLine()) != null) {
-                parser.parseMultiLine(response);
-                if (parser.mEndOfMessage) {
-                    break;
-                }
-                if (unindexedUids.contains(parser.mUniqueId)) {
-                    Pop3Message message = mUidToMsgMap.get(parser.mUniqueId);
-                    if (message == null) {
-                        message = new Pop3Message(parser.mUniqueId, this);
-                    }
-                    indexMessage(parser.mMessageNumber, message);
-                }
-            }
-        }
-
         /**
          * Simple parser class for UIDL messages.
          *
@@ -651,130 +590,8 @@ public class Pop3Store extends Store {
         @Override
         public void fetch(Message[] messages, FetchProfile fp, MessageRetrievalListener listener)
                 throws MessagingException {
-            if (messages == null || messages.length == 0) {
+            throw new UnsupportedOperationException(
-                return;
+                    "Pop3Folder.fetch(Message[], FetchProfile, MessageRetrievalListener)");
-            }
-            ArrayList<String> uids = new ArrayList<String>();
-            for (Message message : messages) {
-                uids.add(message.getUid());
-            }
-            try {
-                indexUids(uids);
-                if (fp.contains(FetchProfile.Item.ENVELOPE)) {
-                    // Note: We never pass the listener for the ENVELOPE call, because we're going
-                    // to be calling the listener below in the per-message loop.
-                    fetchEnvelope(messages, null);
-                }
-            } catch (IOException ioe) {
-                mTransport.close();
-                if (MailActivityEmail.DEBUG) {
-                    Log.d(Logging.LOG_TAG, ioe.toString());
-                }
-                throw new MessagingException("fetch", ioe);
-            }
-            for (int i = 0, count = messages.length; i < count; i++) {
-                Message message = messages[i];
-                if (!(message instanceof Pop3Message)) {
-                    throw new MessagingException("Pop3Store.fetch called with non-Pop3 Message");
-                }
-                Pop3Message pop3Message = (Pop3Message)message;
-                try {
-                    if (fp.contains(FetchProfile.Item.BODY)) {
-                        fetchBody(pop3Message, -1);
-                    }
-                    else if (fp.contains(FetchProfile.Item.BODY_SANE)) {
-                        /*
-                         * To convert the suggested download size we take the size
-                         * divided by the maximum line size (76).
-                         */
-                        fetchBody(pop3Message,
-                                FETCH_BODY_SANE_SUGGESTED_SIZE / 76);
-                    }
-                    else if (fp.contains(FetchProfile.Item.STRUCTURE)) {
-                        /*
-                         * If the user is requesting STRUCTURE we are required to set the body
-                         * to null since we do not support the function.
-                         */
-                        pop3Message.setBody(null);
-                    }
-                    if (listener != null) {
-                        listener.messageRetrieved(message);
-                    }
-                } catch (IOException ioe) {
-                    mTransport.close();
-                    if (MailActivityEmail.DEBUG) {
-                        Log.d(Logging.LOG_TAG, ioe.toString());
-                    }
-                    throw new MessagingException("Unable to fetch message", ioe);
-                }
-            }
-        }
-
-        private void fetchEnvelope(Message[] messages,
-                MessageRetrievalListener listener)  throws IOException, MessagingException {
-            int unsizedMessages = 0;
-            for (Message message : messages) {
-                if (message.getSize() == -1) {
-                    unsizedMessages++;
-                }
-            }
-            if (unsizedMessages == 0) {
-                return;
-            }
-            if (unsizedMessages < 50 && mMessageCount > 5000) {
-                /*
-                 * In extreme cases we'll do a command per message instead of a bulk request
-                 * to hopefully save some time and bandwidth.
-                 */
-                for (int i = 0, count = messages.length; i < count; i++) {
-                    Message message = messages[i];
-                    if (!(message instanceof Pop3Message)) {
-                        throw new MessagingException(
-                                "Pop3Store.fetch called with non-Pop3 Message");
-                    }
-                    Pop3Message pop3Message = (Pop3Message)message;
-                    String response = executeSimpleCommand(String.format("LIST %d",
-                            mUidToMsgNumMap.get(pop3Message.getUid())));
-                    try {
-                        String[] listParts = response.split(" ");
-                        int msgNum = Integer.parseInt(listParts[1]);
-                        int msgSize = Integer.parseInt(listParts[2]);
-                        pop3Message.setSize(msgSize);
-                    } catch (NumberFormatException nfe) {
-                        throw new IOException();
-                    }
-                    if (listener != null) {
-                        listener.messageRetrieved(pop3Message);
-                    }
-                }
-            } else {
-                HashSet<String> msgUidIndex = new HashSet<String>();
-                for (Message message : messages) {
-                    msgUidIndex.add(message.getUid());
-                }
-                String response = executeSimpleCommand("LIST");
-                while ((response = mTransport.readLine()) != null) {
-                    if (response.equals(".")) {
-                        break;
-                    }
-                    Pop3Message pop3Message = null;
-                    int msgSize = 0;
-                    try {
-                        String[] listParts = response.split(" ");
-                        int msgNum = Integer.parseInt(listParts[0]);
-                        msgSize = Integer.parseInt(listParts[1]);
-                        pop3Message = mMsgNumToMsgMap.get(msgNum);
-                    } catch (NumberFormatException nfe) {
-                        throw new IOException();
-                    }
-                    if (pop3Message != null && msgUidIndex.contains(pop3Message.getUid())) {
-                        pop3Message.setSize(msgSize);
-                        if (listener != null) {
-                            listener.messageRetrieved(pop3Message);
-                        }
-                    }
-                }
-            }
         }
 
         /**
@@ -791,7 +608,7 @@ public class Pop3Store extends Store {
          * @param message
          * @param lines
          */
-        private void fetchBody(Pop3Message message, int lines)
+        public void fetchBody(Pop3Message message, int lines)
                 throws IOException, MessagingException {
             String response = null;
             int messageId = mUidToMsgNumMap.get(message.getUid());
@@ -808,6 +625,22 @@ public class Pop3Store extends Store {
             }
             if (response != null)  {
                 try {
+                    int ok = response.indexOf("OK");
+                    if (ok > 0) {
+                        try {
+                            int start = ok + 3;
+                            int end = response.indexOf(" ", start);
+                            String intString;
+                            if (end > 0) {
+                                intString = response.substring(start, end);
+                            } else {
+                                intString = response.substring(start);
+                            }
+                            message.setSize(Integer.parseInt(intString));
+                        } catch (NumberFormatException e) {
+                            // We tried
+                        }
+                    }
                     InputStream in = mTransport.getInputStream();
                     if (DEBUG_LOG_RAW_STREAM && MailActivityEmail.DEBUG) {
                         in = new LoggingInputStream(in);
@@ -875,13 +708,6 @@ public class Pop3Store extends Store {
             throw new UnsupportedOperationException("copyMessages is not supported in POP3");
         }
 
-//        private boolean isRoundTripModeSuggested() {
-//            long roundTripMethodMs =
-//                (uncachedMessageCount * 2 * mLatencyMs);
-//            long bulkMethodMs =
-//                    (mMessageCount * 58) / (mThroughputKbS * 1024 / 8) * 1000;
-//        }
-
         private Pop3Capabilities getCapabilities() throws IOException {
             Pop3Capabilities capabilities = new Pop3Capabilities();
             try {
@@ -889,22 +715,9 @@ public class Pop3Store extends Store {
                 while ((response = mTransport.readLine()) != null) {
                     if (response.equals(".")) {
                         break;
-                    }
+                    } else if (response.equalsIgnoreCase("STLS")){
-                    if (response.equalsIgnoreCase("STLS")){
                         capabilities.stls = true;
                     }
-                    else if (response.equalsIgnoreCase("UIDL")) {
-                        capabilities.uidl = true;
-                    }
-                    else if (response.equalsIgnoreCase("PIPELINING")) {
-                        capabilities.pipelining = true;
-                    }
-                    else if (response.equalsIgnoreCase("USER")) {
-                        capabilities.user = true;
-                    }
-                    else if (response.equalsIgnoreCase("TOP")) {
-                        capabilities.top = true;
-                    }
                 }
             }
             catch (MessagingException me) {
@@ -1008,23 +821,10 @@ public class Pop3Store extends Store {
     class Pop3Capabilities {
         /** The STLS (start TLS) command is supported */
         public boolean stls;
-        /** the TOP command (retrieve a partial message) is supported */
-        public boolean top;
-        /** USER and PASS login/auth commands are supported */
-        public boolean user;
-        /** the optional UIDL command is supported (unused) */
-        public boolean uidl;
-        /** the server is capable of accepting multiple commands at a time (unused) */
-        public boolean pipelining;
 
         @Override
         public String toString() {
-            return String.format("STLS %b, TOP %b, USER %b, UIDL %b, PIPELINING %b",
+            return String.format("STLS %b", stls);
-                    stls,
-                    top,
-                    user,
-                    uidl,
-                    pipelining);
         }
     }
 

src/com/android/email/provider/EmailProvider.java

@@ -893,6 +893,8 @@ public class EmailProvider extends ContentProvider {
                         resolver.notifyChange(UIPROVIDER_ALL_ACCOUNTS_NOTIFIER, null);
                     } else if (match == MAILBOX_ID) {
                         notifyUI(UIPROVIDER_FOLDER_NOTIFIER, id);
+                    } else if (match == ATTACHMENT_ID) {
+                        notifyUI(UIPROVIDER_ATTACHMENT_NOTIFIER, id);
                     }
                     break;
                 case ATTACHMENTS_MESSAGE_ID:
@@ -2414,15 +2416,6 @@ outer:
         ContentValues values = new ContentValues();
         String attachmentJson = null;
         if (msg != null) {
-            if (msg.mFlagLoaded == Message.FLAG_LOADED_PARTIAL) {
-                EmailServiceProxy service =
-                        EmailServiceUtils.getServiceForAccount(context, null, msg.mAccountKey);
-                try {
-                    service.loadMore(messageId);
-                } catch (RemoteException e) {
-                    // Nothing to do
-                }
-            }
             Body body = Body.restoreBodyWithMessageId(context, messageId);
             if (body != null) {
                 if (body.mHtmlContent != null) {

src/com/android/email/provider/Utilities.java

@@ -25,6 +25,7 @@ import android.util.Log;
 
 import com.android.email.LegacyConversions;
 import com.android.emailcommon.Logging;
+import com.android.emailcommon.internet.MimeBodyPart;
 import com.android.emailcommon.internet.MimeUtility;
 import com.android.emailcommon.mail.Message;
 import com.android.emailcommon.mail.MessagingException;
@@ -35,6 +36,7 @@ import com.android.emailcommon.provider.EmailContent.MessageColumns;
 import com.android.emailcommon.provider.EmailContent.SyncColumns;
 import com.android.emailcommon.provider.Mailbox;
 import com.android.emailcommon.utility.ConversionUtilities;
+import com.android.mail.providers.Attachment;
 
 import java.io.IOException;
 import java.util.ArrayList;
@@ -59,20 +61,24 @@ public class Utilities {
                     EmailContent.Message.CONTENT_URI,
                     EmailContent.Message.CONTENT_PROJECTION,
                     EmailContent.MessageColumns.ACCOUNT_KEY + "=?" +
-                    " AND " + MessageColumns.MAILBOX_KEY + "=?" +
+                            " AND " + MessageColumns.MAILBOX_KEY + "=?" +
-                    " AND " + SyncColumns.SERVER_ID + "=?",
+                            " AND " + SyncColumns.SERVER_ID + "=?",
-                    new String[] {
+                            new String[] {
                             String.valueOf(account.mId),
                             String.valueOf(folder.mId),
                             String.valueOf(message.getUid())
                     },
                     null);
-            if (c.moveToNext()) {
+            if (c == null) {
+                return;
+            } else if (c.moveToNext()) {
                 localMessage = EmailContent.getContent(c, EmailContent.Message.class);
-                localMessage.mMailboxKey = folder.mId;
+            } else {
-                localMessage.mAccountKey = account.mId;
+                localMessage = new EmailContent.Message();
-                copyOneMessageToProvider(context, message, localMessage, loadStatus);
             }
+            localMessage.mMailboxKey = folder.mId;
+            localMessage.mAccountKey = account.mId;
+            copyOneMessageToProvider(context, message, localMessage, loadStatus);
         } finally {
             if (c != null) {
                 c.close();
@@ -94,8 +100,10 @@ public class Utilities {
             EmailContent.Message localMessage, int loadStatus) {
         try {
 
-            EmailContent.Body body = EmailContent.Body.restoreBodyWithMessageId(context,
+            EmailContent.Body body = null;
-                    localMessage.mId);
+            if (localMessage.mId != EmailContent.Message.NO_MESSAGE) {
+                body = EmailContent.Body.restoreBodyWithMessageId(context, localMessage.mId);
+            }
             if (body == null) {
                 body = new EmailContent.Body();
             }
@@ -113,10 +121,23 @@ public class Utilities {
 
                 // Commit the message & body to the local store immediately
                 saveOrUpdate(localMessage, context);
+                body.mMessageKey = localMessage.mId;
                 saveOrUpdate(body, context);
 
                 // process (and save) attachments
-                LegacyConversions.updateAttachments(context, localMessage, attachments);
+                if (loadStatus != EmailContent.Message.FLAG_LOADED_UNKNOWN) {
+                    LegacyConversions.updateAttachments(context, localMessage, attachments);
+                } else {
+                    EmailContent.Attachment att = new EmailContent.Attachment();
+                    // STOPSHIP: Redo UI or localize
+                    att.mFileName = "Click to load entire message";
+                    att.mSize = message.getSize();
+                    att.mMimeType = "text/plain";
+                    att.mMessageKey = localMessage.mId;
+                    att.mAccountKey = localMessage.mAccountKey;
+                    att.save(context);
+                    localMessage.mFlagAttachment = true;
+                }
 
                 // One last update of message with two updated flags
                 localMessage.mFlagLoaded = loadStatus;

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

@@ -87,7 +87,7 @@ public abstract class EmailServiceStub extends IEmailService.Stub implements IEm
         MailboxColumns.TYPE,
     };
 
-    private Context mContext;
+    protected Context mContext;
     private IEmailServiceCallback.Stub mCallback;
 
     protected void init(Context context, IEmailServiceCallback.Stub callbackProxy) {

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

@@ -30,42 +30,42 @@ import android.os.RemoteCallbackList;
 import android.os.RemoteException;
 import android.util.Log;
 
-import com.android.email.LegacyConversions;
 import com.android.email.NotificationController;
 import com.android.email.mail.Store;
+import com.android.email.mail.store.Pop3Store;
+import com.android.email.mail.store.Pop3Store.Pop3Folder;
+import com.android.email.mail.store.Pop3Store.Pop3Message;
 import com.android.email.provider.Utilities;
 import com.android.email2.ui.MailActivityEmail;
 import com.android.emailcommon.Logging;
 import com.android.emailcommon.TrafficFlags;
-import com.android.emailcommon.internet.MimeUtility;
 import com.android.emailcommon.mail.AuthenticationFailedException;
-import com.android.emailcommon.mail.FetchProfile;
-import com.android.emailcommon.mail.Flag;
-import com.android.emailcommon.mail.Folder;
-import com.android.emailcommon.mail.Folder.FolderType;
-import com.android.emailcommon.mail.Folder.MessageRetrievalListener;
 import com.android.emailcommon.mail.Folder.OpenMode;
-import com.android.emailcommon.mail.Message;
 import com.android.emailcommon.mail.MessagingException;
-import com.android.emailcommon.mail.Part;
 import com.android.emailcommon.provider.Account;
 import com.android.emailcommon.provider.EmailContent;
+import com.android.emailcommon.provider.EmailContent.Attachment;
+import com.android.emailcommon.provider.EmailContent.AttachmentColumns;
 import com.android.emailcommon.provider.EmailContent.MailboxColumns;
+import com.android.emailcommon.provider.EmailContent.Message;
 import com.android.emailcommon.provider.EmailContent.MessageColumns;
 import com.android.emailcommon.provider.EmailContent.SyncColumns;
 import com.android.emailcommon.provider.Mailbox;
+import com.android.emailcommon.service.EmailServiceCallback;
 import com.android.emailcommon.service.EmailServiceStatus;
 import com.android.emailcommon.service.IEmailServiceCallback;
 import com.android.emailcommon.utility.AttachmentUtilities;
+import com.android.mail.providers.UIProvider;
 import com.android.mail.providers.UIProvider.AccountCapabilities;
+import com.android.mail.providers.UIProvider.AttachmentState;
 
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.HashSet;
 
 public class Pop3Service extends Service {
     private static final String TAG = "Pop3Service";
-    private static final int MAX_SMALL_MESSAGE_SIZE = (25 * 1024);
 
     @Override
     public int onStartCommand(Intent intent, int flags, int startId) {
@@ -76,102 +76,8 @@ public class Pop3Service extends Service {
     private static final RemoteCallbackList<IEmailServiceCallback> mCallbackList =
             new RemoteCallbackList<IEmailServiceCallback>();
 
-    private interface ServiceCallbackWrapper {
+    private static final EmailServiceCallback sCallbackProxy =
-        public void call(IEmailServiceCallback cb) throws RemoteException;
+            new EmailServiceCallback(mCallbackList);
-    }
-
-    /**
-     * Proxy that can be used by various sync adapters to tie into ExchangeService's callback system
-     * Used this way:  ExchangeService.callback().callbackMethod(args...);
-     * The proxy wraps checking for existence of a ExchangeService instance
-     * Failures of these callbacks can be safely ignored.
-     */
-    static private final IEmailServiceCallback.Stub sCallbackProxy =
-            new IEmailServiceCallback.Stub() {
-
-        /**
-         * Broadcast a callback to the everyone that's registered
-         *
-         * @param wrapper the ServiceCallbackWrapper used in the broadcast
-         */
-        private synchronized void broadcastCallback(ServiceCallbackWrapper wrapper) {
-            RemoteCallbackList<IEmailServiceCallback> callbackList = mCallbackList;
-            if (callbackList != null) {
-                // Call everyone on our callback list
-                int count = callbackList.beginBroadcast();
-                try {
-                    for (int i = 0; i < count; i++) {
-                        try {
-                            wrapper.call(callbackList.getBroadcastItem(i));
-                        } catch (RemoteException e) {
-                            // Safe to ignore
-                        } catch (RuntimeException e) {
-                            // We don't want an exception in one call to prevent other calls, so
-                            // we'll just log this and continue
-                            Log.e(TAG, "Caught RuntimeException in broadcast", e);
-                        }
-                    }
-                } finally {
-                    // No matter what, we need to finish the broadcast
-                    callbackList.finishBroadcast();
-                }
-            }
-        }
-
-        @Override
-        public void loadAttachmentStatus(final long messageId, final long attachmentId,
-                final int status, final int progress) {
-            broadcastCallback(new ServiceCallbackWrapper() {
-                @Override
-                public void call(IEmailServiceCallback cb) throws RemoteException {
-                    cb.loadAttachmentStatus(messageId, attachmentId, status, progress);
-                }
-            });
-        }
-
-        @Override
-        public void loadMessageStatus(final long messageId, final int status, final int progress) {
-            broadcastCallback(new ServiceCallbackWrapper() {
-                @Override
-                public void call(IEmailServiceCallback cb) throws RemoteException {
-                    cb.loadMessageStatus(messageId, status, progress);
-                }
-            });
-        }
-
-        @Override
-        public void sendMessageStatus(final long messageId, final String subject, final int status,
-                final int progress) {
-            broadcastCallback(new ServiceCallbackWrapper() {
-                @Override
-                public void call(IEmailServiceCallback cb) throws RemoteException {
-                    cb.sendMessageStatus(messageId, subject, status, progress);
-                }
-            });
-        }
-
-        @Override
-        public void syncMailboxListStatus(final long accountId, final int status,
-                final int progress) {
-            broadcastCallback(new ServiceCallbackWrapper() {
-                @Override
-                public void call(IEmailServiceCallback cb) throws RemoteException {
-                    cb.syncMailboxListStatus(accountId, status, progress);
-                }
-            });
-        }
-
-        @Override
-        public void syncMailboxStatus(final long mailboxId, final int status,
-                final int progress) {
-            broadcastCallback(new ServiceCallbackWrapper() {
-                @Override
-                public void call(IEmailServiceCallback cb) throws RemoteException {
-                    cb.syncMailboxStatus(mailboxId, status, progress);
-                }
-            });
-        }
-    };
 
     /**
      * Create our EmailService implementation here.
@@ -187,6 +93,16 @@ public class Pop3Service extends Service {
         public int getCapabilities(Account acct) throws RemoteException {
             return AccountCapabilities.UNDO;
         }
+
+        @Override
+        public void loadAttachment(long attachmentId, 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);
+            if (inboxId == Mailbox.NO_MAILBOX) return;
+            // We load attachments during a sync
+            startSync(inboxId, true);
+        }
     };
 
     @Override
@@ -196,16 +112,14 @@ public class Pop3Service extends Service {
     }
 
     private static void sendMailboxStatus(Mailbox mailbox, int status) {
-        try {
             sCallbackProxy.syncMailboxStatus(mailbox.mId, status, 0);
-        } catch (RemoteException e) {
-        }
     }
 
     /**
-     * Start foreground synchronization of the specified folder. This is called by
+     * Start foreground synchronization of the specified folder. This is called
-     * synchronizeMailbox or checkMail.
+     * by synchronizeMailbox or checkMail. TODO this should use ID's instead of
-     * TODO this should use ID's instead of fully-restored objects
+     * fully-restored objects
+     *
      * @param account
      * @param folder
      * @throws MessagingException
@@ -221,7 +135,7 @@ public class Pop3Service extends Service {
         NotificationController nc = NotificationController.getInstance(context);
         try {
             processPendingActionsSynchronous(context, account);
-            synchronizeMailboxGeneric(context, account, folder);
+            synchronizePop3Mailbox(context, account, folder);
             // Clear authentication notification for this account
             nc.cancelLoginFailedNotification(account.mId);
             sendMailboxStatus(folder, EmailServiceStatus.SUCCESS);
@@ -239,52 +153,34 @@ public class Pop3Service extends Service {
     }
 
     /**
-     * Lightweight record for the first pass of message sync, where I'm just seeing if
+     * Lightweight record for the first pass of message sync, where I'm just
-     * the local message requires sync.  Later (for messages that need syncing) we'll do a full
+     * seeing if the local message requires sync. Later (for messages that need
-     * readout from the DB.
+     * syncing) we'll do a full readout from the DB.
      */
     private static class LocalMessageInfo {
         private static final int COLUMN_ID = 0;
-        private static final int COLUMN_FLAG_READ = 1;
+        private static final int COLUMN_FLAG_LOADED = 1;
-        private static final int COLUMN_FLAG_FAVORITE = 2;
+        private static final int COLUMN_SERVER_ID = 2;
-        private static final int COLUMN_FLAG_LOADED = 3;
-        private static final int COLUMN_SERVER_ID = 4;
-        private static final int COLUMN_FLAGS =  7;
         private static final String[] PROJECTION = new String[] {
-            EmailContent.RECORD_ID,
+                EmailContent.RECORD_ID, MessageColumns.FLAG_LOADED, SyncColumns.SERVER_ID
-            MessageColumns.FLAG_READ, MessageColumns.FLAG_FAVORITE, MessageColumns.FLAG_LOADED,
-            SyncColumns.SERVER_ID, MessageColumns.MAILBOX_KEY, MessageColumns.ACCOUNT_KEY,
-            MessageColumns.FLAGS
         };
 
         final long mId;
-        final boolean mFlagRead;
-        final boolean mFlagFavorite;
         final int mFlagLoaded;
         final String mServerId;
-        final int mFlags;
 
         public LocalMessageInfo(Cursor c) {
             mId = c.getLong(COLUMN_ID);
-            mFlagRead = c.getInt(COLUMN_FLAG_READ) != 0;
-            mFlagFavorite = c.getInt(COLUMN_FLAG_FAVORITE) != 0;
             mFlagLoaded = c.getInt(COLUMN_FLAG_LOADED);
             mServerId = c.getString(COLUMN_SERVER_ID);
-            mFlags = c.getInt(COLUMN_FLAGS);
+            // Note: mailbox key and account key not needed - they are projected
-            // Note: mailbox key and account key not needed - they are projected for the SELECT
+            // for the SELECT
-        }
-    }
-
-    private static void saveOrUpdate(EmailContent content, Context context) {
-        if (content.isSaved()) {
-            content.update(context, content.toContentValues());
-        } else {
-            content.save(context);
         }
     }
 
     /**
      * Load the structure and body of messages not yet synced
+     *
      * @param account the account we're syncing
      * @param remoteFolder the (open) Folder we're working on
      * @param unsyncedMessages an array of Message's we've got headers for
@@ -292,184 +188,50 @@ public class Pop3Service extends Service {
      * @throws MessagingException
      */
     static void loadUnsyncedMessages(final Context context, final Account account,
-            Folder remoteFolder, ArrayList<Message> unsyncedMessages, final Mailbox toMailbox)
+            Pop3Folder remoteFolder, ArrayList<Pop3Message> unsyncedMessages,
-            throws MessagingException {
+            final Mailbox toMailbox) throws MessagingException {
-
+        if (MailActivityEmail.DEBUG) {
-        // 1. Divide the unsynced messages into small & large (by size)
+            Log.d(TAG, "Loading " + unsyncedMessages.size() + " unsynced messages");
-
-        // TODO doing this work here (synchronously) is problematic because it prevents the UI
-        // from affecting the order (e.g. download a message because the user requested it.)  Much
-        // of this logic should move out to a different sync loop that attempts to update small
-        // groups of messages at a time, as a background task.  However, we can't just return
-        // (yet) because POP messages don't have an envelope yet....
-
-        ArrayList<Message> largeMessages = new ArrayList<Message>();
-        ArrayList<Message> smallMessages = new ArrayList<Message>();
-        for (Message message : unsyncedMessages) {
-            if (message.getSize() > (MAX_SMALL_MESSAGE_SIZE)) {
-                largeMessages.add(message);
-            } else {
-                smallMessages.add(message);
-            }
         }
-
+        try {
-        // 2. Download small messages
+            int cnt = unsyncedMessages.size();
-
+            // We'll load them from most recent to oldest
-        // TODO Problems with this implementation.  1. For IMAP, where we get a real envelope,
+            for (int i = cnt - 1; i >= 0; i--) {
-        // this is going to be inefficient and duplicate work we've already done.  2.  It's going
+                Pop3Message message = unsyncedMessages.get(i);
-        // back to the DB for a local message that we already had (and discarded).
+                remoteFolder.fetchBody(message, Pop3Store.FETCH_BODY_SANE_SUGGESTED_SIZE / 76);
-
+                int flag = EmailContent.Message.FLAG_LOADED_COMPLETE;
-        // For small messages, we specify "body", which returns everything (incl. attachments)
+                if (!message.isComplete()) {
-        FetchProfile fp = new FetchProfile();
+                     flag = EmailContent.Message.FLAG_LOADED_UNKNOWN;
-        fp.add(FetchProfile.Item.BODY);
-        remoteFolder.fetch(smallMessages.toArray(new Message[smallMessages.size()]), fp,
-                new MessageRetrievalListener() {
-                    @Override
-                    public void messageRetrieved(Message message) {
-                        // Store the updated message locally and mark it fully loaded
-                        Utilities.copyOneMessageToProvider(context, message, account, toMailbox,
-                                EmailContent.Message.FLAG_LOADED_COMPLETE);
-                    }
-
-                    @Override
-                    public void loadAttachmentProgress(int progress) {
-                    }
-        });
-
-        // 3. Download large messages.  We ask the server to give us the message structure,
-        // but not all of the attachments.
-        fp.clear();
-        fp.add(FetchProfile.Item.STRUCTURE);
-        remoteFolder.fetch(largeMessages.toArray(new Message[largeMessages.size()]), fp, null);
-        for (Message message : largeMessages) {
-            if (message.getBody() == null) {
-                // POP doesn't support STRUCTURE mode, so we'll just do a partial download
-                // (hopefully enough to see some/all of the body) and mark the message for
-                // further download.
-                fp.clear();
-                fp.add(FetchProfile.Item.BODY_SANE);
-                //  TODO a good optimization here would be to make sure that all Stores set
-                //  the proper size after this fetch and compare the before and after size. If
-                //  they equal we can mark this SYNCHRONIZED instead of PARTIALLY_SYNCHRONIZED
-                remoteFolder.fetch(new Message[] { message }, fp, null);
-
-                // Store the partially-loaded message and mark it partially loaded
-                Utilities.copyOneMessageToProvider(context, message, account, toMailbox,
-                        EmailContent.Message.FLAG_LOADED_PARTIAL);
-            } else {
-                // We have a structure to deal with, from which
-                // we can pull down the parts we want to actually store.
-                // Build a list of parts we are interested in. Text parts will be downloaded
-                // right now, attachments will be left for later.
-                ArrayList<Part> viewables = new ArrayList<Part>();
-                ArrayList<Part> attachments = new ArrayList<Part>();
-                MimeUtility.collectParts(message, viewables, attachments);
-                // Download the viewables immediately
-                for (Part part : viewables) {
-                    fp.clear();
-                    fp.add(part);
-                    // TODO what happens if the network connection dies? We've got partial
-                    // messages with incorrect status stored.
-                    remoteFolder.fetch(new Message[] { message }, fp, null);
                 }
-                // Store the updated message locally and mark it fully loaded
+                if (MailActivityEmail.DEBUG) {
-                Utilities.copyOneMessageToProvider(context, message, account, toMailbox,
+                    Log.d(TAG, "Message is " + (message.isComplete() ? "" : "NOT ") + "complete");
-                        EmailContent.Message.FLAG_LOADED_COMPLETE);
+                }
+                // If message is incomplete, create a "fake" attachment
+                Utilities.copyOneMessageToProvider(context, message, account, toMailbox, flag);
             }
+        } catch (IOException e) {
+            throw new MessagingException(MessagingException.IOERROR);
         }
-
-    }
-
-    public static void downloadFlagAndEnvelope(final Context context, final Account account,
-            final Mailbox mailbox, Folder remoteFolder, ArrayList<Message> unsyncedMessages,
-            HashMap<String, LocalMessageInfo> localMessageMap, final ArrayList<Long> unseenMessages)
-            throws MessagingException {
-        FetchProfile fp = new FetchProfile();
-        fp.add(FetchProfile.Item.FLAGS);
-        fp.add(FetchProfile.Item.ENVELOPE);
-
-        final HashMap<String, LocalMessageInfo> localMapCopy;
-        if (localMessageMap != null)
-            localMapCopy = new HashMap<String, LocalMessageInfo>(localMessageMap);
-        else {
-            localMapCopy = new HashMap<String, LocalMessageInfo>();
-        }
-
-        remoteFolder.fetch(unsyncedMessages.toArray(new Message[0]), fp,
-                new MessageRetrievalListener() {
-                    @Override
-                    public void messageRetrieved(Message message) {
-                        try {
-                            // Determine if the new message was already known (e.g. partial)
-                            // And create or reload the full message info
-                            LocalMessageInfo localMessageInfo =
-                                localMapCopy.get(message.getUid());
-                            EmailContent.Message localMessage = null;
-                            if (localMessageInfo == null) {
-                                localMessage = new EmailContent.Message();
-                            } else {
-                                localMessage = EmailContent.Message.restoreMessageWithId(
-                                        context, localMessageInfo.mId);
-                            }
-
-                            if (localMessage != null) {
-                                try {
-                                    // Copy the fields that are available into the message
-                                    LegacyConversions.updateMessageFields(localMessage,
-                                            message, account.mId, mailbox.mId);
-                                    // Commit the message to the local store
-                                    saveOrUpdate(localMessage, context);
-                                    // Track the "new" ness of the downloaded message
-                                    if (!message.isSet(Flag.SEEN) && unseenMessages != null) {
-                                        unseenMessages.add(localMessage.mId);
-                                    }
-                                } catch (MessagingException me) {
-                                    Log.e(Logging.LOG_TAG,
-                                            "Error while copying downloaded message." + me);
-                                }
-
-                            }
-                        }
-                        catch (Exception e) {
-                            Log.e(Logging.LOG_TAG,
-                                    "Error while storing downloaded message." + e.toString());
-                        }
-                    }
-
-                    @Override
-                    public void loadAttachmentProgress(int progress) {
-                    }
-                });
-
     }
 
     /**
-     * Synchronizer for IMAP.
+     * Synchronizer
-     *
-     * TODO Break this method up into smaller chunks.
      *
      * @param account the account to sync
      * @param mailbox the mailbox to sync
-     * @return results of the sync pass
      * @throws MessagingException
      */
-    private static void synchronizeMailboxGeneric(final Context context,
+    private static void synchronizePop3Mailbox(final Context context,
             final Account account, final Mailbox mailbox) throws MessagingException {
 
-        /*
-         * A list of IDs for messages that were downloaded and did not have the seen flag set.
-         * This serves as the "true" new message count reported to the user via notification.
-         */
-        final ArrayList<Long> unseenMessages = new ArrayList<Long>();
-
         ContentResolver resolver = context.getContentResolver();
 
-        // 0.  We do not ever sync DRAFTS or OUTBOX (down or up)
+        // We only sync Inbox
-        if (mailbox.mType == Mailbox.TYPE_DRAFTS || mailbox.mType == Mailbox.TYPE_OUTBOX) {
+        if (mailbox.mType != Mailbox.TYPE_INBOX) {
             return;
         }
 
-        // 1.  Get the message list from the local store and create an index of the uids
+        // Get the message list from EmailProvider and create an index of the uids
 
         Cursor localUidCursor = null;
         HashMap<String, LocalMessageInfo> localMessageMap = new HashMap<String, LocalMessageInfo>();
@@ -479,7 +241,7 @@ public class Pop3Service extends Service {
                     EmailContent.Message.CONTENT_URI,
                     LocalMessageInfo.PROJECTION,
                     EmailContent.MessageColumns.ACCOUNT_KEY + "=?" +
-                    " AND " + MessageColumns.MAILBOX_KEY + "=?",
+                            " AND " + MessageColumns.MAILBOX_KEY + "=?",
                     new String[] {
                             String.valueOf(account.mId),
                             String.valueOf(mailbox.mId)
@@ -495,51 +257,34 @@ public class Pop3Service extends Service {
             }
         }
 
-        // 2.  Open the remote folder and create the remote folder if necessary
+        // Open the remote folder and create the remote folder if necessary
 
-        Store remoteStore = Store.getInstance(account, context);
+        Pop3Store remoteStore = (Pop3Store)Store.getInstance(account, context);
         // The account might have been deleted
-        if (remoteStore == null) return;
+        if (remoteStore == null)
-        Folder remoteFolder = remoteStore.getFolder(mailbox.mServerId);
+            return;
+        Pop3Folder remoteFolder = (Pop3Folder)remoteStore.getFolder(mailbox.mServerId);
 
-        /*
+        // Open the remote folder. This pre-loads certain metadata like message
-         * If the folder is a "special" folder we need to see if it exists
+        // count.
-         * on the remote server. It if does not exist we'll try to create it. If we
-         * can't create we'll abort. This will happen on every single Pop3 folder as
-         * designed and on Imap folders during error conditions. This allows us
-         * to treat Pop3 and Imap the same in this code.
-         */
-        if (mailbox.mType == Mailbox.TYPE_TRASH || mailbox.mType == Mailbox.TYPE_SENT
-                || mailbox.mType == Mailbox.TYPE_DRAFTS) {
-            if (!remoteFolder.exists()) {
-                if (!remoteFolder.create(FolderType.HOLDS_MESSAGES)) {
-                    return;
-                }
-            }
-        }
-
-        // 3, Open the remote folder. This pre-loads certain metadata like message count.
         remoteFolder.open(OpenMode.READ_WRITE);
 
-        // 4. Trash any remote messages that are marked as trashed locally.
+        // Get the remote message count.
-        // TODO - this comment was here, but no code was here.
-
-        // 5. Get the remote message count.
         int remoteMessageCount = remoteFolder.getMessageCount();
         ContentValues values = new ContentValues();
         values.put(MailboxColumns.TOTAL_COUNT, remoteMessageCount);
         mailbox.update(context, values);
 
-        // 6. Determine the limit # of messages to download
+        // Determine the limit # of messages to download
         int visibleLimit = mailbox.mVisibleLimit;
         if (visibleLimit <= 0) {
             visibleLimit = MailActivityEmail.VISIBLE_LIMIT_DEFAULT;
         }
 
-        // 7.  Create a list of messages to download
+        // Create a list of messages to download
-        Message[] remoteMessages = new Message[0];
+        Pop3Message[] remoteMessages = new Pop3Message[0];
-        final ArrayList<Message> unsyncedMessages = new ArrayList<Message>();
+        final ArrayList<Pop3Message> unsyncedMessages = new ArrayList<Pop3Message>();
-        HashMap<String, Message> remoteUidMap = new HashMap<String, Message>();
+        HashMap<String, Pop3Message> remoteUidMap = new HashMap<String, Pop3Message>();
 
         if (remoteMessageCount > 0) {
             /*
@@ -548,27 +293,22 @@ public class Pop3Service extends Service {
             int remoteStart = Math.max(0, remoteMessageCount - visibleLimit) + 1;
             int remoteEnd = remoteMessageCount;
             remoteMessages = remoteFolder.getMessages(remoteStart, remoteEnd, null);
-            // TODO Why are we running through the list twice? Combine w/ for loop below
-            for (Message message : remoteMessages) {
-                remoteUidMap.put(message.getUid(), message);
-            }
 
             /*
-             * Get a list of the messages that are in the remote list but not on the
+             * Get a list of the messages that are in the remote list but not on
-             * local store, or messages that are in the local store but failed to download
+             * the local store, or messages that are in the local store but
-             * on the last sync. These are the new messages that we will download.
+             * failed to download on the last sync. These are the new messages
-             * Note, we also skip syncing messages which are flagged as "deleted message" sentinels,
+             * that we will download. Note, we also skip syncing messages which
-             * because they are locally deleted and we don't need or want the old message from
+             * are flagged as "deleted message" sentinels, because they are
+             * locally deleted and we don't need or want the old message from
              * the server.
              */
-            for (Message message : remoteMessages) {
+            for (Pop3Message message : remoteMessages) {
-                LocalMessageInfo localMessage = localMessageMap.get(message.getUid());
+                String uid = message.getUid();
+                remoteUidMap.put(uid, message);
+                LocalMessageInfo localMessage = localMessageMap.get(uid);
                 // localMessage == null -> message has never been created (not even headers)
                 // mFlagLoaded = UNLOADED -> message created, but none of body loaded
-                // mFlagLoaded = PARTIAL -> message created, a "sane" amt of body has been loaded
-                // mFlagLoaded = COMPLETE -> message body has been completely loaded
-                // mFlagLoaded = DELETED -> message has been deleted
-                // Only the first two of these are "unsynced", so let's retrieve them
                 if (localMessage == null ||
                         (localMessage.mFlagLoaded == EmailContent.Message.FLAG_LOADED_UNLOADED)) {
                     unsyncedMessages.add(message);
@@ -576,76 +316,63 @@ public class Pop3Service extends Service {
             }
         }
 
-        // 8.  Download basic info about the new/unloaded messages (if any)
+        // Get "attachments" to be loaded
-        /*
+        Cursor c = resolver.query(Attachment.CONTENT_URI, Attachment.CONTENT_PROJECTION,
-         * Fetch the flags and envelope only of the new messages. This is intended to get us
+                AttachmentColumns.ACCOUNT_KEY + "=? AND " +
-         * critical data as fast as possible, and then we'll fill in the details.
+                        AttachmentColumns.UI_STATE + "=" + AttachmentState.DOWNLOADING,
-         */
+                new String[] {Long.toString(account.mId)}, null);
-        if (unsyncedMessages.size() > 0) {
+        try {
-            downloadFlagAndEnvelope(context, account, mailbox, remoteFolder, unsyncedMessages,
+            values.clear();
-                    localMessageMap, unseenMessages);
+            while (c.moveToNext()) {
-        }
+                values.put(AttachmentColumns.UI_STATE, UIProvider.AttachmentState.SAVED);
-
+                Attachment att = new Attachment();
-        // 9. Refresh the flags for any messages in the local store that we didn't just download.
+                att.restore(c);
-        FetchProfile fp = new FetchProfile();
+                Message msg = Message.restoreMessageWithId(context, att.mMessageKey);
-        fp.add(FetchProfile.Item.FLAGS);
+                if (msg == null || (msg.mFlagLoaded == Message.FLAG_LOADED_COMPLETE)) {
-        remoteFolder.fetch(remoteMessages, fp, null);
+                    values.put(AttachmentColumns.UI_DOWNLOADED_SIZE, att.mSize);
-        boolean remoteSupportsSeen = false;
+                    resolver.update(ContentUris.withAppendedId(Attachment.CONTENT_URI, att.mId),
-        boolean remoteSupportsFlagged = false;
+                            values, null, null);
-        boolean remoteSupportsAnswered = false;
-        for (Flag flag : remoteFolder.getPermanentFlags()) {
-            if (flag == Flag.SEEN) {
-                remoteSupportsSeen = true;
-            }
-            if (flag == Flag.FLAGGED) {
-                remoteSupportsFlagged = true;
-            }
-            if (flag == Flag.ANSWERED) {
-                remoteSupportsAnswered = true;
-            }
-        }
-        // Update SEEN/FLAGGED/ANSWERED (star) flags (if supported remotely - e.g. not for POP3)
-        if (remoteSupportsSeen || remoteSupportsFlagged || remoteSupportsAnswered) {
-            for (Message remoteMessage : remoteMessages) {
-                LocalMessageInfo localMessageInfo = localMessageMap.get(remoteMessage.getUid());
-                if (localMessageInfo == null) {
                     continue;
-                }
+                } else {
-                boolean localSeen = localMessageInfo.mFlagRead;
+                    String uid = msg.mServerId;
-                boolean remoteSeen = remoteMessage.isSet(Flag.SEEN);
+                    Pop3Message popMessage = remoteUidMap.get(uid);
-                boolean newSeen = (remoteSupportsSeen && (remoteSeen != localSeen));
+                    if (popMessage != null) {
-                boolean localFlagged = localMessageInfo.mFlagFavorite;
+                        try {
-                boolean remoteFlagged = remoteMessage.isSet(Flag.FLAGGED);
+                            remoteFolder.fetchBody(popMessage, -1);
-                boolean newFlagged = (remoteSupportsFlagged && (localFlagged != remoteFlagged));
+                        } catch (IOException e) {
-                int localFlags = localMessageInfo.mFlags;
+                            throw new MessagingException(MessagingException.IOERROR);
-                boolean localAnswered = (localFlags & EmailContent.Message.FLAG_REPLIED_TO) != 0;
+                        }
-                boolean remoteAnswered = remoteMessage.isSet(Flag.ANSWERED);
+
-                boolean newAnswered = (remoteSupportsAnswered && (localAnswered != remoteAnswered));
+                        // Say we've downloaded the attachment
-                if (newSeen || newFlagged || newAnswered) {
+                        values.put(AttachmentColumns.UI_STATE, AttachmentState.SAVED);
-                    Uri uri = ContentUris.withAppendedId(
+                        Uri attUri = ContentUris.withAppendedId(Attachment.CONTENT_URI, att.mId);
-                            EmailContent.Message.CONTENT_URI, localMessageInfo.mId);
+                        resolver.update(attUri, values, null, null);
-                    ContentValues updateValues = new ContentValues();
+
-                    updateValues.put(MessageColumns.FLAG_READ, remoteSeen);
+                        int flag = EmailContent.Message.FLAG_LOADED_COMPLETE;
-                    updateValues.put(MessageColumns.FLAG_FAVORITE, remoteFlagged);
+                        if (!popMessage.isComplete()) {
-                    if (remoteAnswered) {
+                            Log.e(TAG, "How is this possible?");
-                        localFlags |= EmailContent.Message.FLAG_REPLIED_TO;
+                        }
-                    } else {
+                        Utilities.copyOneMessageToProvider(
-                        localFlags &= ~EmailContent.Message.FLAG_REPLIED_TO;
+                                context, popMessage, account, mailbox, flag);
+                        // Get rid of the temporary attachment
+                        resolver.delete(attUri, null, null);
+
                     }
-                    updateValues.put(MessageColumns.FLAGS, localFlags);
-                    resolver.update(uri, updateValues, null, null);
                 }
             }
+        } finally {
+            c.close();
         }
 
-        // 10. Remove any messages that are in the local store but no longer on the remote store.
+        // Remove any messages that are in the local store but no longer on the remote store.
         HashSet<String> localUidsToDelete = new HashSet<String>(localMessageMap.keySet());
         localUidsToDelete.removeAll(remoteUidMap.keySet());
         for (String uidToDelete : localUidsToDelete) {
             LocalMessageInfo infoToDelete = localMessageMap.get(uidToDelete);
 
             // Delete associated data (attachment files)
-            // Attachment & Body records are auto-deleted when we delete the Message record
+            // Attachment & Body records are auto-deleted when we delete the
+            // Message record
             AttachmentUtilities.deleteAllAttachmentFiles(context, account.mId,
                     infoToDelete.mId);
 
@@ -655,47 +382,43 @@ public class Pop3Service extends Service {
             resolver.delete(uriToDelete, null, null);
 
             // Delete extra rows (e.g. synced or deleted)
-            Uri syncRowToDelete = ContentUris.withAppendedId(
+            Uri updateRowToDelete = ContentUris.withAppendedId(
                     EmailContent.Message.UPDATED_CONTENT_URI, infoToDelete.mId);
-            resolver.delete(syncRowToDelete, null, null);
+            resolver.delete(updateRowToDelete, null, null);
-            Uri deletERowToDelete = ContentUris.withAppendedId(
+            Uri deleteRowToDelete = ContentUris.withAppendedId(
-                    EmailContent.Message.UPDATED_CONTENT_URI, infoToDelete.mId);
+                    EmailContent.Message.DELETED_CONTENT_URI, infoToDelete.mId);
-            resolver.delete(deletERowToDelete, null, null);
+            resolver.delete(deleteRowToDelete, null, null);
         }
 
+        // Load messages we need to sync
         loadUnsyncedMessages(context, account, remoteFolder, unsyncedMessages, mailbox);
 
-        // 14. Clean up and report results
+        // Clean up and report results
         remoteFolder.close(false);
     }
 
     /**
-     * Find messages in the updated table that need to be written back to server.
+     * Find messages in the updated table that need to be written back to
-     *
+     * server. Handles: Read/Unread Flagged Append (upload) Move To Trash Empty
-     * Handles:
+     * trash TODO: Move
-     *   Read/Unread
-     *   Flagged
-     *   Append (upload)
-     *   Move To Trash
-     *   Empty trash
-     * TODO:
-     *   Move
      *
      * @param account the account to scan for pending actions
      * @throws MessagingException
      */
     private static void processPendingActionsSynchronous(Context context, Account account)
-           throws MessagingException {
+            throws MessagingException {
         TrafficStats.setThreadStatsTag(TrafficFlags.getSyncFlags(context, account));
-        String[] accountIdArgs = new String[] { Long.toString(account.mId) };
+        String[] accountIdArgs = new String[] {
+            Long.toString(account.mId)
+        };
 
         // Handle deletes first, it's always better to get rid of things first
         processPendingDeletesSynchronous(context, account, accountIdArgs);
     }
 
     /**
-     * Scan for messages that are in the Message_Deletes table, look for differences that
+     * Scan for messages that are in the Message_Deletes table, look for
-     * we can deal with, and do the work.
+     * differences that we can deal with, and do the work.
      *
      * @param account
      * @param resolver
@@ -712,7 +435,7 @@ public class Pop3Service extends Service {
             // loop through messages marked as deleted
             while (deletes.moveToNext()) {
                 EmailContent.Message oldMessage =
-                    EmailContent.getContent(deletes, EmailContent.Message.class);
+                        EmailContent.getContent(deletes, EmailContent.Message.class);
 
                 // Finally, delete the update
                 Uri uri = ContentUris.withAppendedId(EmailContent.Message.DELETED_CONTENT_URI,
@@ -723,4 +446,4 @@ public class Pop3Service extends Service {
             deletes.close();
         }
     }
-}
+}