Bug 1254797 - Post: Ensure we ignore deleted sites in pinned query r=rnewman, a=ritu
authorAndrzej Hunt <andrzej@ahunt.org>
Tue, 15 Mar 2016 14:44:13 -0700
changeset 323712 876a1f819d83ef8035e579dd9247693c9526875b
parent 323711 d6ec426e0c96a9811881b1a0d5b43fc0707ea7a9
child 323713 4e5aa8d948fc7ea01db1b3d1fd5b0bf96c155978
push id5913
push userjlund@mozilla.com
push dateMon, 25 Apr 2016 16:57:49 +0000
treeherdermozilla-beta@dcaf0a6fa115 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrnewman, ritu
bugs1254797
milestone47.0a2
Bug 1254797 - Post: Ensure we ignore deleted sites in pinned query r=rnewman, a=ritu Pinned sites should be deleted directly, however I'm not confident enough in my knowledge of sync to be certain that we won't end up with deleted pinned sites in our table. (We use normal bookmark deletion for removing pinned sites.) MozReview-Commit-ID: SSLDkSXWlI
mobile/android/base/java/org/mozilla/gecko/db/BrowserProvider.java
--- a/mobile/android/base/java/org/mozilla/gecko/db/BrowserProvider.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/BrowserProvider.java
@@ -708,17 +708,19 @@ public class BrowserProvider extends Sha
         }
 
         if (gridLimitParam == null) {
             suggestedGridLimit = getContext().getResources().getInteger(R.integer.number_of_top_sites);
         } else {
             suggestedGridLimit = Integer.parseInt(gridLimitParam, 10);
         }
 
-        final String pinnedSitesFromClause = "FROM " + TABLE_BOOKMARKS + " WHERE " + Bookmarks.PARENT + " == " + Bookmarks.FIXED_PINNED_LIST_ID;
+        final String pinnedSitesFromClause = "FROM " + TABLE_BOOKMARKS + " WHERE " +
+                                             Bookmarks.PARENT + " == " + Bookmarks.FIXED_PINNED_LIST_ID +
+                                             " AND " + Bookmarks.IS_DELETED + " IS NOT 1";
 
         // Ideally we'd use a recursive CTE to generate our sequence, e.g. something like this worked at one point:
         // " WITH RECURSIVE" +
         // " cnt(x) AS (VALUES(1) UNION ALL SELECT x+1 FROM cnt WHERE x < 6)" +
         // However that requires SQLite >= 3.8.3 (available on Android >= 5.0), so in the meantime
         // we use a temporary numbers table.
         // Note: SQLite rowids are 1-indexed, whereas we're expecting 0-indexed values for the position. Our numbers
         // table starts at position = 0, which ensures the correct results here.
@@ -919,18 +921,17 @@ public class BrowserProvider extends Sha
                         Bookmarks._ID + ", " +
                         Bookmarks._ID + " AS " + TopSites.BOOKMARK_ID + ", " +
                         " -1 AS " + TopSites.HISTORY_ID + ", " +
                         Bookmarks.URL + ", " +
                         Bookmarks.TITLE + ", " +
                         Bookmarks.POSITION + ", " +
                         "NULL AS " + Combined.HISTORY_ID + ", " +
                         TopSites.TYPE_PINNED + " as " + TopSites.TYPE +
-                        " FROM " + TABLE_BOOKMARKS +
-                        " WHERE " + Bookmarks.PARENT + " == " + Bookmarks.FIXED_PINNED_LIST_ID +
+                        " " + pinnedSitesFromClause +
 
                         " ORDER BY " + Bookmarks.POSITION,
 
                         null);
 
             c.setNotificationUri(getContext().getContentResolver(),
                                  BrowserContract.AUTHORITY_URI);