Bug 760956 - Part 5: use BrowserProvider TopSites query instead of TopSitesCursorWrapper r=rnewman draft
authorAndrzej Hunt <ahunt@mozilla.com>
Thu, 25 Feb 2016 16:54:50 -0800
changeset 335013 363cdb8cea403217bdbed8d8e46f40e0ecb60b1c
parent 335012 e0b20eef191ea62bfabbc6c2f4d29c68fc647e10
child 335014 2c436475c74c2ad50f55c022fa33ad5de1011e33
push id11698
push userahunt@mozilla.com
push dateFri, 26 Feb 2016 18:23:22 +0000
reviewersrnewman
bugs760956
milestone47.0a1
Bug 760956 - Part 5: use BrowserProvider TopSites query instead of TopSitesCursorWrapper r=rnewman This means we now have only one open cursor for the topsites query, instead of 2 real cursors (and one MatrixCursor). We still need a MergeCursor and MatrixCursor to supply blank tiles, this will disappear as soon as the user has sufficient history items to fill the suggested sites grid. MozReview-Commit-ID: LnJaSMIDM2O
mobile/android/base/java/org/mozilla/gecko/db/BrowserDB.java
mobile/android/base/java/org/mozilla/gecko/db/LocalBrowserDB.java
mobile/android/base/java/org/mozilla/gecko/db/StubBrowserDB.java
--- a/mobile/android/base/java/org/mozilla/gecko/db/BrowserDB.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/BrowserDB.java
@@ -74,19 +74,20 @@ public interface BrowserDB {
      * @return a cursor over top sites (high-ranking bookmarks and history).
      * Can return <code>null</code>.
      */
     public abstract Cursor getTopSites(ContentResolver cr, int limit);
 
     /**
      * @return a cursor over top sites (high-ranking bookmarks and history).
      * Can return <code>null</code>.
-     * Returns no more than <code>maxLimit</code> results.
+     * Returns no more than <code>limit</code> results.
+     * Suggested sites will be limited to being within the first <code>suggestedRangeLimit</code> results.
      */
-    public abstract Cursor getTopSites(ContentResolver cr, int minLimit, int maxLimit);
+    public abstract Cursor getTopSites(ContentResolver cr, int suggestedRangeLimit, int limit);
 
     public abstract void updateVisitedHistory(ContentResolver cr, String uri);
 
     public abstract void updateHistoryTitle(ContentResolver cr, String uri, String title);
 
     /**
      * Can return <code>null</code>.
      */
--- a/mobile/android/base/java/org/mozilla/gecko/db/LocalBrowserDB.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/LocalBrowserDB.java
@@ -27,32 +27,35 @@ import org.mozilla.gecko.annotation.Robo
 import org.mozilla.gecko.R;
 import org.mozilla.gecko.db.BrowserContract.Bookmarks;
 import org.mozilla.gecko.db.BrowserContract.Combined;
 import org.mozilla.gecko.db.BrowserContract.ExpirePriority;
 import org.mozilla.gecko.db.BrowserContract.Favicons;
 import org.mozilla.gecko.db.BrowserContract.History;
 import org.mozilla.gecko.db.BrowserContract.SyncColumns;
 import org.mozilla.gecko.db.BrowserContract.Thumbnails;
+import org.mozilla.gecko.db.BrowserContract.TopSites;
 import org.mozilla.gecko.distribution.Distribution;
 import org.mozilla.gecko.favicons.decoders.FaviconDecoder;
 import org.mozilla.gecko.favicons.decoders.LoadFaviconResult;
 import org.mozilla.gecko.gfx.BitmapUtils;
 import org.mozilla.gecko.Restrictions;
 import org.mozilla.gecko.sync.Utils;
 import org.mozilla.gecko.util.GeckoJarReader;
 import org.mozilla.gecko.util.StringUtils;
 
 import android.content.ContentProviderOperation;
 import android.content.ContentResolver;
 import android.content.ContentValues;
 import android.content.Context;
 import android.database.ContentObserver;
 import android.database.Cursor;
 import android.database.CursorWrapper;
+import android.database.MatrixCursor;
+import android.database.MergeCursor;
 import android.graphics.Bitmap;
 import android.graphics.Color;
 import android.graphics.drawable.BitmapDrawable;
 import android.net.Uri;
 import android.text.TextUtils;
 import android.util.Log;
 import org.mozilla.gecko.util.IOUtils;
 
@@ -90,16 +93,17 @@ public class LocalBrowserDB implements B
     private final Uri mBookmarksUriWithProfile;
     private final Uri mParentsUriWithProfile;
     private final Uri mHistoryUriWithProfile;
     private final Uri mHistoryExpireUriWithProfile;
     private final Uri mCombinedUriWithProfile;
     private final Uri mUpdateHistoryUriWithProfile;
     private final Uri mFaviconsUriWithProfile;
     private final Uri mThumbnailsUriWithProfile;
+    private final Uri mTopSitesUriWithProfile;
     private final Uri mSearchHistoryUri;
 
     private LocalSearches searches;
     private LocalTabsAccessor tabsAccessor;
     private LocalURLMetadata urlMetadata;
     private LocalReadingListAccessor readingListAccessor;
 
     private static final String[] DEFAULT_BOOKMARK_COLUMNS =
@@ -115,16 +119,17 @@ public class LocalBrowserDB implements B
         mFolderIdMap = new HashMap<String, Long>();
 
         mBookmarksUriWithProfile = DBUtils.appendProfile(profile, Bookmarks.CONTENT_URI);
         mParentsUriWithProfile = DBUtils.appendProfile(profile, Bookmarks.PARENTS_CONTENT_URI);
         mHistoryUriWithProfile = DBUtils.appendProfile(profile, History.CONTENT_URI);
         mHistoryExpireUriWithProfile = DBUtils.appendProfile(profile, History.CONTENT_OLD_URI);
         mCombinedUriWithProfile = DBUtils.appendProfile(profile, Combined.CONTENT_URI);
         mFaviconsUriWithProfile = DBUtils.appendProfile(profile, Favicons.CONTENT_URI);
+        mTopSitesUriWithProfile = DBUtils.appendProfile(profile, TopSites.CONTENT_URI);
         mThumbnailsUriWithProfile = DBUtils.appendProfile(profile, Thumbnails.CONTENT_URI);
 
         mSearchHistoryUri = BrowserContract.SearchHistory.CONTENT_URI;
 
         mUpdateHistoryUriWithProfile =
                 mHistoryUriWithProfile.buildUpon()
                                       .appendQueryParameter(BrowserContract.PARAM_INCREMENT_VISITS, "true")
                                       .appendQueryParameter(BrowserContract.PARAM_INSERT_IF_NEEDED, "true")
@@ -1674,32 +1679,54 @@ public class LocalBrowserDB implements B
                 url = StringUtils.decodeUserEnteredUrl(url);
             }
 
             urls.add(url);
         } while (c.moveToNext());
     }
 
     @Override
