Bug 885084 - Only return top bookmarks for bookmarks page thumbnails. r=wesj
authorMargaret Leibovic <margaret.leibovic@gmail.com>
Wed, 21 Aug 2013 11:33:58 -0700
changeset 143673 936c053afda0972c1fd7441f04ce5de2dee92cf9
parent 143672 71dbfbd1449bb6a9b3d05ef9fe1328374b16a707
child 143674 26b37a40d65f5415094032f5bc28df8c5f37beeb
push id2303
push usermleibovic@mozilla.com
push dateWed, 21 Aug 2013 18:45:31 +0000
treeherderfx-team@5393a1a4e3ef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerswesj
bugs885084
milestone26.0a1
Bug 885084 - Only return top bookmarks for bookmarks page thumbnails. r=wesj
mobile/android/base/db/BrowserDB.java
mobile/android/base/db/LocalBrowserDB.java
mobile/android/base/home/BookmarksPage.java
--- a/mobile/android/base/db/BrowserDB.java
+++ b/mobile/android/base/db/BrowserDB.java
@@ -33,19 +33,19 @@ public class BrowserDB {
 
     private static BrowserDBIface sDb = null;
 
     public interface BrowserDBIface {
         public void invalidateCachedState();
 
         public Cursor filter(ContentResolver cr, CharSequence constraint, int limit);
 
-        // This should onlyl return frecent sites, BrowserDB.getTopSites will do the
+        // This should only return frecent bookmarks, BrowserDB.getTopBookmarks will do the
         // work to combine that list with the pinned sites list
-        public Cursor getTopSites(ContentResolver cr, int limit);
+        public Cursor getTopBookmarks(ContentResolver cr, int limit);
 
         public void updateVisitedHistory(ContentResolver cr, String uri);
 
         public void updateHistoryTitle(ContentResolver cr, String uri, String title);
 
         public void updateHistoryEntry(ContentResolver cr, String uri, String title,
                                        long date, int visits);
 
@@ -132,22 +132,22 @@ public class BrowserDB {
     public static void invalidateCachedState() {
         sDb.invalidateCachedState();
     }
 
     public static Cursor filter(ContentResolver cr, CharSequence constraint, int limit) {
         return sDb.filter(cr, constraint, limit);
     }
 
-    public static Cursor getTopSites(ContentResolver cr, int limit) {
-        // Note this is not a single query anymore, but actually returns a mixture of two queries, one for topSites
-        // and one for pinned sites
-        Cursor topSites = sDb.getTopSites(cr, limit);
+    public static Cursor getTopBookmarks(ContentResolver cr, int limit) {
+        // Note this is not a single query anymore, but actually returns a mixture of two queries,
+        // one for top bookmarks, and one for pinned sites (which are actually bookmarks as well).
+        Cursor topBookmarks = sDb.getTopBookmarks(cr, limit);
         Cursor pinnedSites = sDb.getPinnedSites(cr, limit);
-        return new TopSitesCursorWrapper(pinnedSites, topSites, limit);
+        return new TopSitesCursorWrapper(pinnedSites, topBookmarks, limit);
     }
 
     public static void updateVisitedHistory(ContentResolver cr, String uri) {
         sDb.updateVisitedHistory(cr, uri);
     }
 
     public static void updateHistoryTitle(ContentResolver cr, String uri, String title) {
         sDb.updateHistoryTitle(cr, uri, title);
--- a/mobile/android/base/db/LocalBrowserDB.java
+++ b/mobile/android/base/db/LocalBrowserDB.java
@@ -226,19 +226,23 @@ public class LocalBrowserDB implements B
                                              Combined.BOOKMARK_ID,
                                              Combined.HISTORY_ID },
                               constraint,
                               limit,
                               null);
     }
 
     @Override
-    public Cursor getTopSites(ContentResolver cr, int limit) {
-        // Filter out sites that are pinned
-        String selection = DBUtils.concatenateWhere("", Combined.URL + " NOT IN (SELECT " +
+    public Cursor getTopBookmarks(ContentResolver cr, int limit) {
+        // Only select bookmarks. Unfortunately, we need to query the combined view,
+        // instead of just the bookmarks table, in order to do the frecency calculation.
+        String selection = Combined.BOOKMARK_ID + " IS NOT NULL";
+
+        // Filter out sites that are pinned.
+        selection = DBUtils.concatenateWhere(selection, Combined.URL + " NOT IN (SELECT " +
                                              Bookmarks.URL + " FROM bookmarks WHERE " +
                                              DBUtils.qualifyColumn("bookmarks", Bookmarks.PARENT) + " == ? AND " +
                                              DBUtils.qualifyColumn("bookmarks", Bookmarks.IS_DELETED) + " == 0)");
         String[] selectionArgs = DBUtils.appendSelectionArgs(new String[0], new String[] { String.valueOf(Bookmarks.FIXED_PINNED_LIST_ID) });
         return filterAllSites(cr,
                               new String[] { Combined._ID,
                                              Combined.URL,
                                              Combined.TITLE },
--- a/mobile/android/base/home/BookmarksPage.java
+++ b/mobile/android/base/home/BookmarksPage.java
@@ -357,17 +357,17 @@ public class BookmarksPage extends HomeF
     private static class TopBookmarksLoader extends SimpleCursorLoader {
         public TopBookmarksLoader(Context context) {
             super(context);
         }
 
         @Override
         public Cursor loadCursor() {
             final int max = getContext().getResources().getInteger(R.integer.number_of_top_sites);
-            return BrowserDB.getTopSites(getContext().getContentResolver(), max);
+            return BrowserDB.getTopBookmarks(getContext().getContentResolver(), max);
         }
     }
 
     /**
      * Loader callbacks for the LoaderManager of this fragment.
      */
     private class CursorLoaderCallbacks extends HomeCursorLoaderCallbacks {
         public CursorLoaderCallbacks(Context context, LoaderManager loaderManager) {