Bug 1278644 - Stable getItemId implementation for VisitedAdapter r=sebastian draft
authorGrigory Kruglov <gkruglov@mozilla.com>
Tue, 28 Jun 2016 16:04:09 -0700
changeset 382129 2af8f4e2b77977b2d634c8f013aaa5ef5b3ee517
parent 382128 2da5ae91c3f8cc13078a6b1b252463cf2681d2e7
child 382130 34e97821cbcf4211ebd87317435be8e5dfeec58a
push id21631
push usergkruglov@mozilla.com
push dateWed, 29 Jun 2016 00:20:45 +0000
reviewerssebastian
bugs1278644
milestone50.0a1
Bug 1278644 - Stable getItemId implementation for VisitedAdapter r=sebastian MozReview-Commit-ID: AwiwGAVRA3k
mobile/android/base/java/org/mozilla/gecko/home/TopSitesPanel.java
--- a/mobile/android/base/java/org/mozilla/gecko/home/TopSitesPanel.java
+++ b/mobile/android/base/java/org/mozilla/gecko/home/TopSitesPanel.java
@@ -521,16 +521,29 @@ public class TopSitesPanel extends HomeF
             return Math.max(0, super.getCount() - mMaxGridEntries);
         }
 
         @Override
         public Object getItem(int position) {
             return super.getItem(position + mMaxGridEntries);
         }
 
+        /**
+         * We have to override default getItemId implementation, since for a given position, it returns
+         * value of the _id column. In our case _id is always 0 (see Combined view).
+         */
+        @Override
+        public long getItemId(int position) {
+            final int adjustedPosition = position + mMaxGridEntries;
+            final Cursor cursor = getCursor();
+
+            cursor.moveToPosition(adjustedPosition);
+            return getItemIdForTopSitesCursor(cursor);
+        }
+
         @Override
         public void bindView(View view, Context context, Cursor cursor) {
             final int position = cursor.getPosition();
             cursor.moveToPosition(position + mMaxGridEntries);
 
             final TwoLinePageRow row = (TwoLinePageRow) view;
             row.updateFromCursor(cursor);
         }
@@ -580,33 +593,26 @@ public class TopSitesPanel extends HomeF
                 // This will force each view to load favicons for the missing
                 // thumbnails if necessary.
                 gridItem.markAsDirty();
             }
 
             notifyDataSetChanged();
         }
 
+        /**
+         * We have to override default getItemId implementation, since for a given position, it returns
+         * value of the _id column. In our case _id is always 0 (see Combined view).
+         */
         @Override
         public long getItemId(int position) {
-            // We are trying to return stable ids so that Android can recycle views appropriately:
-            // * If we have a history id then we return it
-            // * If we only have a bookmark id then we negate it and return it. We negate it in order
-            //   to avoid clashing/conflicting with history ids.
-
             final Cursor cursor = getCursor();
             cursor.moveToPosition(position);
 
-            final long historyId = cursor.getLong(cursor.getColumnIndexOrThrow(TopSites.HISTORY_ID));
-            if (historyId != 0) {
-                return historyId;
-            }
-
-            final long bookmarkId = cursor.getLong(cursor.getColumnIndexOrThrow(TopSites.BOOKMARK_ID));
-            return -1 * bookmarkId;
+            return getItemIdForTopSitesCursor(cursor);
         }
 
         @Override
         public void bindView(View bindView, Context context, Cursor cursor) {
             final String url = cursor.getString(cursor.getColumnIndexOrThrow(TopSites.URL));
             final String title = cursor.getString(cursor.getColumnIndexOrThrow(TopSites.TITLE));
             final int type = cursor.getInt(cursor.getColumnIndexOrThrow(TopSites.TYPE));
 
@@ -960,9 +966,30 @@ public class TopSitesPanel extends HomeF
 
         @Override
         public void onLoaderReset(Loader<Map<String, ThumbnailInfo>> loader) {
             if (mGridAdapter != null) {
                 mGridAdapter.updateThumbnails(null);
             }
         }
     }
+
+    /**
+     * We are trying to return stable IDs so that Android can recycle views appropriately:
+     * - If we have a history ID then we return it
+     * - If we only have a bookmark ID then we negate it and return it. We negate it in order
+     *   to avoid clashing/conflicting with history IDs.
+     *
+     * @param cursorInPosition Cursor already moved to position for which we're getting a stable ID
+     * @return Stable ID for a given cursor
+     */
+    private static long getItemIdForTopSitesCursor(final Cursor cursorInPosition) {
+        final int historyIdCol = cursorInPosition.getColumnIndexOrThrow(TopSites.HISTORY_ID);
+        final long historyId = cursorInPosition.getLong(historyIdCol);
+        if (historyId != 0) {
+            return historyId;
+        }
+
+        final int bookmarkIdCol = cursorInPosition.getColumnIndexOrThrow(TopSites.BOOKMARK_ID);
+        final long bookmarkId = cursorInPosition.getLong(bookmarkIdCol);
+        return -1 * bookmarkId;
+    }
 }