Bug 1293790 - Implement getPlainTopSites query r?grisha draft
authorAndrzej Hunt <ahunt@mozilla.com>
Tue, 23 Aug 2016 14:02:06 -0700
changeset 404592 6c291ec12a340befa9c128b76984e9186a3e8a68
parent 404159 08e9ded26ada149385af32e1c1d89f30e3d8279c
child 404593 e57802d851654abe72e209ad7be23e29e1630d85
push id27256
push userahunt@mozilla.com
push dateTue, 23 Aug 2016 21:38:12 +0000
reviewersgrisha
bugs1293790
milestone51.0a1
Bug 1293790 - Implement getPlainTopSites query r?grisha ActivityStream's topsites won't support pinned sites for now, this allows us to use a simpler query that only retrieves topsites without the complexity of a temporary table to merge pinned sites. This results in some duplication between the old and new topsites queries, however eventually we're going to want to get rid of one of these queries (we don't know whether ActivityStream will support pinned sites in the future yet, so we definitely want to keep the pinned query for now - it's also needed for the old topsites panel). MozReview-Commit-ID: AQyzXHGl3Cf
mobile/android/base/java/org/mozilla/gecko/db/BrowserContract.java
mobile/android/base/java/org/mozilla/gecko/db/BrowserProvider.java
--- a/mobile/android/base/java/org/mozilla/gecko/db/BrowserContract.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/BrowserContract.java
@@ -40,16 +40,17 @@ public class BrowserContract {
 
     public static final String LOGINS_AUTHORITY = AppConstants.ANDROID_PACKAGE_NAME + ".db.logins";
     public static final Uri LOGINS_AUTHORITY_URI = Uri.parse("content://" + LOGINS_AUTHORITY);
 
     public static final String PARAM_PROFILE = "profile";
     public static final String PARAM_PROFILE_PATH = "profilePath";
     public static final String PARAM_LIMIT = "limit";
     public static final String PARAM_SUGGESTEDSITES_LIMIT = "suggestedsites_limit";
+    public static final String PARAM_TOPSITES_DISABLE_PINNED = "topsites_disable_pinned";
     public static final String PARAM_IS_SYNC = "sync";
     public static final String PARAM_SHOW_DELETED = "show_deleted";
     public static final String PARAM_IS_TEST = "test";
     public static final String PARAM_INSERT_IF_NEEDED = "insert_if_needed";
     public static final String PARAM_INCREMENT_VISITS = "increment_visits";
     public static final String PARAM_INCREMENT_REMOTE_AGGREGATES = "increment_remote_aggregates";
     public static final String PARAM_EXPIRE_PRIORITY = "priority";
     public static final String PARAM_DATASET_ID = "dataset_id";
--- a/mobile/android/base/java/org/mozilla/gecko/db/BrowserProvider.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/BrowserProvider.java
@@ -38,16 +38,17 @@ import android.content.ContentValues;
 import android.content.Context;
 import android.content.Intent;
 import android.content.IntentFilter;
 import android.content.OperationApplicationException;
 import android.content.UriMatcher;
 import android.database.Cursor;
 import android.database.DatabaseUtils;
 import android.database.MatrixCursor;
+import android.database.MergeCursor;
 import android.database.SQLException;
 import android.database.sqlite.SQLiteCursor;
 import android.database.sqlite.SQLiteDatabase;
 import android.database.sqlite.SQLiteQueryBuilder;
 import android.net.Uri;
 import android.support.v4.content.LocalBroadcastManager;
 import android.text.TextUtils;
 import android.util.Log;
@@ -789,16 +790,76 @@ 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);
+
+        if (c.getCount() == limit) {
+            return c;
+        }
+
+        // If we don't have enough data: get suggested sites too
+        final SuggestedSites suggestedSites = GeckoProfile.get(getContext(), uri.getQueryParameter(BrowserContract.PARAM_PROFILE)).getDB().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
@@ -941,20 +1002,20 @@ public class BrowserProvider extends Sha
         final String suggestedLimitClause = " LIMIT MAX(0, (" + suggestedGridLimit + " - (SELECT COUNT(*) FROM " + TABLE_TOPSITES + ") - (SELECT COUNT(*) " + pinnedSitesFromClause + "))) ";
 
         // Pinned site positions are zero indexed, but we need to get the maximum 1-indexed position.
         // Hence to correctly calculate the largest pinned position (which should be 0 if there are
         // no sites, or 1-6 if we have at least one pinned site), we coalesce the DB position (0-5)
         // with -1 to represent no-sites, which allows us to directly add 1 to obtain the expected value
         // regardless of whether a position was actually retrieved.
         final String blanksLimitClause = " LIMIT MAX(0, " +
-                                         "COALESCE((SELECT " + Bookmarks.POSITION + " " + pinnedSitesFromClause + "), -1) + 1" +
-                                         " - (SELECT COUNT(*) " + pinnedSitesFromClause + ")" +
-                                         " - (SELECT COUNT(*) FROM " + TABLE_TOPSITES + ")" +
-                                         ")";
+                            "COALESCE((SELECT " + Bookmarks.POSITION + " " + pinnedSitesFromClause + "), -1) + 1" +
+                            " - (SELECT COUNT(*) " + pinnedSitesFromClause + ")" +
+                            " - (SELECT COUNT(*) FROM " + TABLE_TOPSITES + ")" +
+                            ")";
 
         db.beginTransaction();
         try {
             db.execSQL("DROP TABLE IF EXISTS " + TABLE_TOPSITES);
 
             db.execSQL("CREATE TEMP TABLE " + TABLE_TOPSITES + " AS" +
                        " SELECT " +
                        Bookmarks._ID + ", " +
@@ -1072,17 +1133,21 @@ public class BrowserProvider extends Sha
     }
 
     @Override
     public Cursor query(Uri uri, String[] projection, String selection,
             String[] selectionArgs, String sortOrder) {
         final int match = URI_MATCHER.match(uri);
 
         if (match == TOPSITES) {
-            return getTopSites(uri);
+            if (uri.getBooleanQueryParameter(BrowserContract.PARAM_TOPSITES_DISABLE_PINNED, false)) {
+                return getPlainTopSites(uri);
+            } else {
+                return getTopSites(uri);
+            }
         }
 
         SQLiteDatabase db = getReadableDatabase(uri);
 
         SQLiteQueryBuilder qb = new SQLiteQueryBuilder();
         String limit = uri.getQueryParameter(BrowserContract.PARAM_LIMIT);
         String groupBy = null;