-    public Cursor getTopSites(ContentResolver cr, int minLimit, int maxLimit) {
-        // 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 pinnedSites = getPinnedSites(cr, minLimit);
-
-        int pinnedCount = pinnedSites.getCount();
-        Cursor topSites = getTopSites(cr, maxLimit - pinnedCount);
-        int topCount = topSites.getCount();
+    public Cursor getTopSites(ContentResolver cr, int suggestedRangeLimit, int limit) {
+        final Uri uri = mTopSitesUriWithProfile.buildUpon()
+                .appendQueryParameter(BrowserContract.PARAM_LIMIT,
+                        String.valueOf(limit))
+                .appendQueryParameter(BrowserContract.PARAM_SUGGESTEDSITES_LIMIT,
+                        String.valueOf(suggestedRangeLimit))
+                .build();
 
-        Cursor suggestedSites = null;
-        if (mSuggestedSites != null) {
-            final int count = minLimit - pinnedCount - topCount;
-            if (count > 0) {
-                final List<String> excludeUrls = new ArrayList<String>(pinnedCount + topCount);
-                appendUrlsFromCursor(excludeUrls, pinnedSites);
-                appendUrlsFromCursor(excludeUrls, topSites);
+        Cursor topSitesCursor = cr.query(uri,
+                                         new String[] { Combined._ID,
+                                                 Combined.URL,
+                                                 Combined.TITLE,
+                                                 Combined.BOOKMARK_ID,
+                                                 Combined.HISTORY_ID },
+                                         null,
+                                         null,
+                                         null);
 
-                suggestedSites = mSuggestedSites.get(count, excludeUrls);
-            }
+        // It's possible that we will retrieve fewer sites than are required to fill the top-sites panel - in this case
+        // we need to add "blank" tiles. It's much easier to add these here (as opposed to SQL), since we don't care
+        // about their ordering (they go after all the other sites), but we do care about their number (and calculating
+        // that inside out topsites SQL query would be difficult given the other processing we're already doing there).
+        final int blanksRequired = suggestedRangeLimit - topSitesCursor.getCount();
+
+        if (blanksRequired < 0) {
+            return topSitesCursor;
         }
 
-        return new TopSitesCursorWrapper(pinnedSites, topSites, suggestedSites, minLimit);
+        MatrixCursor blanksCursor = new MatrixCursor(new String[] {
+                Bookmarks._ID,
+                Bookmarks.URL,
+                Bookmarks.TITLE,
+                Bookmarks.TYPE});
+
+        for (int i = 0; i < blanksRequired; i++) {
+            final MatrixCursor.RowBuilder rb = blanksCursor.newRow();
+            rb.add(-1);
+            rb.add("");
+            rb.add("");
+            rb.add(TopSites.TYPE_BLANK);
+        }
+
+        return new MergeCursor(new Cursor[] {topSitesCursor, blanksCursor});
+
     }
 }
--- a/mobile/android/base/java/org/mozilla/gecko/db/StubBrowserDB.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/StubBrowserDB.java
@@ -378,17 +378,17 @@ public class StubBrowserDB implements Br
     public String getSuggestedImageUrlForUrl(String url) {
         return null;
     }
 
     public int getSuggestedBackgroundColorForUrl(String url) {
         return 0;
     }
 
-    public Cursor getTopSites(ContentResolver cr, int minLimit, int maxLimit) {
+    public Cursor getTopSites(ContentResolver cr, int suggestedRangeLimit, int limit) {
         return null;
     }
 
     public static Factory getFactory() {
         return new Factory() {
             @Override
             public BrowserDB get(String profileName, File profileDir) {
                 return new StubBrowserDB(profileName);