Fix bug 5399568 - ListView check states inconsistent after data set change

adamp · adamp · commit 14c080497e42 · 2011-10-06T19:55:42.000-07:00
ListView tracks check states in two ways, by position and by ID if an
adapter reports stable IDs. After a data set change there was no
guarantee that the position checked mapping was consistent.

Fix up the position mapping from the ID mapping after a data set
change. In the future this should happen by asking the adapter where a
given ID is now located, but this will require new API and not all
adapters in the wild will implement it. For now make a best guess by
searching in a limited window around the item's last known position.

Change-Id: I70ba89eb103c438b0410c3c6d066acc3918459f9
diff --git a/core/java/android/widget/AbsListView.java b/core/java/android/widget/AbsListView.java
@@ -237,9 +237,11 @@ public abstract class AbsListView extends AdapterView<ListAdapter> implements Te
     SparseBooleanArray mCheckStates;
 
     /**
-     * Running state of which IDs are currently checked
+     * Running state of which IDs are currently checked.
+     * If there is a value for a given key, the checked state for that ID is true
+     * and the value holds the last known position in the adapter for that id.
      */
-    LongSparseArray<Boolean> mCheckedIdStates;
+    LongSparseArray<Integer> mCheckedIdStates;
 
     /**
      * Controls how the next layout will happen
@@ -471,6 +473,13 @@ public abstract class AbsListView extends AdapterView<ListAdapter> implements Te
      */
     static final int OVERSCROLL_LIMIT_DIVISOR = 3;
 
+    /**
+     * How many positions in either direction we will search to try to
+     * find a checked item with a stable ID that moved position across
+     * a data set change. If the item isn't found it will be unselected.
+     */
+    private static final int CHECK_POSITION_SEARCH_DISTANCE = 20;
+
     /**
      * Used to request a layout when we changed touch mode
      */
