Save the focused conversation via id instead of position

Don't rely on saving the position in the adapter because the position
might change as the adapter's data set changes (e.g. new mails via sync),
and we have no way of modifying the focused position accordingly.

Instead, save the focused conversation with its id (tried using the uri,
but the uri might change when the conversation gets cached). This way,
no matter what happens to the items in the adapter, the focused item
remains consistent.

b/18027602

Change-Id: I51aa68bc15c5892c3b34bde5c199de281390ce7b

6 files changed, 72 insertions, 53 deletions

diff --git a/src/com/android/mail/browse/ConversationItemView.java b/src/com/android/mail/browse/ConversationItemView.java
index 65cf5ce33..e2dcf472f 100644
--- a/src/com/android/mail/browse/ConversationItemView.java
+++ b/src/com/android/mail/browse/ConversationItemView.java
@@ -58,7 +58,6 @@ import android.view.View;
 import android.view.ViewGroup;
 import android.view.ViewParent;
 import android.view.animation.DecelerateInterpolator;
-import android.widget.ListView;
 import android.widget.TextView;
 
 import com.android.mail.R;
@@ -197,7 +196,6 @@ public class ConversationItemView extends View
     private final TextView mSubjectTextView;
     private final TextView mSnippetTextView;
     private int mGadgetMode;
-    private int mAdapterPosition = ListView.INVALID_POSITION;
 
     private static int sFoldersMaxCount;
     private static TextAppearanceSpan sSubjectTextUnreadSpan;
@@ -472,9 +470,8 @@ public class ConversationItemView extends View
             final ConversationCheckedSet set, final Folder folder,
             final int checkboxOrSenderImage,
             final boolean swipeEnabled, final boolean importanceMarkersEnabled,
