Bug 1319274 - Part 1: Use regular getTopSites method for A-S top sites r=ahunt draft
authorGrisha Kruglov <gkruglov@mozilla.com>
Thu, 01 Dec 2016 18:53:47 -0800
changeset 449765 763633fca5f9f606a8f6cfd1f4a4332273c34dee
parent 449666 18b5a7a5d833f09b1f1e5cc45f4c0ff6235a1d5e
child 449766 e4a2f46f91c85bb4a7071361ba9da7d69f4d93da
push id38649
push userbmo:gkruglov@mozilla.com
push dateThu, 15 Dec 2016 00:24:34 +0000
reviewersahunt
bugs1319274
milestone53.0a1
Bug 1319274 - Part 1: Use regular getTopSites method for A-S top sites r=ahunt Since we want to include pinned sites in A-S Top Sites, this removes the "plain top sites" query which excludes pinned sites. Maximum number of suggested sites displayed is set so that they will fill out at most two "pages". MozReview-Commit-ID: 8uynmwiaPkt
mobile/android/base/java/org/mozilla/gecko/db/BrowserDB.java
mobile/android/base/java/org/mozilla/gecko/db/BrowserProvider.java
mobile/android/base/java/org/mozilla/gecko/db/LocalBrowserDB.java
mobile/android/base/java/org/mozilla/gecko/home/activitystream/ActivityStream.java
mobile/android/base/java/org/mozilla/gecko/home/activitystream/topsites/TopSitesPagerAdapter.java
--- a/mobile/android/base/java/org/mozilla/gecko/db/BrowserDB.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/BrowserDB.java
@@ -68,17 +68,17 @@ public abstract class BrowserDB {
     /**
      * @return a cursor over top sites (high-ranking bookmarks and history).
      * Can return <code>null</code>.
      * 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 suggestedRangeLimit, int limit);
 
-    public abstract CursorLoader getActivityStreamTopSites(Context context, int limit);
+    public abstract CursorLoader getActivityStreamTopSites(Context context, 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/BrowserProvider.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/BrowserProvider.java
@@ -867,80 +867,16 @@ public class BrowserProvider extends Sha
                 }
             }
         }
 
         debug("Updated " + updated + " rows for URI: " + uri);
         return updated;
     }
 
-    /**
-     * Get topsites by themselves, without the inclusion of pinned sites. Suggested sites
-     * will be appended (if necessary) to the end of the list in order to provide up to PARAM_LIMIT items.
-     */
-    private Cursor getPlainTopSites(final Uri uri) {
-        final SQLiteDatabase db = getReadableDatabase(uri);
-
-        final String limitParam = uri.getQueryParameter(BrowserContract.PARAM_LIMIT);
-        final int limit;
-        if (limitParam != null) {
-            limit = Integer.parseInt(limitParam);
-        } else {
-            limit = 12;
-        }
-
-        // Filter out: unvisited pages (history_id == -1) pinned (and other special) sites, deleted sites,
-        // and about: pages.
-        final String ignoreForTopSitesWhereClause =
-                "(" + Combined.HISTORY_ID + " IS NOT -1)" +
-                " AND " +
-                Combined.URL + " NOT IN (SELECT " +
-                Bookmarks.URL + " FROM " + TABLE_BOOKMARKS + " WHERE " +
-                DBUtils.qualifyColumn(TABLE_BOOKMARKS, Bookmarks.PARENT) + " < " + Bookmarks.FIXED_ROOT_ID + " AND " +
-                DBUtils.qualifyColumn(TABLE_BOOKMARKS, Bookmarks.IS_DELETED) + " == 0)" +
-                " AND " +
-                "(" + Combined.URL + " NOT LIKE ?)";
-
-        final String[] ignoreForTopSitesArgs = new String[] {
-                AboutPages.URL_FILTER
-        };
-
-        final Cursor c = db.rawQuery("SELECT " +
-                   Bookmarks._ID + ", " +
-                   Combined.BOOKMARK_ID + ", " +
-                   Combined.HISTORY_ID + ", " +
-                   Bookmarks.URL + ", " +
-                   Bookmarks.TITLE + ", " +
-                   Combined.HISTORY_ID + ", " +
-                   TopSites.TYPE_TOP + " AS " + TopSites.TYPE +
-                   " FROM " + Combined.VIEW_NAME +
-                   " WHERE " + ignoreForTopSitesWhereClause +
-                   " ORDER BY " + BrowserContract.getCombinedFrecencySortOrder(true, false) +
-                   " LIMIT " + limit,
-                ignoreForTopSitesArgs);
-
-        c.setNotificationUri(getContext().getContentResolver(),
-                BrowserContract.AUTHORITY_URI);
-
-        if (c.getCount() == limit) {
-            return c;
-        }
-
-        // If we don't have enough data: get suggested sites too
-        final SuggestedSites suggestedSites = BrowserDB.from(GeckoProfile.get(
-                getContext(), uri.getQueryParameter(BrowserContract.PARAM_PROFILE))).getSuggestedSites();
-
-        final Cursor suggestedSitesCursor = suggestedSites.get(limit - c.getCount());
-
-        return new MergeCursor(new Cursor[]{
-                c,
-                suggestedSitesCursor
-        });
-    }
-
     private Cursor getTopSites(final Uri uri) {
         // In order to correctly merge the top and pinned sites we:
         //
         // 1. Generate a list of free ids for topsites - this is the positions that are NOT used by pinned sites.
         //    We do this using a subquery with a self-join in order to generate rowids, that allow us to join with
         //    the list of topsites.
         // 2. Generate the list of topsites in order of frecency.
         // 3. Join these, so that each topsite is given its resulting position
@@ -1285,21 +1221,17 @@ public class BrowserProvider extends Sha
             String[] selectionArgs, String sortOrder) {
         final int match = URI_MATCHER.match(uri);
 
         // Handle only queries requiring a writable DB connection here: most queries need only a readable
         // connection, hence we can get a readable DB once, and then handle most queries within a switch.
         // TopSites requires a writable connection (because of the temporary tables it uses), hence
         // we handle that separately, i.e. before retrieving a readable connection.
         if (match == TOPSITES) {
-            if (uri.getBooleanQueryParameter(BrowserContract.PARAM_TOPSITES_DISABLE_PINNED, false)) {
-                return getPlainTopSites(uri);
-            } else {
-                return getTopSites(uri);
-            }
+            return getTopSites(uri);
         }
 
         SQLiteDatabase db = getReadableDatabase(uri);
 
         SQLiteQueryBuilder qb = new SQLiteQueryBuilder();
         String limit = uri.getQueryParameter(BrowserContract.PARAM_LIMIT);
         String groupBy = null;
 
--- a/mobile/android/base/java/org/mozilla/gecko/db/LocalBrowserDB.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/LocalBrowserDB.java
@@ -1846,21 +1846,22 @@ public class LocalBrowserDB extends Brow
             final long end = SystemClock.uptimeMillis();
             final long took = end - start;
 
             Telemetry.addToHistogram(mHistogramName, (int) Math.min(took, Integer.MAX_VALUE));
             return cursor;
         }
     }
 