@@ -806,7 +815,7 @@ public void setAdapter(ListAdapter adapter) {
         if (adapter != null) {
             if (mChoiceMode != CHOICE_MODE_NONE && mAdapter.hasStableIds() &&
                     mCheckedIdStates == null) {
-                mCheckedIdStates = new LongSparseArray<Boolean>();
+                mCheckedIdStates = new LongSparseArray<Integer>();
             }
         }
 
@@ -901,7 +910,7 @@ public long[] getCheckedItemIds() {
             return new long[0];
         }
 
-        final LongSparseArray<Boolean> idStates = mCheckedIdStates;
+        final LongSparseArray<Integer> idStates = mCheckedIdStates;
         final int count = idStates.size();
         final long[] ids = new long[count];
 
@@ -948,7 +957,7 @@ public void setItemChecked(int position, boolean value) {
             mCheckStates.put(position, value);
             if (mCheckedIdStates != null && mAdapter.hasStableIds()) {
                 if (value) {
-                    mCheckedIdStates.put(mAdapter.getItemId(position), Boolean.TRUE);
+                    mCheckedIdStates.put(mAdapter.getItemId(position), position);
                 } else {
                     mCheckedIdStates.delete(mAdapter.getItemId(position));
                 }
@@ -980,7 +989,7 @@ public void setItemChecked(int position, boolean value) {
             if (value) {
                 mCheckStates.put(position, true);
                 if (updateIds) {
-                    mCheckedIdStates.put(mAdapter.getItemId(position), Boolean.TRUE);
+                    mCheckedIdStates.put(mAdapter.getItemId(position), position);
                 }
                 mCheckedItemCount = 1;
             } else if (mCheckStates.size() == 0 || !mCheckStates.valueAt(0)) {
@@ -1010,7 +1019,7 @@ public boolean performItemClick(View view, int position, long id) {
                 mCheckStates.put(position, newValue);
                 if (mCheckedIdStates != null && mAdapter.hasStableIds()) {
                     if (newValue) {
-                        mCheckedIdStates.put(mAdapter.getItemId(position), Boolean.TRUE);
+                        mCheckedIdStates.put(mAdapter.getItemId(position), position);
                     } else {
                         mCheckedIdStates.delete(mAdapter.getItemId(position));
                     }
@@ -1032,7 +1041,7 @@ public boolean performItemClick(View view, int position, long id) {
                     mCheckStates.put(position, true);
                     if (mCheckedIdStates != null && mAdapter.hasStableIds()) {
                         mCheckedIdStates.clear();
-                        mCheckedIdStates.put(mAdapter.getItemId(position), Boolean.TRUE);
+                        mCheckedIdStates.put(mAdapter.getItemId(position), position);
                     }
                     mCheckedItemCount = 1;
                 } else if (mCheckStates.size() == 0 || !mCheckStates.valueAt(0)) {
@@ -1081,7 +1090,7 @@ public void setChoiceMode(int choiceMode) {
                 mCheckStates = new SparseBooleanArray();
             }
             if (mCheckedIdStates == null && mAdapter != null && mAdapter.hasStableIds()) {
-                mCheckedIdStates = new LongSparseArray<Boolean>();
+                mCheckedIdStates = new LongSparseArray<Integer>();
             }
             // Modal multi-choice mode only has choices when the mode is active. Clear them.
             if (mChoiceMode == CHOICE_MODE_MULTIPLE_MODAL) {
@@ -1427,7 +1436,7 @@ static class SavedState extends BaseSavedState {
         boolean inActionMode;
         int checkedItemCount;
         SparseBooleanArray checkState;
-        LongSparseArray<Boolean> checkIdState;
+        LongSparseArray<Integer> checkIdState;
 
         /**
          * Constructor called from {@link AbsListView#onSaveInstanceState()}
@@ -1451,10 +1460,14 @@ private SavedState(Parcel in) {
             checkedItemCount = in.readInt();
             checkState = in.readSparseBooleanArray();
             long[] idState = in.createLongArray();
+            int[] idPositions = in.createIntArray();
 
-            if (idState.length > 0) {
-                checkIdState = new LongSparseArray<Boolean>();
-                checkIdState.setValues(idState, Boolean.TRUE);
+            final int idLength = idState.length;
+            if (idLength > 0) {
+                checkIdState = new LongSparseArray<Integer>();
+                for (int i = 0; i < idLength; i++) {
+                    checkIdState.put(idState[i], idPositions[i]);
+                }
             }
         }
 
@@ -1471,6 +1484,15 @@ public void writeToParcel(Parcel out, int flags) {
             out.writeInt(checkedItemCount);
             out.writeSparseBooleanArray(checkState);
             out.writeLongArray(checkIdState != null ? checkIdState.getKeys() : new long[0]);
+
+            int size = checkIdState != null ? checkIdState.size() : 0;
+            int[] idPositions = new int[size];
+            if (size > 0) {
+                for (int i = 0; i < size; i++) {
+                    idPositions[i] = checkIdState.valueAt(i);
+                }
+                out.writeIntArray(idPositions);
+            }
         }
 
         @Override
@@ -1565,7 +1587,7 @@ public Parcelable onSaveInstanceState() {
             ss.checkState = mCheckStates.clone();
         }
         if (mCheckedIdStates != null) {
-            final LongSparseArray<Boolean> idState = new LongSparseArray<Boolean>();
+            final LongSparseArray<Integer> idState = new LongSparseArray<Integer>();
             final int count = mCheckedIdStates.size();
             for (int i = 0; i < count; i++) {
                 idState.put(mCheckedIdStates.keyAt(i), mCheckedIdStates.valueAt(i));
@@ -4786,15 +4808,63 @@ boolean resurrectSelection() {
         return selectedPos >= 0;
     }
 
+    void confirmCheckedPositionsById() {
+        // Clear out the positional check states, we'll rebuild it below from IDs.
+        mCheckStates.clear();
+
+        boolean checkedCountChanged = false;
+        for (int checkedIndex = 0; checkedIndex < mCheckedIdStates.size(); checkedIndex++) {
+            final long id = mCheckedIdStates.keyAt(checkedIndex);
+            final int lastPos = mCheckedIdStates.valueAt(checkedIndex);
+
+            final long lastPosId = mAdapter.getItemId(lastPos);
+            if (id != lastPosId) {
+                // Look around to see if the ID is nearby. If not, uncheck it.
+                final int start = Math.max(0, lastPos - CHECK_POSITION_SEARCH_DISTANCE);
+                final int end = Math.min(lastPos + CHECK_POSITION_SEARCH_DISTANCE, mItemCount);
+                boolean found = false;
+                for (int searchPos = start; searchPos < end; searchPos++) {
+                    final long searchId = mAdapter.getItemId(searchPos);
+                    if (id == searchId) {
+                        found = true;
+                        mCheckStates.put(searchPos, true);
+                        mCheckedIdStates.setValueAt(checkedIndex, searchPos);
+                        break;
+                    }
+                }
+
+                if (!found) {
+                    mCheckedIdStates.delete(id);
+                    checkedIndex--;
+                    mCheckedItemCount--;
+                    checkedCountChanged = true;
+                    if (mChoiceActionMode != null && mMultiChoiceModeCallback != null) {
+                        mMultiChoiceModeCallback.onItemCheckedStateChanged(mChoiceActionMode,
+                                lastPos, id, false);
+                    }
+                }
+            } else {
+                mCheckStates.put(lastPos, true);
+            }
+        }
+
+        if (checkedCountChanged && mChoiceActionMode != null) {
+            mChoiceActionMode.invalidate();
+        }
+    }
+
     @Override
     protected void handleDataChanged() {
         int count = mItemCount;
         int lastHandledItemCount = mLastHandledItemCount;
         mLastHandledItemCount = mItemCount;
-        if (count > 0) {
 
-            int newPos;
+        if (mChoiceMode != CHOICE_MODE_NONE && mAdapter != null && mAdapter.hasStableIds()) {
+            confirmCheckedPositionsById();
+        }
 
+        if (count > 0) {
+            int newPos;
             int selectablePos;
 
             // Find the row we are supposed to sync to