-            final boolean showChevronsEnabled, final AnimatedAdapter adapter, int position) {
+            final boolean showChevronsEnabled, final AnimatedAdapter adapter) {
         Utils.traceBeginSection("CIVC.bind");
-        mAdapterPosition = position;
         bind(ConversationItemViewModel.forConversation(mAccount.getEmailAddress(), conversation),
                 activity, null /* conversationItemAreaClickListener */,
                 set, folder, checkboxOrSenderImage, swipeEnabled, importanceMarkersEnabled,
@@ -1277,7 +1274,7 @@ public class ConversationItemView extends View
 
         // The focused bar
         final SwipeableListView listView = getListView();
-        if (listView != null && listView.isPositionSelected(getViewPosition())) {
+        if (listView != null && listView.isConversationSelected(getConversation())) {
             final int w = FOCUSED_CONVERSATION_HIGHLIGHT.getIntrinsicWidth();
             final boolean isRtl = ViewUtils.isViewRtl(this);
             // This bar is on the right side of the conv list if it's RTL
@@ -1299,26 +1296,12 @@ public class ConversationItemView extends View
         if (selected) {
             final SwipeableListView listView = getListView();
             if (listView != null) {
-                listView.setSelectedPosition(getViewPosition());
+                listView.setSelectedConversation(getConversation());
             }
         }
         super.setSelected(selected);
     }
 
-    private int getViewPosition() {
-        if (mAdapterPosition != ListView.INVALID_POSITION) {
-            return mAdapterPosition;
-        } else {
-            final ListView listView = getListView();
-            final int position = listView != null ?
-                    listView.getPositionForView(this) : ListView.INVALID_POSITION;
-            LogUtils.e(LOG_TAG,
-                    "ConversationItemView didn't set position, using listview's position: %d",
-                    position);
-            return position;
-        }
-    }
-
     private void drawSendersImage(final Canvas canvas) {
         if (!mSendersImageView.isFlipping()) {
             final boolean showSenders = !mChecked;
@@ -1405,7 +1388,8 @@ public class ConversationItemView extends View
             final SwipeableListView listView = getListView();
 
             try {
-                conv.position = mChecked ? getViewPosition() : Conversation.NO_POSITION;
+                conv.position = mChecked && listView != null ? listView.getPositionForView(this)
+                        : Conversation.NO_POSITION;
             } catch (final NullPointerException e) {
                 // TODO(skennedy) Remove this if we find the root cause b/9527863
             }
diff --git a/src/com/android/mail/browse/SwipeableConversationItemView.java b/src/com/android/mail/browse/SwipeableConversationItemView.java
index 9db966906..10657f4f1 100644
--- a/src/com/android/mail/browse/SwipeableConversationItemView.java
+++ b/src/com/android/mail/browse/SwipeableConversationItemView.java
@@ -56,10 +56,9 @@ public class SwipeableConversationItemView extends FrameLayout implements Toggle
             final ConversationCheckedSet set, final Folder folder,
             final int checkboxOrSenderImage, boolean swipeEnabled,
             final boolean importanceMarkersEnabled, final boolean showChevronsEnabled,
-            final AnimatedAdapter animatedAdapter, final int position) {
+            final AnimatedAdapter animatedAdapter) {
         mConversationItemView.bind(conversation, activity, set, folder, checkboxOrSenderImage,
-                swipeEnabled, importanceMarkersEnabled, showChevronsEnabled, animatedAdapter,
-                position);
+                swipeEnabled, importanceMarkersEnabled, showChevronsEnabled, animatedAdapter);
     }
 
     public void startUndoAnimation(AnimatorListener listener, boolean swipe) {
diff --git a/src/com/android/mail/ui/AnimatedAdapter.java b/src/com/android/mail/ui/AnimatedAdapter.java
index 86a3658bb..2b38d27f8 100644
--- a/src/com/android/mail/ui/AnimatedAdapter.java
+++ b/src/com/android/mail/ui/AnimatedAdapter.java
@@ -380,12 +380,12 @@ public class AnimatedAdapter extends SimpleCursorAdapter {
     }
 
     public View createConversationItemView(SwipeableConversationItemView view, Context context,
-            Conversation conv, int position) {
+            Conversation conv) {
         if (view == null) {
             view = new SwipeableConversationItemView(context, mAccount);
         }
         view.bind(conv, mActivity, mBatchConversations, mFolder, getCheckboxSetting(),
-                mSwipeEnabled, mImportanceMarkersEnabled, mShowChevronsEnabled, this, position);
+                mSwipeEnabled, mImportanceMarkersEnabled, mShowChevronsEnabled, this);
         return view;
     }
 
@@ -556,7 +556,7 @@ public class AnimatedAdapter extends SimpleCursorAdapter {
             ((SwipeableConversationItemView) convertView).reset();
         }
         final View v = createConversationItemView((SwipeableConversationItemView) convertView,
-                mContext, conv, position);
+                mContext, conv);
         Utils.traceEndSection();
         return v;
     }
@@ -790,7 +790,7 @@ public class AnimatedAdapter extends SimpleCursorAdapter {
                 position, null, parent);
         view.reset();
         view.bind(conversation, mActivity, mBatchConversations, mFolder, getCheckboxSetting(),
-                mSwipeEnabled, mImportanceMarkersEnabled, mShowChevronsEnabled, this, position);
+                mSwipeEnabled, mImportanceMarkersEnabled, mShowChevronsEnabled, this);
         mAnimatingViews.put(conversation.id, view);
         return view;
     }