-    public CursorLoader getActivityStreamTopSites(Context context, int limit) {
+    public CursorLoader getActivityStreamTopSites(Context context, int suggestedRangeLimit, int limit) {
         final Uri uri = mTopSitesUriWithProfile.buildUpon()
                 .appendQueryParameter(BrowserContract.PARAM_LIMIT,
                         String.valueOf(limit))
-                .appendQueryParameter(BrowserContract.PARAM_TOPSITES_DISABLE_PINNED, Boolean.TRUE.toString())
+                .appendQueryParameter(BrowserContract.PARAM_SUGGESTEDSITES_LIMIT,
+                        String.valueOf(suggestedRangeLimit))
                 .build();
 
         return new TelemetrisedCursorLoader(context,
                 uri,
                 new String[]{ Combined._ID,
                         Combined.URL,
                         Combined.TITLE,
                         Combined.BOOKMARK_ID,
--- a/mobile/android/base/java/org/mozilla/gecko/home/activitystream/ActivityStream.java
+++ b/mobile/android/base/java/org/mozilla/gecko/home/activitystream/ActivityStream.java
@@ -113,17 +113,19 @@ public class ActivityStream extends Fram
     private class CursorLoaderCallbacks implements LoaderManager.LoaderCallbacks<Cursor> {
         @Override
         public Loader<Cursor> onCreateLoader(int id, Bundle args) {
             final Context context = getContext();
             if (id == LOADER_ID_HIGHLIGHTS) {
                 return BrowserDB.from(context).getHighlights(context, 10);
             } else if (id == LOADER_ID_TOPSITES) {
                 return BrowserDB.from(context).getActivityStreamTopSites(
-                        context, TopSitesPagerAdapter.PAGES * MAXIMUM_TILES);
+                        context,
+                        MAXIMUM_TILES * TopSitesPagerAdapter.SUGGESTED_SITES_MAX_PAGES,
+                        MAXIMUM_TILES * TopSitesPagerAdapter.PAGES);
             } else {
                 throw new IllegalArgumentException("Can't handle loader id " + id);
             }
         }
 
         @Override
         public void onLoadFinished(Loader<Cursor> loader, Cursor data) {
             if (loader.getId() == LOADER_ID_HIGHLIGHTS) {
--- a/mobile/android/base/java/org/mozilla/gecko/home/activitystream/topsites/TopSitesPagerAdapter.java
+++ b/mobile/android/base/java/org/mozilla/gecko/home/activitystream/topsites/TopSitesPagerAdapter.java
@@ -17,16 +17,17 @@ import org.mozilla.gecko.home.HomePager;
 import java.util.LinkedList;
 
 /**
  * The primary / top-level TopSites adapter: it handles the ViewPager, and also handles
  * all lower-level Adapters that populate the individual topsite items.
  */
 public class TopSitesPagerAdapter extends PagerAdapter {
     public static final int PAGES = 4;
+    public static final int SUGGESTED_SITES_MAX_PAGES = 2;
 
     private int tiles;
     private int tilesWidth;
     private int tilesHeight;
 
     private LinkedList<TopSitesPage> pages = new LinkedList<>();
 
     private final Context context;