From 75a97a82c02b64d6091d4319bb84ba58f5f75814 Mon Sep 17 00:00:00 2001 From: cketti Date: Wed, 5 Sep 2012 00:35:43 +0200 Subject: [PATCH 1/8] Make MessageListHandler a real Handler that uses Android's message queue --- src/com/fsck/k9/activity/MessageList.java | 376 ++++++++++++---------- 1 file changed, 212 insertions(+), 164 deletions(-) diff --git a/src/com/fsck/k9/activity/MessageList.java b/src/com/fsck/k9/activity/MessageList.java index 417c49c1e..37aa9186e 100644 --- a/src/com/fsck/k9/activity/MessageList.java +++ b/src/com/fsck/k9/activity/MessageList.java @@ -17,6 +17,7 @@ import android.graphics.Color; import android.graphics.Typeface; import android.graphics.drawable.Drawable; import android.os.Bundle; +import android.os.Handler; import android.text.Spannable; import android.text.SpannableStringBuilder; import android.text.style.AbsoluteSizeSpan; @@ -348,215 +349,262 @@ public class MessageList } } - class MessageListHandler { + class MessageListHandler extends Handler { + private static final int ACTION_REMOVE_MESSAGES = 1; + private static final int ACTION_ADD_MESSAGES = 2; + private static final int ACTION_RESET_UNREAD_COUNT = 3; + private static final int ACTION_SORT_MESSAGES = 4; + private static final int ACTION_FOLDER_LOADING = 5; + private static final int ACTION_REFRESH_TITLE = 6; + private static final int ACTION_PROGRESS = 7; + + /** * @param messages Never {@code null}. */ public void removeMessages(final List messages) { - if (messages.isEmpty()) { - return; - } - runOnUiThread(new Runnable() { - @Override - public void run() { - for (MessageInfoHolder message : messages) { - if (message != null && (mFolderName == null || ( - message.folder != null && - message.folder.name.equals(mFolderName)))) { - if (message.selected && mSelectedCount > 0) { - mSelectedCount--; - } - mAdapter.messages.remove(message); - } - } - resetUnreadCountOnThread(); - - mAdapter.notifyDataSetChanged(); - toggleBatchButtons(); - } - }); + android.os.Message msg = android.os.Message.obtain(this, ACTION_REMOVE_MESSAGES, messages); + sendMessage(msg); } /** * @param messages Never {@code null}. */ public void addMessages(final List messages) { - if (messages.isEmpty()) { - return; + android.os.Message msg = android.os.Message.obtain(this, ACTION_ADD_MESSAGES, messages); + sendMessage(msg); + } + + public void resetUnreadCount() { + android.os.Message msg = android.os.Message.obtain(this, ACTION_RESET_UNREAD_COUNT); + sendMessage(msg); + } + + public void sortMessages() { + android.os.Message msg = android.os.Message.obtain(this, ACTION_SORT_MESSAGES); + sendMessage(msg); + } + + public void folderLoading(String folder, boolean loading) { + android.os.Message msg = android.os.Message.obtain(this, ACTION_FOLDER_LOADING, + (loading) ? 1 : 0, 0, folder); + sendMessage(msg); + } + + public void refreshTitle() { + android.os.Message msg = android.os.Message.obtain(this, ACTION_REFRESH_TITLE); + sendMessage(msg); + } + + public void progress(final boolean progress) { + android.os.Message msg = android.os.Message.obtain(this, ACTION_PROGRESS, + (progress) ? 1 : 0, 0); + sendMessage(msg); + } + + @SuppressWarnings("unchecked") + @Override + public void handleMessage(android.os.Message msg) { + switch (msg.what) { + case ACTION_REMOVE_MESSAGES: { + List messages = (List) msg.obj; + MessageList.this.removeMessages(messages); + break; + } + case ACTION_ADD_MESSAGES: { + List messages = (List) msg.obj; + MessageList.this.addMessages(messages); + break; + } + case ACTION_RESET_UNREAD_COUNT: { + MessageList.this.resetUnreadCount(); + break; + } + case ACTION_SORT_MESSAGES: { + MessageList.this.sortMessages(); + break; + } + case ACTION_FOLDER_LOADING: { + String folder = (String) msg.obj; + boolean loading = (msg.arg1 == 1); + MessageList.this.folderLoading(folder, loading); + break; + } + case ACTION_REFRESH_TITLE: { + MessageList.this.refreshTitle(); + break; + } + case ACTION_PROGRESS: { + boolean progress = (msg.arg1 == 1); + MessageList.this.progress(progress); + break; + } } - final boolean wasEmpty = mAdapter.messages.isEmpty(); - runOnUiThread(new Runnable() { - @Override - public void run() { - for (final MessageInfoHolder message : messages) { - if (mFolderName == null || (message.folder != null && message.folder.name.equals(mFolderName))) { - int index; - synchronized (mAdapter.messages) { - index = Collections.binarySearch(mAdapter.messages, message, getComparator()); - } + } + } - if (index < 0) { - index = (index * -1) - 1; - } - - mAdapter.messages.add(index, message); - } - } - - if (wasEmpty) { - mListView.setSelection(0); - } - resetUnreadCountOnThread(); - - mAdapter.notifyDataSetChanged(); - } - }); + private void removeMessages(final List messages) { + if (messages.isEmpty()) { + return; } - private void resetUnreadCount() { - runOnUiThread(new Runnable() { - @Override - public void run() { - resetUnreadCountOnThread(); + for (MessageInfoHolder message : messages) { + if (message != null && (mFolderName == null || ( + message.folder != null && + message.folder.name.equals(mFolderName)))) { + if (message.selected && mSelectedCount > 0) { + mSelectedCount--; } - }); + mAdapter.messages.remove(message); + } + } + resetUnreadCount(); + + mAdapter.notifyDataSetChanged(); + toggleBatchButtons(); + } + + private void addMessages(final List messages) { + if (messages.isEmpty()) { + return; } - private void resetUnreadCountOnThread() { - if (mQueryString != null) { - int unreadCount = 0; + final boolean wasEmpty = mAdapter.messages.isEmpty(); + + for (final MessageInfoHolder message : messages) { + if (mFolderName == null || (message.folder != null && message.folder.name.equals(mFolderName))) { + int index; synchronized (mAdapter.messages) { - for (MessageInfoHolder holder : mAdapter.messages) { - unreadCount += holder.read ? 0 : 1; - } + index = Collections.binarySearch(mAdapter.messages, message, getComparator()); } - mUnreadMessageCount = unreadCount; - refreshTitleOnThread(); + + if (index < 0) { + index = (index * -1) - 1; + } + + mAdapter.messages.add(index, message); } } - private void sortMessages() { - final Comparator chainComparator = getComparator(); + if (wasEmpty) { + mListView.setSelection(0); + } + resetUnreadCount(); - runOnUiThread(new Runnable() { - @Override - public void run() { - synchronized (mAdapter.messages) { - Collections.sort(mAdapter.messages, chainComparator); - } - mAdapter.notifyDataSetChanged(); + mAdapter.notifyDataSetChanged(); + } + + private void resetUnreadCount() { + if (mQueryString != null) { + int unreadCount = 0; + synchronized (mAdapter.messages) { + for (MessageInfoHolder holder : mAdapter.messages) { + unreadCount += holder.read ? 0 : 1; } - }); + } + mUnreadMessageCount = unreadCount; + refreshTitle(); + } + } + + private void sortMessages() { + final Comparator chainComparator = getComparator(); + + synchronized (mAdapter.messages) { + Collections.sort(mAdapter.messages, chainComparator); + } + mAdapter.notifyDataSetChanged(); + } + + /** + * @return The comparator to use to display messages in an ordered + * fashion. Never null. + */ + protected Comparator getComparator() { + final List> chain = new ArrayList>(2 /* we add 2 comparators at most */); + + { + // add the specified comparator + final Comparator comparator = SORT_COMPARATORS.get(mSortType); + if (mSortAscending) { + chain.add(comparator); + } else { + chain.add(new ReverseComparator(comparator)); + } } - /** - * @return The comparator to use to display messages in an ordered - * fashion. Never null. - */ - protected Comparator getComparator() { - final List> chain = new ArrayList>(2 /* we add 2 comparators at most */); - - { - // add the specified comparator - final Comparator comparator = SORT_COMPARATORS.get(mSortType); - if (mSortAscending) { + { + // add the date comparator if not already specified + if (mSortType != SortType.SORT_DATE && mSortType != SortType.SORT_ARRIVAL) { + final Comparator comparator = SORT_COMPARATORS.get(SortType.SORT_DATE); + if (mSortDateAscending) { chain.add(comparator); } else { chain.add(new ReverseComparator(comparator)); } } + } - { - // add the date comparator if not already specified - if (mSortType != SortType.SORT_DATE && mSortType != SortType.SORT_ARRIVAL) { - final Comparator comparator = SORT_COMPARATORS.get(SortType.SORT_DATE); - if (mSortDateAscending) { - chain.add(comparator); - } else { - chain.add(new ReverseComparator(comparator)); - } + // build the comparator chain + final Comparator chainComparator = new ComparatorChain(chain); + + return chainComparator; + } + + private void folderLoading(String folder, boolean loading) { + if (mCurrentFolder != null && mCurrentFolder.name.equals(folder)) { + mCurrentFolder.loading = loading; + } + updateFooterView(); + } + + private void refreshTitle() { + setWindowTitle(); + setWindowProgress(); + } + + private void setWindowProgress() { + int level = Window.PROGRESS_END; + + if (mCurrentFolder != null && mCurrentFolder.loading && mAdapter.mListener.getFolderTotal() > 0) { + int divisor = mAdapter.mListener.getFolderTotal(); + if (divisor != 0) { + level = (Window.PROGRESS_END / divisor) * (mAdapter.mListener.getFolderCompleted()) ; + if (level > Window.PROGRESS_END) { + level = Window.PROGRESS_END; } } - - // build the comparator chain - final Comparator chainComparator = new ComparatorChain(chain); - - return chainComparator; } - public void folderLoading(String folder, boolean loading) { - if (mCurrentFolder != null && mCurrentFolder.name.equals(folder)) { - mCurrentFolder.loading = loading; - } - runOnUiThread(new Runnable() { - @Override public void run() { - updateFooterView(); - } - }); - } + getWindow().setFeatureInt(Window.FEATURE_PROGRESS, level); + } - private void refreshTitle() { - runOnUiThread(new Runnable() { - @Override - public void run() { - refreshTitleOnThread(); - } - }); - } + private void setWindowTitle() { + String displayName; - private void refreshTitleOnThread() { - setWindowTitle(); - setWindowProgress(); - } + if (mFolderName != null) { + displayName = mFolderName; - private void setWindowProgress() { - int level = Window.PROGRESS_END; - - if (mCurrentFolder != null && mCurrentFolder.loading && mAdapter.mListener.getFolderTotal() > 0) { - int divisor = mAdapter.mListener.getFolderTotal(); - if (divisor != 0) { - level = (Window.PROGRESS_END / divisor) * (mAdapter.mListener.getFolderCompleted()) ; - if (level > Window.PROGRESS_END) { - level = Window.PROGRESS_END; - } - } + if (mAccount.getInboxFolderName().equalsIgnoreCase(displayName)) { + displayName = getString(R.string.special_mailbox_name_inbox); + } else if (mAccount.getOutboxFolderName().equals(displayName)) { + displayName = getString(R.string.special_mailbox_name_outbox); } - getWindow().setFeatureInt(Window.FEATURE_PROGRESS, level); - } - - private void setWindowTitle() { - String displayName; - - if (mFolderName != null) { - displayName = mFolderName; - - if (mAccount.getInboxFolderName().equalsIgnoreCase(displayName)) { - displayName = getString(R.string.special_mailbox_name_inbox); - } else if (mAccount.getOutboxFolderName().equals(displayName)) { - displayName = getString(R.string.special_mailbox_name_outbox); - } - - String dispString = mAdapter.mListener.formatHeader(MessageList.this, getString(R.string.message_list_title, mAccount.getDescription(), displayName), mUnreadMessageCount, getTimeFormat()); + String dispString = mAdapter.mListener.formatHeader(MessageList.this, getString(R.string.message_list_title, mAccount.getDescription(), displayName), mUnreadMessageCount, getTimeFormat()); + setTitle(dispString); + } else if (mQueryString != null) { + if (mTitle != null) { + String dispString = mAdapter.mListener.formatHeader(MessageList.this, mTitle, mUnreadMessageCount, getTimeFormat()); setTitle(dispString); - } else if (mQueryString != null) { - if (mTitle != null) { - String dispString = mAdapter.mListener.formatHeader(MessageList.this, mTitle, mUnreadMessageCount, getTimeFormat()); - setTitle(dispString); - } else { - setTitle(getString(R.string.search_results) + ": " + mQueryString); - } + } else { + setTitle(getString(R.string.search_results) + ": " + mQueryString); } } + } - public void progress(final boolean progress) { - runOnUiThread(new Runnable() { - @Override - public void run() { - showProgressIndicator(progress); - } - }); - } + private void progress(final boolean progress) { + showProgressIndicator(progress); } /** From bd9b6aea2d87245ed3603af309ccbf96ae863c91 Mon Sep 17 00:00:00 2001 From: cketti Date: Wed, 5 Sep 2012 00:51:04 +0200 Subject: [PATCH 2/8] Moved methods modifying the message list to MessageListAdapter --- src/com/fsck/k9/activity/MessageList.java | 181 ++++++++++++---------- 1 file changed, 97 insertions(+), 84 deletions(-) diff --git a/src/com/fsck/k9/activity/MessageList.java b/src/com/fsck/k9/activity/MessageList.java index 37aa9186e..a68294091 100644 --- a/src/com/fsck/k9/activity/MessageList.java +++ b/src/com/fsck/k9/activity/MessageList.java @@ -349,6 +349,9 @@ public class MessageList } } + /** + * FIXME + */ class MessageListHandler extends Handler { private static final int ACTION_REMOVE_MESSAGES = 1; private static final int ACTION_ADD_MESSAGES = 2; @@ -408,20 +411,20 @@ public class MessageList switch (msg.what) { case ACTION_REMOVE_MESSAGES: { List messages = (List) msg.obj; - MessageList.this.removeMessages(messages); + mAdapter.removeMessages(messages); break; } case ACTION_ADD_MESSAGES: { List messages = (List) msg.obj; - MessageList.this.addMessages(messages); + mAdapter.addMessages(messages); break; } case ACTION_RESET_UNREAD_COUNT: { - MessageList.this.resetUnreadCount(); + mAdapter.resetUnreadCount(); break; } case ACTION_SORT_MESSAGES: { - MessageList.this.sortMessages(); + mAdapter.sortMessages(); break; } case ACTION_FOLDER_LOADING: { @@ -443,79 +446,6 @@ public class MessageList } } - private void removeMessages(final List messages) { - if (messages.isEmpty()) { - return; - } - - for (MessageInfoHolder message : messages) { - if (message != null && (mFolderName == null || ( - message.folder != null && - message.folder.name.equals(mFolderName)))) { - if (message.selected && mSelectedCount > 0) { - mSelectedCount--; - } - mAdapter.messages.remove(message); - } - } - resetUnreadCount(); - - mAdapter.notifyDataSetChanged(); - toggleBatchButtons(); - } - - private void addMessages(final List messages) { - if (messages.isEmpty()) { - return; - } - - final boolean wasEmpty = mAdapter.messages.isEmpty(); - - for (final MessageInfoHolder message : messages) { - if (mFolderName == null || (message.folder != null && message.folder.name.equals(mFolderName))) { - int index; - synchronized (mAdapter.messages) { - index = Collections.binarySearch(mAdapter.messages, message, getComparator()); - } - - if (index < 0) { - index = (index * -1) - 1; - } - - mAdapter.messages.add(index, message); - } - } - - if (wasEmpty) { - mListView.setSelection(0); - } - resetUnreadCount(); - - mAdapter.notifyDataSetChanged(); - } - - private void resetUnreadCount() { - if (mQueryString != null) { - int unreadCount = 0; - synchronized (mAdapter.messages) { - for (MessageInfoHolder holder : mAdapter.messages) { - unreadCount += holder.read ? 0 : 1; - } - } - mUnreadMessageCount = unreadCount; - refreshTitle(); - } - } - - private void sortMessages() { - final Comparator chainComparator = getComparator(); - - synchronized (mAdapter.messages) { - Collections.sort(mAdapter.messages, chainComparator); - } - mAdapter.notifyDataSetChanged(); - } - /** * @return The comparator to use to display messages in an ordered * fashion. Never null. @@ -1985,7 +1915,7 @@ public class MessageList if (holder == null) { Log.w(K9.LOG_TAG, "Got callback to remove non-existent message with UID " + message.getUid()); } else { - removeMessages(Collections.singletonList(holder)); + mHandler.removeMessages(Collections.singletonList(holder)); } } @@ -2025,7 +1955,7 @@ public class MessageList public void listLocalMessagesRemoveMessage(Account account, String folder, Message message) { MessageInfoHolder holder = getMessage(message); if (holder != null) { - removeMessages(Collections.singletonList(holder)); + mHandler.removeMessages(Collections.singletonList(holder)); } } @@ -2103,18 +2033,101 @@ public class MessageList mSelectedCount--; toggleBatchButtons(); } - mAdapter.removeMessages(Collections.singletonList(holder)); + mHandler.removeMessages(Collections.singletonList(holder)); } } } } /** + * Note: Only call this from the UI thread! + * * @param holders * Never {@code null}. */ - public void removeMessages(List holders) { - mHandler.removeMessages(holders); + public void removeMessages(final List messages) { + if (messages.isEmpty()) { + return; + } + + for (MessageInfoHolder message : messages) { + if (message != null && (mFolderName == null || ( + message.folder != null && + message.folder.name.equals(mFolderName)))) { + if (message.selected && mSelectedCount > 0) { + mSelectedCount--; + } + mAdapter.messages.remove(message); + } + } + resetUnreadCount(); + + notifyDataSetChanged(); + toggleBatchButtons(); + } + + /** + * Note: Only call this from the UI thread! + * + * @param messages + */ + public void addMessages(final List messages) { + if (messages.isEmpty()) { + return; + } + + final boolean wasEmpty = mAdapter.messages.isEmpty(); + + for (final MessageInfoHolder message : messages) { + if (mFolderName == null || (message.folder != null && message.folder.name.equals(mFolderName))) { + int index; + synchronized (mAdapter.messages) { + index = Collections.binarySearch(mAdapter.messages, message, getComparator()); + } + + if (index < 0) { + index = (index * -1) - 1; + } + + mAdapter.messages.add(index, message); + } + } + + if (wasEmpty) { + mListView.setSelection(0); + } + resetUnreadCount(); + + notifyDataSetChanged(); + } + + /** + * Note: Only call this from the UI thread! + */ + public void resetUnreadCount() { + if (mQueryString != null) { + int unreadCount = 0; + synchronized (mAdapter.messages) { + for (MessageInfoHolder holder : mAdapter.messages) { + unreadCount += holder.read ? 0 : 1; + } + } + mUnreadMessageCount = unreadCount; + refreshTitle(); + } + } + + /** + * Note: Only call this from the UI thread! + */ + public void sortMessages() { + final Comparator chainComparator = getComparator(); + + synchronized (mAdapter.messages) { + Collections.sort(mAdapter.messages, chainComparator); + } + + notifyDataSetChanged(); } private void addOrUpdateMessage(Account account, String folderName, Message message, boolean verifyAgainstSearch) { @@ -2180,7 +2193,7 @@ public class MessageList } if (!messagesToRemove.isEmpty()) { - removeMessages(messagesToRemove); + mHandler.removeMessages(messagesToRemove); } if (!messagesToAdd.isEmpty()) { @@ -2719,7 +2732,7 @@ public class MessageList } } } - mAdapter.removeMessages(removeHolderList); + mHandler.removeMessages(removeHolderList); if (!messageList.isEmpty()) { if (v == mBatchDeleteButton) { From fb6d004692185512f0a70fc93b2a44234fa54416 Mon Sep 17 00:00:00 2001 From: cketti Date: Wed, 5 Sep 2012 01:02:05 +0200 Subject: [PATCH 3/8] Don't use MessageListHandler when calling from the UI thread --- src/com/fsck/k9/activity/MessageList.java | 33 +++++++++++++---------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/src/com/fsck/k9/activity/MessageList.java b/src/com/fsck/k9/activity/MessageList.java index a68294091..816328fd4 100644 --- a/src/com/fsck/k9/activity/MessageList.java +++ b/src/com/fsck/k9/activity/MessageList.java @@ -867,9 +867,10 @@ public class MessageList if (mAccount != null && mFolderName != null) { mController.getFolderUnreadMessageCount(mAccount, mFolderName, mAdapter.mListener); } - mHandler.refreshTitle(); + refreshTitle(); } + private void initializeLayout() { requestWindowFeature(Window.FEATURE_INDETERMINATE_PROGRESS); requestWindowFeature(Window.FEATURE_PROGRESS); @@ -1232,7 +1233,7 @@ public class MessageList Toast toast = Toast.makeText(this, toastString, Toast.LENGTH_SHORT); toast.show(); - mHandler.sortMessages(); + mAdapter.sortMessages(); } private void onCycleSort() { @@ -1264,7 +1265,7 @@ public class MessageList for (MessageInfoHolder holder : holders) { messagesToRemove.add(holder.message); } - mHandler.removeMessages(holders); + mAdapter.removeMessages(holders); mController.deleteMessages(messagesToRemove.toArray(EMPTY_MESSAGE_ARRAY), null); } @@ -1335,7 +1336,7 @@ public class MessageList holder.read = true; } } - mHandler.sortMessages(); + mAdapter.sortMessages(); } catch (Exception e) { // Ignore } @@ -1430,7 +1431,7 @@ public class MessageList String folderName = folder.getName(); mController.setFlag(account, folderName, new Message[] { message }, Flag.SEEN, !holder.read); holder.read = !holder.read; - mHandler.sortMessages(); + mAdapter.sortMessages(); } private void onToggleFlag(MessageInfoHolder holder) { @@ -1440,7 +1441,7 @@ public class MessageList String folderName = folder.getName(); mController.setFlag(account, folderName, new Message[] { message }, Flag.FLAGGED, !holder.flagged); holder.flagged = !holder.flagged; - mHandler.sortMessages(); + mAdapter.sortMessages(); } private void checkMail(Account account, String folderName) { @@ -2040,7 +2041,7 @@ public class MessageList } /** - * Note: Only call this from the UI thread! + * Note: Must be called from the UI thread! * * @param holders * Never {@code null}. @@ -2067,7 +2068,7 @@ public class MessageList } /** - * Note: Only call this from the UI thread! + * Note: Must be called from the UI thread! * * @param messages */ @@ -2102,7 +2103,7 @@ public class MessageList } /** - * Note: Only call this from the UI thread! + * Note: Must be called from the UI thread! */ public void resetUnreadCount() { if (mQueryString != null) { @@ -2118,7 +2119,7 @@ public class MessageList } /** - * Note: Only call this from the UI thread! + * Note: Must be called from the UI thread! */ public void sortMessages() { final Comparator chainComparator = getComparator(); @@ -2732,7 +2733,8 @@ public class MessageList } } } - mHandler.removeMessages(removeHolderList); + + mAdapter.removeMessages(removeHolderList); if (!messageList.isEmpty()) { if (v == mBatchDeleteButton) { @@ -2746,7 +2748,8 @@ public class MessageList // Should not happen Toast.makeText(this, R.string.no_message_seletected_toast, Toast.LENGTH_SHORT).show(); } - mHandler.sortMessages(); + + mAdapter.sortMessages(); } @Override @@ -2811,7 +2814,7 @@ public class MessageList } } mController.setFlag(messageList, flag, newState); - mHandler.sortMessages(); + mAdapter.sortMessages(); } /** @@ -3005,6 +3008,8 @@ public class MessageList * {@link #move(List, String)}. This method was added mainly because those 2 * methods share common behavior. * + * Note: Must be called from the UI thread! + * * @param holders * Never {@code null}. * @param destination @@ -3054,7 +3059,7 @@ public class MessageList if (operation == FolderOperation.MOVE) { mController.moveMessages(account, folderName, messages.toArray(new Message[messages.size()]), destination, null); - mHandler.removeMessages(holders); + mAdapter.removeMessages(holders); } else { mController.copyMessages(account, folderName, messages.toArray(new Message[messages.size()]), destination, null); From d97da517fa00dceb79eb67a79561c1ca635993f1 Mon Sep 17 00:00:00 2001 From: cketti Date: Wed, 5 Sep 2012 01:31:09 +0200 Subject: [PATCH 4/8] Rewrote code accessing mAdapter.messages from outside MessageListAdapter --- src/com/fsck/k9/activity/MessageList.java | 151 +++++++++++----------- 1 file changed, 75 insertions(+), 76 deletions(-) diff --git a/src/com/fsck/k9/activity/MessageList.java b/src/com/fsck/k9/activity/MessageList.java index 816328fd4..da3d93df2 100644 --- a/src/com/fsck/k9/activity/MessageList.java +++ b/src/com/fsck/k9/activity/MessageList.java @@ -743,7 +743,7 @@ public class MessageList final ActivityState previousData = getLastNonConfigurationInstance(); if (previousData != null) { - mAdapter.messages.addAll(previousData.messages); + mAdapter.restoreMessages(previousData.messages); mActiveMessages = previousData.activeMessages; } } @@ -821,7 +821,7 @@ public class MessageList mController.notifyAccountCancel(this, accountWithNotification); } - if (mAdapter.messages.isEmpty()) { + if (mAdapter.isEmpty()) { if (mFolderName != null) { mController.listLocalMessages(mAccount, mFolderName, mAdapter.mListener); // Hide the archive button if we don't have an archive folder. @@ -930,7 +930,7 @@ public class MessageList @Override public ActivityState onRetainNonConfigurationInstance() { final ActivityState state = new ActivityState(); - state.messages = mAdapter.messages; + state.messages = mAdapter.getMessages(); state.activeMessages = mActiveMessages; return state; } @@ -1121,12 +1121,11 @@ public class MessageList // Need to get the list before the sort starts ArrayList messageRefs = new ArrayList(); - synchronized (mAdapter.messages) { - for (MessageInfoHolder holder : mAdapter.messages) { - MessageReference ref = holder.message.makeMessageReference(); - messageRefs.add(ref); - } + for (MessageInfoHolder holder : mAdapter.getMessages()) { + MessageReference ref = holder.message.makeMessageReference(); + messageRefs.add(ref); } + MessageReference ref = message.message.makeMessageReference(); Log.i(K9.LOG_TAG, "MessageList sending message " + ref); @@ -1331,10 +1330,8 @@ public class MessageList try { mController.markAllMessagesRead(mAccount, mCurrentFolder.name); - synchronized (mAdapter.messages) { - for (MessageInfoHolder holder : mAdapter.messages) { - holder.read = true; - } + for (MessageInfoHolder holder : mAdapter.getMessages()) { + holder.read = true; } mAdapter.sortMessages(); } catch (Exception e) { @@ -1864,7 +1861,8 @@ public class MessageList } class MessageListAdapter extends BaseAdapter { - private final List messages = java.util.Collections.synchronizedList(new ArrayList()); + private final List mMessages = + Collections.synchronizedList(new ArrayList()); private final ActivityListener mListener = new ActivityListener() { @@ -2007,6 +2005,14 @@ public class MessageList } } + public List getMessages() { + return mMessages; + } + + public void restoreMessages(List messages) { + mMessages.addAll(messages); + } + private Drawable mAttachmentIcon; private Drawable mForwardedIcon; private Drawable mAnsweredIcon; @@ -2020,13 +2026,14 @@ public class MessageList } public void markAllMessagesAsDirty() { - for (MessageInfoHolder holder : mAdapter.messages) { + for (MessageInfoHolder holder : mMessages) { holder.dirty = true; } } + public void pruneDirtyMessages() { - synchronized (mAdapter.messages) { - Iterator iter = mAdapter.messages.iterator(); + synchronized (mMessages) { + Iterator iter = mMessages.iterator(); while (iter.hasNext()) { MessageInfoHolder holder = iter.next(); if (holder.dirty) { @@ -2058,7 +2065,7 @@ public class MessageList if (message.selected && mSelectedCount > 0) { mSelectedCount--; } - mAdapter.messages.remove(message); + mMessages.remove(message); } } resetUnreadCount(); @@ -2077,20 +2084,20 @@ public class MessageList return; } - final boolean wasEmpty = mAdapter.messages.isEmpty(); + final boolean wasEmpty = mMessages.isEmpty(); for (final MessageInfoHolder message : messages) { if (mFolderName == null || (message.folder != null && message.folder.name.equals(mFolderName))) { int index; - synchronized (mAdapter.messages) { - index = Collections.binarySearch(mAdapter.messages, message, getComparator()); + synchronized (mMessages) { + index = Collections.binarySearch(mMessages, message, getComparator()); } if (index < 0) { index = (index * -1) - 1; } - mAdapter.messages.add(index, message); + mMessages.add(index, message); } } @@ -2108,8 +2115,8 @@ public class MessageList public void resetUnreadCount() { if (mQueryString != null) { int unreadCount = 0; - synchronized (mAdapter.messages) { - for (MessageInfoHolder holder : mAdapter.messages) { + synchronized (mMessages) { + for (MessageInfoHolder holder : mMessages) { unreadCount += holder.read ? 0 : 1; } } @@ -2124,8 +2131,8 @@ public class MessageList public void sortMessages() { final Comparator chainComparator = getComparator(); - synchronized (mAdapter.messages) { - Collections.sort(mAdapter.messages, chainComparator); + synchronized (mMessages) { + Collections.sort(mMessages, chainComparator); } notifyDataSetChanged(); @@ -2212,8 +2219,8 @@ public class MessageList // XXX TODO - make this not use a for loop public MessageInfoHolder getMessage(MessageReference messageReference) { - synchronized (mAdapter.messages) { - for (MessageInfoHolder holder : mAdapter.messages) { + synchronized (mMessages) { + for (MessageInfoHolder holder : mMessages) { /* * 2010-06-21 - cketti * Added null pointer check. Not sure what's causing 'holder' @@ -2257,7 +2264,7 @@ public class MessageList @Override public int getCount() { - return messages.size(); + return mMessages.size(); } @Override @@ -2280,13 +2287,13 @@ public class MessageList @Override public Object getItem(int position) { try { - synchronized (mAdapter.messages) { - if (position < mAdapter.messages.size()) { - return mAdapter.messages.get(position); + synchronized (mMessages) { + if (position < mMessages.size()) { + return mMessages.get(position); } } } catch (Exception e) { - Log.e(K9.LOG_TAG, "getItem(" + position + "), but folder.messages.size() = " + mAdapter.messages.size(), e); + Log.e(K9.LOG_TAG, "getItem(" + position + "), but folder.messages.size() = " + mMessages.size(), e); } return null; } @@ -2506,11 +2513,7 @@ public class MessageList } public boolean isItemSelectable(int position) { - if (position < mAdapter.messages.size()) { - return true; - } else { - return false; - } + return (position < mMessages.size()); } } @@ -2659,34 +2662,32 @@ public class MessageList private boolean computeBatchDirection(boolean flagged) { boolean newState = false; - synchronized (mAdapter.messages) { - for (MessageInfoHolder holder : mAdapter.messages) { - if (holder.selected) { - if (flagged) { - if (!holder.flagged) { - newState = true; - break; - } - } else { - if (!holder.read) { - newState = true; - break; - } + for (MessageInfoHolder holder : mAdapter.getMessages()) { + if (holder.selected) { + if (flagged) { + if (!holder.flagged) { + newState = true; + break; + } + } else { + if (!holder.read) { + newState = true; + break; } } } } + return newState; } private boolean anySelected() { - synchronized (mAdapter.messages) { - for (MessageInfoHolder holder : mAdapter.messages) { - if (holder.selected) { - return true; - } + for (MessageInfoHolder holder : mAdapter.getMessages()) { + if (holder.selected) { + return true; } } + return false; } @@ -2719,18 +2720,16 @@ public class MessageList return; } - synchronized (mAdapter.messages) { - for (MessageInfoHolder holder : mAdapter.messages) { - if (holder.selected) { - if (v == mBatchDeleteButton) { - removeHolderList.add(holder); - } else if (v == mBatchFlagButton) { - holder.flagged = newState; - } else if (v == mBatchReadButton) { - holder.read = newState; - } - messageList.add(holder.message); + for (MessageInfoHolder holder : mAdapter.getMessages()) { + if (holder.selected) { + if (v == mBatchDeleteButton) { + removeHolderList.add(holder); + } else if (v == mBatchFlagButton) { + holder.flagged = newState; + } else if (v == mBatchReadButton) { + holder.read = newState; } + messageList.add(holder.message); } } @@ -2768,12 +2767,12 @@ public class MessageList private void setAllSelected(boolean isSelected) { mSelectedCount = 0; - synchronized (mAdapter.messages) { - for (MessageInfoHolder holder : mAdapter.messages) { - holder.selected = isSelected; - mSelectedCount += (isSelected ? 1 : 0); - } + + for (MessageInfoHolder holder : mAdapter.getMessages()) { + holder.selected = isSelected; + mSelectedCount += (isSelected ? 1 : 0); } + mAdapter.notifyDataSetChanged(); toggleBatchButtons(); } @@ -2973,13 +2972,13 @@ public class MessageList */ private List getSelectionFromCheckboxes() { final List selection = new ArrayList(); - synchronized (mAdapter.messages) { - for (final MessageInfoHolder holder : mAdapter.messages) { - if (holder.selected) { - selection.add(holder); - } + + for (final MessageInfoHolder holder : mAdapter.getMessages()) { + if (holder.selected) { + selection.add(holder); } } + return selection; } From 16ab1b67bc8d0fbf3a8306e86bff593c1e8bc909 Mon Sep 17 00:00:00 2001 From: cketti Date: Wed, 5 Sep 2012 03:02:22 +0200 Subject: [PATCH 5/8] Made sure the message list is only modified from the UI thread --- src/com/fsck/k9/activity/MessageList.java | 215 ++++++++++++---------- 1 file changed, 115 insertions(+), 100 deletions(-) diff --git a/src/com/fsck/k9/activity/MessageList.java b/src/com/fsck/k9/activity/MessageList.java index da3d93df2..1d57bc808 100644 --- a/src/com/fsck/k9/activity/MessageList.java +++ b/src/com/fsck/k9/activity/MessageList.java @@ -360,6 +360,7 @@ public class MessageList private static final int ACTION_FOLDER_LOADING = 5; private static final int ACTION_REFRESH_TITLE = 6; private static final int ACTION_PROGRESS = 7; + private static final int ACTION_REMOVE_MESSAGE = 8; /** @@ -405,6 +406,37 @@ public class MessageList sendMessage(msg); } + public void removeMessage(MessageReference messageReference) { + android.os.Message msg = android.os.Message.obtain(this, ACTION_REMOVE_MESSAGE, + messageReference); + sendMessage(msg); + } + + public void changeMessageUid(final MessageReference ref, final String newUid) { + // Instead of creating a container to be able to pass both arguments in a Message we + // post a Runnable to the message queue. + post(new Runnable() { + @Override + public void run() { + mAdapter.changeMessageUid(ref, newUid); + } + }); + } + + public void addOrUpdateMessages(final Account account, final String folderName, + final List providedMessages, final boolean verifyAgainstSearch) { + // We copy the message list because it's later modified by MessagingController + final List messages = new ArrayList(providedMessages); + + post(new Runnable() { + @Override + public void run() { + mAdapter.addOrUpdateMessages(account, folderName, messages, + verifyAgainstSearch); + } + }); + } + @SuppressWarnings("unchecked") @Override public void handleMessage(android.os.Message msg) { @@ -442,6 +474,11 @@ public class MessageList MessageList.this.progress(progress); break; } + case ACTION_REMOVE_MESSAGE: { + MessageReference messageReference = (MessageReference) msg.obj; + mAdapter.removeMessage(messageReference); + break; + } } } } @@ -838,22 +875,21 @@ public class MessageList // reread the selected date format preference in case it has changed mMessageHelper.refresh(); + mAdapter.markAllMessagesAsDirty(); + new Thread() { @Override public void run() { - mAdapter.markAllMessagesAsDirty(); - if (mFolderName != null) { mController.listLocalMessagesSynchronous(mAccount, mFolderName, mAdapter.mListener); } else if (mQueryString != null) { mController.searchLocalMessagesSynchronous(mAccountUuids, mFolderNames, null, mQueryString, mIntegrate, mQueryFlags, mForbiddenFlags, mAdapter.mListener); } - - mAdapter.pruneDirtyMessages(); runOnUiThread(new Runnable() { @Override public void run() { + mAdapter.pruneDirtyMessages(); mAdapter.notifyDataSetChanged(); restoreListState(); } @@ -1905,17 +1941,12 @@ public class MessageList @Override public void synchronizeMailboxAddOrUpdateMessage(Account account, String folder, Message message) { - addOrUpdateMessage(account, folder, message, true); + mHandler.addOrUpdateMessages(account, folder, Collections.singletonList(message), true); } @Override public void synchronizeMailboxRemovedMessage(Account account, String folder, Message message) { - MessageInfoHolder holder = getMessage(message); - if (holder == null) { - Log.w(K9.LOG_TAG, "Got callback to remove non-existent message with UID " + message.getUid()); - } else { - mHandler.removeMessages(Collections.singletonList(holder)); - } + mHandler.removeMessage(message.makeMessageReference()); } @Override @@ -1952,20 +1983,17 @@ public class MessageList @Override public void listLocalMessagesRemoveMessage(Account account, String folder, Message message) { - MessageInfoHolder holder = getMessage(message); - if (holder != null) { - mHandler.removeMessages(Collections.singletonList(holder)); - } + mHandler.removeMessage(message.makeMessageReference()); } @Override public void listLocalMessagesAddMessages(Account account, String folder, List messages) { - addOrUpdateMessages(account, folder, messages, false); + mHandler.addOrUpdateMessages(account, folder, messages, false); } @Override public void listLocalMessagesUpdateMessage(Account account, String folder, Message message) { - addOrUpdateMessage(account, folder, message, false); + mHandler.addOrUpdateMessages(account, folder, Collections.singletonList(message), false); } @Override @@ -1989,11 +2017,7 @@ public class MessageList ref.folderName = folder; ref.uid = oldUid; - MessageInfoHolder holder = getMessage(ref); - if (holder != null) { - holder.uid = newUid; - holder.message.setUid(newUid); - } + mHandler.changeMessageUid(ref, newUid); } }; @@ -2032,27 +2056,26 @@ public class MessageList } public void pruneDirtyMessages() { - synchronized (mMessages) { - Iterator iter = mMessages.iterator(); - while (iter.hasNext()) { - MessageInfoHolder holder = iter.next(); - if (holder.dirty) { - if (holder.selected) { - mSelectedCount--; - toggleBatchButtons(); - } - mHandler.removeMessages(Collections.singletonList(holder)); - } + List messagesToRemove = new ArrayList(); + + for (MessageInfoHolder holder : mMessages) { + if (holder.dirty) { + messagesToRemove.add(holder); } } + removeMessages(messagesToRemove); + } + + public void removeMessage(MessageReference messageReference) { + MessageInfoHolder holder = getMessage(messageReference); + if (holder == null) { + Log.w(K9.LOG_TAG, "Got callback to remove non-existent message with UID " + + messageReference.uid); + } else { + removeMessages(Collections.singletonList(holder)); + } } - /** - * Note: Must be called from the UI thread! - * - * @param holders - * Never {@code null}. - */ public void removeMessages(final List messages) { if (messages.isEmpty()) { return; @@ -2074,11 +2097,6 @@ public class MessageList toggleBatchButtons(); } - /** - * Note: Must be called from the UI thread! - * - * @param messages - */ public void addMessages(final List messages) { if (messages.isEmpty()) { return; @@ -2088,10 +2106,7 @@ public class MessageList for (final MessageInfoHolder message : messages) { if (mFolderName == null || (message.folder != null && message.folder.name.equals(mFolderName))) { - int index; - synchronized (mMessages) { - index = Collections.binarySearch(mMessages, message, getComparator()); - } + int index = Collections.binarySearch(mMessages, message, getComparator()); if (index < 0) { index = (index * -1) - 1; @@ -2109,45 +2124,37 @@ public class MessageList notifyDataSetChanged(); } - /** - * Note: Must be called from the UI thread! - */ + public void changeMessageUid(MessageReference ref, String newUid) { + MessageInfoHolder holder = getMessage(ref); + if (holder != null) { + holder.uid = newUid; + holder.message.setUid(newUid); + } + } + public void resetUnreadCount() { if (mQueryString != null) { int unreadCount = 0; - synchronized (mMessages) { - for (MessageInfoHolder holder : mMessages) { - unreadCount += holder.read ? 0 : 1; - } + + for (MessageInfoHolder holder : mMessages) { + unreadCount += holder.read ? 0 : 1; } + mUnreadMessageCount = unreadCount; refreshTitle(); } } - /** - * Note: Must be called from the UI thread! - */ public void sortMessages() { final Comparator chainComparator = getComparator(); - synchronized (mMessages) { - Collections.sort(mMessages, chainComparator); - } + Collections.sort(mMessages, chainComparator); notifyDataSetChanged(); } - private void addOrUpdateMessage(Account account, String folderName, Message message, boolean verifyAgainstSearch) { - List messages = new ArrayList(); - messages.add(message); - addOrUpdateMessages(account, folderName, messages, verifyAgainstSearch); - } - - private void addOrUpdateMessages(final Account account, final String folderName, final List providedMessages, final boolean verifyAgainstSearch) { - // we copy the message list because the callback doesn't expect - // the callbacks to mutate it. - final List messages = new ArrayList(providedMessages); + public void addOrUpdateMessages(final Account account, final String folderName, + final List messages, final boolean verifyAgainstSearch) { boolean needsSort = false; final List messagesToAdd = new ArrayList(); @@ -2169,7 +2176,9 @@ public class MessageList if (m == null) { if (updateForMe(account, folderName)) { m = new MessageInfoHolder(); - messageHelper.populate(m, message, new FolderInfoHolder(MessageList.this, messageFolder, messageAccount), messageAccount); + FolderInfoHolder folderInfoHolder = new FolderInfoHolder( + MessageList.this, messageFolder, messageAccount); + messageHelper.populate(m, message, folderInfoHolder, messageAccount); messagesToAdd.add(m); } else { if (mQueryString != null) { @@ -2177,63 +2186,71 @@ public class MessageList messagesToSearch.add(message); } else { m = new MessageInfoHolder(); - messageHelper.populate(m, message, new FolderInfoHolder(MessageList.this, messageFolder, messageAccount), messageAccount); + FolderInfoHolder folderInfoHolder = new FolderInfoHolder( + MessageList.this, messageFolder, messageAccount); + messageHelper.populate(m, message, folderInfoHolder, + messageAccount); messagesToAdd.add(m); } } } } else { m.dirty = false; // as we reload the message, unset its dirty flag - messageHelper.populate(m, message, new FolderInfoHolder(MessageList.this, messageFolder, account), account); + FolderInfoHolder folderInfoHolder = new FolderInfoHolder(MessageList.this, + messageFolder, account); + messageHelper.populate(m, message, folderInfoHolder, account); needsSort = true; } } } if (!messagesToSearch.isEmpty()) { - mController.searchLocalMessages(mAccountUuids, mFolderNames, messagesToSearch.toArray(EMPTY_MESSAGE_ARRAY), mQueryString, mIntegrate, mQueryFlags, mForbiddenFlags, + mController.searchLocalMessages(mAccountUuids, mFolderNames, + messagesToSearch.toArray(EMPTY_MESSAGE_ARRAY), mQueryString, mIntegrate, + mQueryFlags, mForbiddenFlags, new MessagingListener() { @Override - public void listLocalMessagesAddMessages(Account account, String folder, List messages) { - addOrUpdateMessages(account, folder, messages, false); + public void listLocalMessagesAddMessages(Account account, String folder, + List messages) { + mHandler.addOrUpdateMessages(account, folder, messages, false); } }); } if (!messagesToRemove.isEmpty()) { - mHandler.removeMessages(messagesToRemove); + removeMessages(messagesToRemove); } if (!messagesToAdd.isEmpty()) { - mHandler.addMessages(messagesToAdd); + addMessages(messagesToAdd); } if (needsSort) { - mHandler.sortMessages(); - mHandler.resetUnreadCount(); + sortMessages(); + resetUnreadCount(); } } - public MessageInfoHolder getMessage(Message message) { + + private MessageInfoHolder getMessage(Message message) { return getMessage(message.makeMessageReference()); } // XXX TODO - make this not use a for loop - public MessageInfoHolder getMessage(MessageReference messageReference) { - synchronized (mMessages) { - for (MessageInfoHolder holder : mMessages) { - /* - * 2010-06-21 - cketti - * Added null pointer check. Not sure what's causing 'holder' - * to be null. See log provided in issue 1749, comment #15. - * - * Please remove this comment once the cause was found and the - * bug(?) fixed. - */ - if ((holder != null) && holder.message.equalsReference(messageReference)) { - return holder; - } + private MessageInfoHolder getMessage(MessageReference messageReference) { + for (MessageInfoHolder holder : mMessages) { + /* + * 2010-06-21 - cketti + * Added null pointer check. Not sure what's causing 'holder' + * to be null. See log provided in issue 1749, comment #15. + * + * Please remove this comment once the cause was found and the + * bug(?) fixed. + */ + if ((holder != null) && holder.message.equalsReference(messageReference)) { + return holder; } } + return null; } @@ -2287,10 +2304,8 @@ public class MessageList @Override public Object getItem(int position) { try { - synchronized (mMessages) { - if (position < mMessages.size()) { - return mMessages.get(position); - } + if (position < mMessages.size()) { + return mMessages.get(position); } } catch (Exception e) { Log.e(K9.LOG_TAG, "getItem(" + position + "), but folder.messages.size() = " + mMessages.size(), e); From 9c335127e2b089550358578dea7862e3bbb1724b Mon Sep 17 00:00:00 2001 From: cketti Date: Wed, 5 Sep 2012 03:50:02 +0200 Subject: [PATCH 6/8] Code cleanup --- src/com/fsck/k9/activity/MessageList.java | 81 ++++++----------------- 1 file changed, 21 insertions(+), 60 deletions(-) diff --git a/src/com/fsck/k9/activity/MessageList.java b/src/com/fsck/k9/activity/MessageList.java index 1d57bc808..03c061729 100644 --- a/src/com/fsck/k9/activity/MessageList.java +++ b/src/com/fsck/k9/activity/MessageList.java @@ -350,37 +350,26 @@ public class MessageList } /** - * FIXME + * This class is used to run operations that modify UI elements in the UI thread. + * + *

We are using convenience methods that add a {@link android.os.Message} instance or a + * {@link Runnable} to the message queue.

+ * + *

Note: If you add a method to this class make sure you don't accidentally + * perform the operation in the calling thread.

*/ class MessageListHandler extends Handler { - private static final int ACTION_REMOVE_MESSAGES = 1; - private static final int ACTION_ADD_MESSAGES = 2; - private static final int ACTION_RESET_UNREAD_COUNT = 3; - private static final int ACTION_SORT_MESSAGES = 4; - private static final int ACTION_FOLDER_LOADING = 5; - private static final int ACTION_REFRESH_TITLE = 6; - private static final int ACTION_PROGRESS = 7; - private static final int ACTION_REMOVE_MESSAGE = 8; + private static final int ACTION_REMOVE_MESSAGE = 1; + private static final int ACTION_RESET_UNREAD_COUNT = 2; + private static final int ACTION_SORT_MESSAGES = 3; + private static final int ACTION_FOLDER_LOADING = 4; + private static final int ACTION_REFRESH_TITLE = 5; + private static final int ACTION_PROGRESS = 6; - /** - * @param messages Never {@code null}. - */ - public void removeMessages(final List messages) { - android.os.Message msg = android.os.Message.obtain(this, ACTION_REMOVE_MESSAGES, messages); - sendMessage(msg); - } - - /** - * @param messages Never {@code null}. - */ - public void addMessages(final List messages) { - android.os.Message msg = android.os.Message.obtain(this, ACTION_ADD_MESSAGES, messages); - sendMessage(msg); - } - - public void resetUnreadCount() { - android.os.Message msg = android.os.Message.obtain(this, ACTION_RESET_UNREAD_COUNT); + public void removeMessage(MessageReference messageReference) { + android.os.Message msg = android.os.Message.obtain(this, ACTION_REMOVE_MESSAGE, + messageReference); sendMessage(msg); } @@ -406,15 +395,9 @@ public class MessageList sendMessage(msg); } - public void removeMessage(MessageReference messageReference) { - android.os.Message msg = android.os.Message.obtain(this, ACTION_REMOVE_MESSAGE, - messageReference); - sendMessage(msg); - } - public void changeMessageUid(final MessageReference ref, final String newUid) { - // Instead of creating a container to be able to pass both arguments in a Message we - // post a Runnable to the message queue. + // Instead of explicitly creating a container to be able to pass both arguments in a + // Message we post a Runnable to the message queue. post(new Runnable() { @Override public void run() { @@ -437,18 +420,12 @@ public class MessageList }); } - @SuppressWarnings("unchecked") @Override public void handleMessage(android.os.Message msg) { switch (msg.what) { - case ACTION_REMOVE_MESSAGES: { - List messages = (List) msg.obj; - mAdapter.removeMessages(messages); - break; - } - case ACTION_ADD_MESSAGES: { - List messages = (List) msg.obj; - mAdapter.addMessages(messages); + case ACTION_REMOVE_MESSAGE: { + MessageReference messageReference = (MessageReference) msg.obj; + mAdapter.removeMessage(messageReference); break; } case ACTION_RESET_UNREAD_COUNT: { @@ -474,11 +451,6 @@ public class MessageList MessageList.this.progress(progress); break; } - case ACTION_REMOVE_MESSAGE: { - MessageReference messageReference = (MessageReference) msg.obj; - mAdapter.removeMessage(messageReference); - break; - } } } } @@ -2297,10 +2269,6 @@ public class MessageList return -1; } - public Object getItem(long position) { - return getItem((int)position); - } - @Override public Object getItem(int position) { try { @@ -2519,17 +2487,10 @@ public class MessageList } } - - - @Override public boolean hasStableIds() { return true; } - - public boolean isItemSelectable(int position) { - return (position < mMessages.size()); - } } class MessageViewHolder From 5678786c97596703b1b466e7f474f391df617ec1 Mon Sep 17 00:00:00 2001 From: cketti Date: Wed, 5 Sep 2012 05:57:52 +0200 Subject: [PATCH 7/8] Properly closing InputStreams to avoid StrictMode warnings --- .../k9/mail/internet/BinaryTempFileBody.java | 18 ++++++++++++------ src/com/fsck/k9/mail/internet/MimeUtility.java | 13 ++++++++++++- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/src/com/fsck/k9/mail/internet/BinaryTempFileBody.java b/src/com/fsck/k9/mail/internet/BinaryTempFileBody.java index 154776ca1..5a119a0a8 100644 --- a/src/com/fsck/k9/mail/internet/BinaryTempFileBody.java +++ b/src/com/fsck/k9/mail/internet/BinaryTempFileBody.java @@ -45,13 +45,16 @@ public class BinaryTempFileBody implements Body { public void writeTo(OutputStream out) throws IOException, MessagingException { InputStream in = getInputStream(); - Base64OutputStream base64Out = new Base64OutputStream(out); try { - IOUtils.copy(in, base64Out); + Base64OutputStream base64Out = new Base64OutputStream(out); + try { + IOUtils.copy(in, base64Out); + } finally { + base64Out.close(); + } } finally { - base64Out.close(); + in.close(); } - mFile.delete(); } class BinaryTempFileBodyInputStream extends FilterInputStream { @@ -61,8 +64,11 @@ public class BinaryTempFileBody implements Body { @Override public void close() throws IOException { - super.close(); - mFile.delete(); + try { + super.close(); + } finally { + mFile.delete(); + } } } } diff --git a/src/com/fsck/k9/mail/internet/MimeUtility.java b/src/com/fsck/k9/mail/internet/MimeUtility.java index 55b360cde..3eb2799f3 100644 --- a/src/com/fsck/k9/mail/internet/MimeUtility.java +++ b/src/com/fsck/k9/mail/internet/MimeUtility.java @@ -1061,7 +1061,18 @@ public class MimeUtility { * the stream is now wrapped we'll remove any transfer encoding at this point. */ InputStream in = part.getBody().getInputStream(); - return readToString(in, charset); + try { + String text = readToString(in, charset); + + // Replace the body with a TextBody that already contains the decoded text + part.setBody(new TextBody(text)); + + return text; + } finally { + try { + in.close(); + } catch (IOException e) { /* Ignore */ } + } } } From f42943f30c5bb8a1e782427685d8b8b3f575f83f Mon Sep 17 00:00:00 2001 From: cketti Date: Thu, 6 Sep 2012 22:33:22 +0200 Subject: [PATCH 8/8] Optimized searching for a message in the message list --- src/com/fsck/k9/activity/MessageList.java | 58 ++++++++++++++++++----- src/com/fsck/k9/helper/MessageHelper.java | 2 +- 2 files changed, 47 insertions(+), 13 deletions(-) diff --git a/src/com/fsck/k9/activity/MessageList.java b/src/com/fsck/k9/activity/MessageList.java index 03c061729..281ddf747 100644 --- a/src/com/fsck/k9/activity/MessageList.java +++ b/src/com/fsck/k9/activity/MessageList.java @@ -2203,23 +2203,57 @@ public class MessageList } } + /** + * Find a specific message in the message list. + * + *

Note: + * This method was optimized because it is called a lot. Don't change it unless you know + * what you are doing.

+ * + * @param message + * A {@link Message} instance describing the message to look for. + * + * @return The corresponding {@link MessageInfoHolder} instance if the message was found in + * the message list. {@code null} otherwise. + */ private MessageInfoHolder getMessage(Message message) { - return getMessage(message.makeMessageReference()); + String uid; + Folder folder; + for (MessageInfoHolder holder : mMessages) { + uid = message.getUid(); + if (holder.uid == uid || uid.equals(holder.uid)) { + folder = message.getFolder(); + if (holder.folder.name.equals(folder.getName()) && + holder.account.equals(folder.getAccount().getUuid())) { + return holder; + } + } + } + + return null; } - // XXX TODO - make this not use a for loop + /** + * Find a specific message in the message list. + * + *

Note: + * This method was optimized because it is called a lot. Don't change it unless you know + * what you are doing.

+ * + * @param messageReference + * A {@link MessageReference} instance describing the message to look for. + * + * @return The corresponding {@link MessageInfoHolder} instance if the message was found in + * the message list. {@code null} otherwise. + */ private MessageInfoHolder getMessage(MessageReference messageReference) { + String uid; for (MessageInfoHolder holder : mMessages) { - /* - * 2010-06-21 - cketti - * Added null pointer check. Not sure what's causing 'holder' - * to be null. See log provided in issue 1749, comment #15. - * - * Please remove this comment once the cause was found and the - * bug(?) fixed. - */ - if ((holder != null) && holder.message.equalsReference(messageReference)) { - return holder; + uid = messageReference.uid; + if ((holder.uid == uid || uid.equals(holder.uid)) && + holder.folder.name.equals(messageReference.folderName) && + holder.account.equals(messageReference.accountUuid)) { + return holder; } } diff --git a/src/com/fsck/k9/helper/MessageHelper.java b/src/com/fsck/k9/helper/MessageHelper.java index 09cfad87e..15394c339 100644 --- a/src/com/fsck/k9/helper/MessageHelper.java +++ b/src/com/fsck/k9/helper/MessageHelper.java @@ -85,7 +85,7 @@ public class MessageHelper { target.uid = message.getUid(); - target.account = account.getDescription(); + target.account = account.getUuid(); target.uri = "email://messages/" + account.getAccountNumber() + "/" + m.getFolder().getName() + "/" + m.getUid(); } catch (MessagingException me) {