Bug 1288127 - Handle pure bookmark cursors in CombinedHistoryAdapter too r=grisha draft
authorAndrzej Hunt <ahunt@mozilla.com>
Thu, 28 Jul 2016 11:00:59 -0700
changeset 399343 158a8c8ad6241382310711dbacc07bf5342f9fee
parent 399342 141ccc00ae58c9192e44e226476a13e7c68a231b
child 399344 0fe680d7ebc47e0eb03c8e56b1d6d9a85bccb1ea
push id25805
push userahunt@mozilla.com
push dateWed, 10 Aug 2016 22:18:08 +0000
reviewersgrisha
bugs1288127
milestone51.0a1
Bug 1288127 - Handle pure bookmark cursors in CombinedHistoryAdapter too r=grisha MozReview-Commit-ID: DF72uaBkghd
mobile/android/base/java/org/mozilla/gecko/home/CombinedHistoryAdapter.java
--- a/mobile/android/base/java/org/mozilla/gecko/home/CombinedHistoryAdapter.java
+++ b/mobile/android/base/java/org/mozilla/gecko/home/CombinedHistoryAdapter.java
@@ -228,25 +228,39 @@ public class CombinedHistoryAdapter exte
             case SECTION_HEADER:
                 // We might have multiple section headers, so we try get unique IDs for them.
                 return position * PRIME_SECTION_HEADERS;
             case HISTORY:
                 if (!historyCursor.moveToPosition(position)) {
                     return RecyclerView.NO_ID;
                 }
 
-                final int historyIdCol = historyCursor.getColumnIndexOrThrow(BrowserContract.Combined.HISTORY_ID);
-                final long historyId = historyCursor.getLong(historyIdCol);
+                final int historyIdCol = historyCursor.getColumnIndex(BrowserContract.Combined.HISTORY_ID);
 
-                if (historyId != -1) {
-                    return historyId;
+                if (historyIdCol != -1) {
+                    final long historyId = historyCursor.getLong(historyIdCol);
+
+                    if (historyId != -1) {
+                        return historyId;
+                    }
                 }
 
-                final int bookmarkIdCol = historyCursor.getColumnIndexOrThrow(BrowserContract.Combined.BOOKMARK_ID);
-                final long bookmarkId = historyCursor.getLong(bookmarkIdCol);
+                final int bookmarkIdCol = historyCursor.getColumnIndex(BrowserContract.Combined.BOOKMARK_ID);
+                final long bookmarkId;
+
+                // We are handling two types of Cursor: a pure bookmarks cursor, or a combined
+                // (history/bookmarks) table Cursor. In the combined case we have a standalone bookmark-ID
+                // column (in addition to history ID, see above). If that column exists we know
+                // we have combined cursor, and can read the bookmark ID column. If that column does
+                // not exist, we know we have a pure bookmarks cursor and can read the _ID column.
+                if (bookmarkIdCol != -1) {
+                    bookmarkId = historyCursor.getLong(bookmarkIdCol);
+                } else {
+                    bookmarkId = historyCursor.getLong(historyCursor.getColumnIndexOrThrow(BrowserContract.Bookmarks._ID));
+                }
 
                 // Avoid clashing with historyId.
                 return bookmarkId * PRIME_BOOKMARKS;
             default:
                 throw new IllegalStateException("Unexpected Home Panel item type");
         }
     }
 
@@ -262,17 +276,22 @@ public class CombinedHistoryAdapter exte
         if (c == null || !c.moveToFirst()) {
             return;
         }
 
         SectionHeader section = null;
 
         do {
             final int historyPosition = c.getPosition();
-            final long visitTime = c.getLong(c.getColumnIndexOrThrow(BrowserContract.History.DATE_LAST_VISITED));
+            final long visitTime;
+            if (c.getColumnIndex(BrowserContract.History.DATE_LAST_VISITED) != -1) {
+                visitTime = c.getLong(c.getColumnIndexOrThrow(BrowserContract.History.DATE_LAST_VISITED));
+            } else {
+                visitTime = c.getLong(c.getColumnIndexOrThrow(BrowserContract.Bookmarks.DATE_CREATED));
+            }
             final SectionHeader itemSection = getSectionFromTime(visitTime);
 
             if (section != itemSection) {
                 section = itemSection;
                 sparseArray.append(historyPosition + sparseArray.size() + getNumSmartFolders(), section);
             }
 
             if (section == SectionHeader.OLDER_THAN_SIX_MONTHS) {
@@ -321,22 +340,32 @@ public class CombinedHistoryAdapter exte
             return populateHistoryInfoFromCursor(info, historyCursor);
         }
         return null;
     }
 
     protected static HomeContextMenuInfo populateHistoryInfoFromCursor(HomeContextMenuInfo info, Cursor cursor) {
         info.url = cursor.getString(cursor.getColumnIndexOrThrow(BrowserContract.Combined.URL));
         info.title = cursor.getString(cursor.getColumnIndexOrThrow(BrowserContract.Combined.TITLE));
-        info.historyId = cursor.getInt(cursor.getColumnIndexOrThrow(BrowserContract.Combined.HISTORY_ID));
+        final int historyIdCol = cursor.getColumnIndex(BrowserContract.Combined.HISTORY_ID);
+        if (historyIdCol == -1 || cursor.isNull(historyIdCol)) {
+            info.historyId = -1;
+        } else {
+            info.historyId = cursor.getInt(historyIdCol);
+        }
         info.itemType = HomeContextMenuInfo.RemoveItemType.HISTORY;
-        final int bookmarkIdCol = cursor.getColumnIndexOrThrow(BrowserContract.Combined.BOOKMARK_ID);
-        if (cursor.isNull(bookmarkIdCol)) {
+
+        final int bookmarkIdCol = cursor.getColumnIndex(BrowserContract.Combined.BOOKMARK_ID);
+        if (bookmarkIdCol == -1) {
+            // This will happen if we have a bookmarks-only cursor, in which case we must have a bookmark ID:
+            info.bookmarkId = cursor.getInt(cursor.getColumnIndexOrThrow(BrowserContract.Bookmarks._ID));
+            info.itemType = HomeContextMenuInfo.RemoveItemType.BOOKMARKS;
+        } else if (cursor.isNull(bookmarkIdCol)) {
             // If this is a combined cursor, we may get a history item without a
             // bookmark, in which case the bookmarks ID column value will be null.
-            info.bookmarkId =  -1;
+            info.bookmarkId = -1;
         } else {
             info.bookmarkId = cursor.getInt(bookmarkIdCol);
         }
         return info;
     }
 
 }