diff --git a/src/com/android/mail/ui/ConversationListFragment.java b/src/com/android/mail/ui/ConversationListFragment.java
index d80cdf9ad..439f6b597 100644
--- a/src/com/android/mail/ui/ConversationListFragment.java
+++ b/src/com/android/mail/ui/ConversationListFragment.java
@@ -493,7 +493,7 @@ public final class ConversationListFragment extends Fragment implements
         sb.append(mViewContext.folder);
         if (mListView != null) {
             sb.append(" selectedPos=");
-            sb.append(mListView.getSelectedPosition());
+            sb.append(mListView.getSelectedConversationPosDebug());
             sb.append(" listSelectedPos=");
             sb.append(mListView.getSelectedItemPosition());
             sb.append(" isListInTouchMode=");
@@ -858,7 +858,7 @@ public final class ConversationListFragment extends Fragment implements
          * special views in the list.
          */
             conv.position = cursor.getPosition();
-            setActivated(conv.position, true);
+            setActivated(conv, true);
             mCallbacks.onConversationSelected(conv, false /* inLoaderCallbacks */);
         } else {
             LogUtils.e(LOG_TAG,
@@ -869,41 +869,41 @@ public final class ConversationListFragment extends Fragment implements
 
     /**
      * Sets the checked conversation to the position given here.
-     * @param cursorPosition The position of the conversation in the cursor (as opposed to
-     * in the list)
+     * @param conversation the activated conversation.
      * @param different if the currently checked conversation is different from the one provided
      * here.  This is a difference in conversations, not a difference in positions. For example, a
      * conversation at position 2 can move to position 4 as a result of new mail.
      */
-    public void setActivated(final int cursorPosition, boolean different) {
-        if (mListView.getChoiceMode() == ListView.CHOICE_MODE_NONE) {
+    public void setActivated(final Conversation conversation, boolean different) {
+        if (mListView.getChoiceMode() == ListView.CHOICE_MODE_NONE || conversation == null) {
             return;
         }
 
+        final int cursorPosition = conversation.position;
         final int position = cursorPosition + mListAdapter.getPositionOffset(cursorPosition);
         setRawActivated(position, different);
-        setRawSelected(position);
+        setRawSelected(conversation, position);
     }
 
     /**
      * Set the selected conversation (used by the framework to indicate current focus in the list).
-     * @param cursorPosition The position of the conversation in the cursor (as opposed to
-     * in the list)
+     * @param conversation the selected conversation.
      */
-    public void setSelected(final int cursorPosition) {
-        if (mListView.getChoiceMode() == ListView.CHOICE_MODE_NONE) {
+    public void setSelected(final Conversation conversation) {
+        if (mListView.getChoiceMode() == ListView.CHOICE_MODE_NONE || conversation == null) {
             return;
         }
 
+        final int cursorPosition = conversation.position;
         final int position = cursorPosition + mListAdapter.getPositionOffset(cursorPosition);
-        setRawSelected(position);
+        setRawSelected(conversation, position);
     }
 
     /**
      * Set the selected conversation (used by the framework to indicate current focus in the list).
      * @param position The position of the item in the list
      */
-    private void setRawSelected(final int position) {
+    private void setRawSelected(Conversation conversation, final int position) {
         final View selectedView = mListView.getChildAt(
                 position - mListView.getFirstVisiblePosition());
         // Don't do anything if the view is already selected.
@@ -919,7 +919,7 @@ public final class ConversationListFragment extends Fragment implements
                 // setSelection calls setSelectionFromTop with y = 0.
                 mListView.setSelectionFromTop(position, selectedView.getTop());
             }
-            mListView.setSelectedPosition(position);
+            mListView.setSelectedConversation(conversation);
         }
     }
 
@@ -1154,7 +1154,7 @@ public final class ConversationListFragment extends Fragment implements
         if (conv != null && !currentConvIsPeeking) {
             if (mListView.getChoiceMode() != ListView.CHOICE_MODE_NONE
                     && mListView.getCheckedItemPosition() == -1) {
-                setActivated(conv.position, true);
+                setActivated(conv, true);
             }
         }
     }
diff --git a/src/com/android/mail/ui/SwipeableListView.java b/src/com/android/mail/ui/SwipeableListView.java
index b8406b3d8..39429d03b 100644
--- a/src/com/android/mail/ui/SwipeableListView.java
+++ b/src/com/android/mail/ui/SwipeableListView.java
@@ -49,6 +49,8 @@ import java.util.Collection;
 import java.util.HashMap;
 
 public class SwipeableListView extends ListView implements Callback, OnScrollListener {
+    private static final long INVALID_CONVERSATION_ID = -1;
+
     private final SwipeHelper mSwipeHelper;
     /**
      * Are swipes enabled on all items? (Each individual item can still prevent swiping.)<br>
@@ -72,7 +74,7 @@ public class SwipeableListView extends ListView implements Callback, OnScrollLis
 
     private SwipeListener mSwipeListener;
 
-    private int mSelectedPosition = ListView.INVALID_POSITION;
+    private long mSelectedConversationId = INVALID_CONVERSATION_ID;
 
     // Instantiated through view inflation
     @SuppressWarnings("unused")
@@ -411,20 +413,54 @@ public class SwipeableListView extends ListView implements Callback, OnScrollLis
     /**
      * Set the currently selected (focused by the list view) position.
      */
-    public void setSelectedPosition(int position) {
-        if (position == ListView.INVALID_POSITION) {
+    public void setSelectedConversation(Conversation conv) {
+        if (conv == null) {
             return;
         }
 
-        mSelectedPosition = position;
+        mSelectedConversationId = conv.id;
+    }
+
+    public boolean isConversationSelected(Conversation conv) {
+        return mSelectedConversationId != INVALID_CONVERSATION_ID && conv != null
+                && mSelectedConversationId == conv.id;
     }
 
-    public boolean isPositionSelected(int position) {
-        return mSelectedPosition != ListView.INVALID_POSITION && mSelectedPosition == position;
+    /**
+     * This is only used for debugging/logging purposes. DO NOT call this function to try to get
+     * the currently selected position. Use {@link #mSelectedConversationId} instead.
+     */
+    public int getSelectedConversationPosDebug() {
+        for (int i = getFirstVisiblePosition(); i < getLastVisiblePosition(); i++) {
+            final Object item = getItemAtPosition(i);
+            if (item instanceof ConversationCursor) {
+                final Conversation c = ((ConversationCursor) item).getConversation();
+                if (c.id == mSelectedConversationId) {
+                    return i;
+                }
+            }
+        }
+        return ListView.INVALID_POSITION;
     }
 
-    public int getSelectedPosition() {
-        return mSelectedPosition;
+    @Override
+    public void onTouchModeChanged(boolean isInTouchMode) {
+        super.onTouchModeChanged(isInTouchMode);
+        if (!isInTouchMode) {
+            // We need to invalidate going from touch mode -> keyboard mode because the currently
+            // selected item might have changed in touch mode. However, since from the framework's
+            // perspective the selected position doesn't matter in touch mode, when we enter
+            // keyboard mode via up/down arrow, the list view will ONLY invalidate the newly
+            // selected item and not the currently selected item. As a result, we might get an
+            // inconsistent UI where it looks like both the old and new selected items are focused.
+            final int index = getSelectedItemPosition();
+            if (index != ListView.INVALID_POSITION) {
+                final View child = getChildAt(index - getFirstVisiblePosition());
+                if (child != null) {
+                    child.invalidate();
+                }
+            }
+        }
     }
 
     @Override
diff --git a/src/com/android/mail/ui/TwoPaneController.java b/src/com/android/mail/ui/TwoPaneController.java
index 2af141a75..12737c41d 100644
--- a/src/com/android/mail/ui/TwoPaneController.java
+++ b/src/com/android/mail/ui/TwoPaneController.java
@@ -653,9 +653,9 @@ public final class TwoPaneController extends AbstractActivityController implemen
         if (different && convList != null && conversation != null) {
             if (mCurrentConversationJustPeeking) {
                 convList.clearChoicesAndActivated();
-                convList.setSelected(conversation.position);
+                convList.setSelected(conversation);
             } else {
-                convList.setActivated(conversation.position, different);
+                convList.setActivated(conversation, different);
             }
         }
     }