Bug 1254797 - Post: Ensure we ignore deleted sites in pinned query r=rnewman
authorAndrzej Hunt <andrzej@ahunt.org>
Tue, 15 Mar 2016 14:44:13 -0700
changeset 289584 da1a66f1c4b74c4bd778e91b9589d73763dddfe3
parent 289583 54080662c8491231102e0e81144b8fa6ddcee3d7
child 289585 90b4e3e5d0f4d8c58678807286c4e7dd6593a8a9
push id18284
push userahunt@mozilla.com
push dateMon, 21 Mar 2016 15:57:34 +0000
treeherderfx-team@da1a66f1c4b7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrnewman
bugs1254797
milestone48.0a1
Bug 1254797 - Post: Ensure we ignore deleted sites in pinned query r=rnewman 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.
@@ -920,18 +922,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);