Bug 1046709 - Post 2: (WIP) Frecency and Top Sites calculation updates draft
authorGrigory Kruglov <gkruglov@mozilla.com>
Sat, 16 Apr 2016 02:23:30 -0700
changeset 352265 80be0d8a91cf049aafccecc2ca7feb784f64baed
parent 352264 26411ec4888c73a7573762878934e55e45433709
child 518627 6bd47a3e22ad2d1fc175cd38a1b3cd0726197f62
push id15665
push usergkruglov@mozilla.com
push dateSat, 16 Apr 2016 09:32:06 +0000
bugs1046709
milestone48.0a1
Bug 1046709 - Post 2: (WIP) Frecency and Top Sites calculation updates Weights local visits much more heavily than remote visits. MozReview-Commit-ID: 6zX7l3L1TyB
mobile/android/base/java/org/mozilla/gecko/db/BrowserContract.java
mobile/android/base/java/org/mozilla/gecko/db/BrowserDatabaseHelper.java
mobile/android/base/java/org/mozilla/gecko/db/BrowserProvider.java
mobile/android/base/java/org/mozilla/gecko/db/LocalBrowserDB.java
--- a/mobile/android/base/java/org/mozilla/gecko/db/BrowserContract.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/BrowserContract.java
@@ -52,29 +52,46 @@ public class BrowserContract {
     public static final String PARAM_DATASET_ID = "dataset_id";
     public static final String PARAM_GROUP_BY = "group_by";
 
     static public enum ExpirePriority {
         NORMAL,
         AGGRESSIVE
     }
 
-    static public String getFrecencySortOrder(boolean includesBookmarks, boolean asc) {
-        final String age = "(" + Combined.DATE_LAST_VISITED + " - " + System.currentTimeMillis() + ") / 86400000";
-
-        StringBuilder order = new StringBuilder(Combined.VISITS + " * MAX(1, 100 * 225 / (" + age + "*" + age + " + 225)) ");
+    static public String getCombinedFrecencySortOrder(boolean includesBookmarks, boolean asc) {
+        StringBuilder order = new StringBuilder(getRemoteFrecencySQL() + " + " + getLocalFrecencySQL());
 
         if (includesBookmarks) {
             order.insert(0, "(CASE WHEN " + Combined.BOOKMARK_ID + " > -1 THEN 100 ELSE 0 END) + ");
         }
 
         order.append(asc ? " ASC" : " DESC");
         return order.toString();
     }
 
+    static public String getRemoteFrecencySQL() {
+        return getFrecencyCalculation(1, 110, Combined.REMOTE_VISITS_COUNT, Combined.REMOTE_DATE_LAST_VISITED);
+    }
+
+    static public String getLocalFrecencySQL() {
+        String visitCountExpr = "(" + Combined.LOCAL_VISITS_COUNT + " + 2)";
+        visitCountExpr = visitCountExpr + " * " + visitCountExpr;
+
+        return getFrecencyCalculation(2, 225, visitCountExpr, Combined.LOCAL_DATE_LAST_VISITED);
+    }
+
+    static public String getFrecencyCalculation(final int minFrecency, final int multiplier, final String visitCountExpr, final String lastVisitExpr) {
+        final long nowInMicroseconds = System.currentTimeMillis() * 1000;
+        final long microsecondsPerDay = 86400000000L;
+        final String ageExpr = "(" + nowInMicroseconds + " - " + lastVisitExpr + ") / " + microsecondsPerDay;
+
+        return visitCountExpr + " * MAX(" + minFrecency + ", 100 * " + multiplier + " / (" + ageExpr + "*" + ageExpr + " + " + multiplier + "))";
+    }
+
     @RobocopTarget
     public interface CommonColumns {
         public static final String _ID = "_id";
     }
 
     @RobocopTarget
     public interface DateSyncColumns {
         public static final String DATE_CREATED = "created";
@@ -237,16 +254,22 @@ public class BrowserContract {
         public static final String VIEW_NAME = "combined";
 
         public static final String VIEW_WITH_FAVICONS = "combined_with_favicons";
 
         public static final Uri CONTENT_URI = Uri.withAppendedPath(AUTHORITY_URI, "combined");
 
         public static final String BOOKMARK_ID = "bookmark_id";
         public static final String HISTORY_ID = "history_id";
+
+        public static final String REMOTE_VISITS_COUNT = "remoteVisitCount";
+        public static final String REMOTE_DATE_LAST_VISITED = "remoteDateLastVisited";
+
+        public static final String LOCAL_VISITS_COUNT = "localVisitCount";
+        public static final String LOCAL_DATE_LAST_VISITED = "localDateLastVisited";
     }
 
     public static final class Schema {
         private Schema() {}
         public static final Uri CONTENT_URI = Uri.withAppendedPath(AUTHORITY_URI, "schema");
 
         public static final String VERSION = "version";
     }
--- a/mobile/android/base/java/org/mozilla/gecko/db/BrowserDatabaseHelper.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/BrowserDatabaseHelper.java
@@ -54,17 +54,17 @@ import android.util.Log;
 
 
 // public for robocop testing
 public final class BrowserDatabaseHelper extends SQLiteOpenHelper {
     private static final String LOGTAG = "GeckoBrowserDBHelper";
 
     // Replace the Bug number below with your Bug that is conducting a DB upgrade, as to force a merge conflict with any
     // other patches that require a DB upgrade.
-    public static final int DATABASE_VERSION = 32; // Bug 1046709
+    public static final int DATABASE_VERSION = 33; // Bug 1046709 - post
     public static final String DATABASE_NAME = "browser.db";
 
     final protected Context mContext;
 
     static final String TABLE_BOOKMARKS = Bookmarks.TABLE_NAME;
     static final String TABLE_HISTORY = History.TABLE_NAME;
     static final String TABLE_VISITS = Visits.TABLE_NAME;
     static final String TABLE_FAVICONS = Favicons.TABLE_NAME;
@@ -381,16 +381,113 @@ public final class BrowserDatabaseHelper
                 " SELECT " + qualifyColumn(VIEW_COMBINED, "*") + ", " +
                     qualifyColumn(TABLE_FAVICONS, Favicons.URL) + " AS " + Combined.FAVICON_URL + ", " +
                     qualifyColumn(TABLE_FAVICONS, Favicons.DATA) + " AS " + Combined.FAVICON +
                 " FROM " + VIEW_COMBINED + " LEFT OUTER JOIN " + TABLE_FAVICONS +
                     " ON " + Combined.FAVICON_ID + " = " + qualifyColumn(TABLE_FAVICONS, Favicons._ID));
 
     }
 
+    private void createCombinedViewOn33(final SQLiteDatabase db) {
+        db.execSQL("CREATE VIEW IF NOT EXISTS " + VIEW_COMBINED + " AS" +
+
+                // Bookmarks without history.
+                " SELECT " + qualifyColumn(TABLE_BOOKMARKS, Bookmarks._ID) + " AS " + Combined.BOOKMARK_ID + "," +
+                "-1 AS " + Combined.HISTORY_ID + "," +
+                "0 AS " + Combined._ID + "," +
+                qualifyColumn(TABLE_BOOKMARKS, Bookmarks.URL) + " AS " + Combined.URL + ", " +
+                qualifyColumn(TABLE_BOOKMARKS, Bookmarks.TITLE) + " AS " + Combined.TITLE + ", " +
+                "-1 AS " + Combined.VISITS + ", " +
+                "-1 AS " + Combined.DATE_LAST_VISITED + "," +
+                qualifyColumn(TABLE_BOOKMARKS, Bookmarks.FAVICON_ID) + " AS " + Combined.FAVICON_ID + "," +
+                "0 AS " + Combined.LOCAL_DATE_LAST_VISITED + ", " +
+                "0 AS " + Combined.REMOTE_DATE_LAST_VISITED + ", " +
+                "0 AS " + Combined.LOCAL_VISITS_COUNT + ", " +
+                "0 AS " + Combined.REMOTE_VISITS_COUNT +
+                " FROM " + TABLE_BOOKMARKS +
+                " WHERE " +
+                qualifyColumn(TABLE_BOOKMARKS, Bookmarks.TYPE)  + " = " + Bookmarks.TYPE_BOOKMARK + " AND " +
+                // Ignore pinned bookmarks.
+                qualifyColumn(TABLE_BOOKMARKS, Bookmarks.PARENT)  + " <> " + Bookmarks.FIXED_PINNED_LIST_ID + " AND " +
+                qualifyColumn(TABLE_BOOKMARKS, Bookmarks.IS_DELETED)  + " = 0 AND " +
+                qualifyColumn(TABLE_BOOKMARKS, Bookmarks.URL) +
+                " NOT IN (SELECT " + History.URL + " FROM " + TABLE_HISTORY + ")" +
+                " UNION ALL" +
+
+                // History with and without bookmark.
+                " SELECT " +
+                "CASE " + qualifyColumn(TABLE_BOOKMARKS, Bookmarks.IS_DELETED) +
+
+                // Give pinned bookmarks a NULL ID so that they're not treated as bookmarks. We can't
+                // completely ignore them here because they're joined with history entries we care about.
+                " WHEN 0 THEN " +
+                "CASE " + qualifyColumn(TABLE_BOOKMARKS, Bookmarks.PARENT) +
+                " WHEN " + Bookmarks.FIXED_PINNED_LIST_ID + " THEN " +
+                "NULL " +
+                "ELSE " +
+                qualifyColumn(TABLE_BOOKMARKS, Bookmarks._ID) +
+                " END " +
+                "ELSE " +
+                "NULL " +
+                "END AS " + Combined.BOOKMARK_ID + "," +
+                qualifyColumn(TABLE_HISTORY, History._ID) + " AS " + Combined.HISTORY_ID + "," +
+                "0 AS " + Combined._ID + "," +
+                qualifyColumn(TABLE_HISTORY, History.URL) + " AS " + Combined.URL + "," +
+
+                // Prioritize bookmark titles over history titles, since the user may have
+                // customized the title for a bookmark.
+                "COALESCE(" + qualifyColumn(TABLE_BOOKMARKS, Bookmarks.TITLE) + ", " +
+                qualifyColumn(TABLE_HISTORY, History.TITLE) +
+                ") AS " + Combined.TITLE + "," +
+                qualifyColumn(TABLE_HISTORY, History.VISITS) + " AS " + Combined.VISITS + "," +
+                qualifyColumn(TABLE_HISTORY, History.DATE_LAST_VISITED) + " AS " + Combined.DATE_LAST_VISITED + "," +
+                qualifyColumn(TABLE_HISTORY, History.FAVICON_ID) + " AS " + Combined.FAVICON_ID + "," +
+
+                "COALESCE(MAX(CASE " + qualifyColumn(TABLE_VISITS, Visits.IS_LOCAL) + " " +
+                    "WHEN 1 THEN " + qualifyColumn(TABLE_VISITS, Visits.DATE_VISITED) + " " +
+                    "ELSE 0 END" +
+                "), 0) AS " + Combined.LOCAL_DATE_LAST_VISITED + ", " +
+
+                "COALESCE(MAX(CASE " + qualifyColumn(TABLE_VISITS, Visits.IS_LOCAL) + " " +
+                    "WHEN 0 THEN " + qualifyColumn(TABLE_VISITS, Visits.DATE_VISITED) + " " +
+                    "ELSE 0 END" +
+                "), 0) AS " + Combined.REMOTE_DATE_LAST_VISITED + ", " +
+
+                "COALESCE(SUM(" + qualifyColumn(TABLE_VISITS, Visits.IS_LOCAL) + "), 0) AS " + Combined.LOCAL_VISITS_COUNT + ", " +
+                "COALESCE(SUM(CASE " + qualifyColumn(TABLE_VISITS, Visits.IS_LOCAL) + " WHEN 0 THEN 1 ELSE 0 END), 0) AS " + Combined.REMOTE_VISITS_COUNT +
+
+                // We really shouldn't be selecting deleted bookmarks, but oh well.
+                " FROM " + TABLE_HISTORY + " " +
+                "INNER JOIN " + TABLE_VISITS +
+                " ON " + qualifyColumn(TABLE_HISTORY, History.GUID) + " = " + qualifyColumn(TABLE_VISITS, Visits.HISTORY_GUID) + " " +
+                "LEFT OUTER JOIN " + TABLE_BOOKMARKS +
+                " ON " + qualifyColumn(TABLE_BOOKMARKS, Bookmarks.URL) + " = " + qualifyColumn(TABLE_HISTORY, History.URL) +
+                " WHERE " +
+                qualifyColumn(TABLE_HISTORY, History.IS_DELETED) + " = 0 AND " +
+                "(" +
+                // The left outer join didn't match...
+                qualifyColumn(TABLE_BOOKMARKS, Bookmarks.TYPE) + " IS NULL OR " +
+
+                // ... or it's a bookmark. This is less efficient than filtering prior
+                // to the join if you have lots of folders.
+                qualifyColumn(TABLE_BOOKMARKS, Bookmarks.TYPE) + " = " + Bookmarks.TYPE_BOOKMARK +
+
+                ") GROUP BY " + qualifyColumn(TABLE_HISTORY, History.GUID)
+        );
+
+        debug("Creating " + VIEW_COMBINED_WITH_FAVICONS + " view");
+
+        db.execSQL("CREATE VIEW IF NOT EXISTS " + VIEW_COMBINED_WITH_FAVICONS + " AS" +
+                " SELECT " + qualifyColumn(VIEW_COMBINED, "*") + ", " +
+                qualifyColumn(TABLE_FAVICONS, Favicons.URL) + " AS " + Combined.FAVICON_URL + ", " +
+                qualifyColumn(TABLE_FAVICONS, Favicons.DATA) + " AS " + Combined.FAVICON +
+                " FROM " + VIEW_COMBINED + " LEFT OUTER JOIN " + TABLE_FAVICONS +
+                " ON " + Combined.FAVICON_ID + " = " + qualifyColumn(TABLE_FAVICONS, Favicons._ID));
+    }
+
     private void createLoginsTable(SQLiteDatabase db, final String tableName) {
         debug("Creating logins.db: " + db.getPath());
         debug("Creating " + tableName + " table");
 
         // Table for each login.
         db.execSQL("CREATE TABLE " + tableName + "(" +
                 BrowserContract.Logins._ID + " INTEGER PRIMARY KEY AUTOINCREMENT," +
                 BrowserContract.Logins.HOSTNAME + " TEXT NOT NULL," +
@@ -457,17 +554,16 @@ public final class BrowserDatabaseHelper
         createClientsTable(db);
         createLocalClient(db);
         createTabsTable(db, TABLE_TABS);
         createTabsTableIndices(db, TABLE_TABS);
 
 
         createBookmarksWithFaviconsView(db);
         createHistoryWithFaviconsView(db);
-        createCombinedViewOn19(db);
 
         createOrUpdateSpecialFolder(db, Bookmarks.PLACES_FOLDER_GUID,
             R.string.bookmarks_folder_places, 0);
 
         createOrUpdateAllSpecialFolders(db);
         createSearchHistoryTable(db);
         createUrlAnnotationsTable(db);
         createNumbersTable(db);
@@ -475,16 +571,17 @@ public final class BrowserDatabaseHelper
         createDeletedLoginsTable(db, TABLE_DELETED_LOGINS);
         createDisabledHostsTable(db, TABLE_DISABLED_HOSTS);
         createLoginsTable(db, TABLE_LOGINS);
         createLoginsTableIndices(db, TABLE_LOGINS);
 
         createBookmarksWithAnnotationsView(db);
 
         createVisitsTable(db);
+        createCombinedViewOn33(db);
     }
 
     /**
      * Copies the tabs and clients tables out of the given tabs.db file and into the destinationDB.
      *
      * @param tabsDBFile Path to existing tabs.db.
      * @param destinationDB The destination database.
      */
@@ -1582,16 +1679,27 @@ public final class BrowserDatabaseHelper
             }
         } catch (Exception e) {
             Log.e(LOGTAG, "Error while synthesizing visits for history record", e);
         } finally {
             cursor.close();
         }
     }
 
+    private void upgradeDatabaseFrom32to33(final SQLiteDatabase db) {
+        createV33CombinedView(db);
+    }
+
+    private void createV33CombinedView(final SQLiteDatabase db) {
+        db.execSQL("DROP VIEW IF EXISTS " + VIEW_COMBINED);
+        db.execSQL("DROP VIEW IF EXISTS " + VIEW_COMBINED_WITH_FAVICONS);
+
+        createCombinedViewOn33(db);
+    }
+
     private void createV19CombinedView(SQLiteDatabase db) {
         db.execSQL("DROP VIEW IF EXISTS " + VIEW_COMBINED);
         db.execSQL("DROP VIEW IF EXISTS " + VIEW_COMBINED_WITH_FAVICONS);
 
         createCombinedViewOn19(db);
     }
 
     @Override
@@ -1687,16 +1795,20 @@ public final class BrowserDatabaseHelper
 
                 case 31:
                     upgradeDatabaseFrom30to31(db);
                     break;
 
                 case 32:
                     upgradeDatabaseFrom31to32(db);
                     break;
+
+                case 33:
+                    upgradeDatabaseFrom32to33(db);
+                    break;
             }
         }
 
         for (Table table : BrowserProvider.sTables) {
             table.onUpgrade(db, oldVersion, newVersion);
         }
 
         // Delete the obsolete favicon database after all other upgrades complete.
--- a/mobile/android/base/java/org/mozilla/gecko/db/BrowserProvider.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/BrowserProvider.java
@@ -316,17 +316,17 @@ public class BrowserProvider extends Sha
         Log.d(LOGTAG, "Expiring history.");
         final long rows = DatabaseUtils.queryNumEntries(db, TABLE_HISTORY);
 
         if (retain >= rows) {
             debug("Not expiring history: only have " + rows + " rows.");
             return;
         }
 
-        final String sortOrder = BrowserContract.getFrecencySortOrder(false, true);
+        final String sortOrder = BrowserContract.getCombinedFrecencySortOrder(false, true);
         final long toRemove = rows - retain;
         debug("Expiring at most " + toRemove + " rows earlier than " + keepAfter + ".");
 
         final String sql;
         if (keepAfter > 0) {
             sql = "DELETE FROM " + TABLE_HISTORY + " " +
                   "WHERE MAX(" + History.DATE_LAST_VISITED + ", " + History.DATE_MODIFIED + ") < " + keepAfter + " " +
                   " AND " + History._ID + " IN ( SELECT " +
@@ -347,17 +347,17 @@ public class BrowserProvider extends Sha
     /**
      * Remove any thumbnails that for sites that aren't likely to be ever shown.
      * Items will be removed according to a frecency calculation and only if they are not pinned
      *
      * Call this method within a transaction.
      */
     private void expireThumbnails(final SQLiteDatabase db) {
         Log.d(LOGTAG, "Expiring thumbnails.");
-        final String sortOrder = BrowserContract.getFrecencySortOrder(true, false);
+        final String sortOrder = BrowserContract.getCombinedFrecencySortOrder(true, false);
         final String sql = "DELETE FROM " + TABLE_THUMBNAILS +
                            " WHERE " + Thumbnails.URL + " NOT IN ( " +
                              " SELECT " + Combined.URL +
                              " FROM " + Combined.VIEW_NAME +
                              " ORDER BY " + sortOrder +
                              " LIMIT " + DEFAULT_EXPIRY_THUMBNAIL_COUNT +
                            ") AND " + Thumbnails.URL + " NOT IN ( " +
                              " SELECT " + Bookmarks.URL +
@@ -893,17 +893,17 @@ public class BrowserProvider extends Sha
                        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.getFrecencySortOrder(true, false) +
+                       " ORDER BY " + BrowserContract.getCombinedFrecencySortOrder(true, false) +
                        " LIMIT " + totalLimit,
 
                        ignoreForTopSitesArgs);
 
             if (hasProcessedAnySuggestedSites) {
                 db.execSQL("INSERT INTO " + TABLE_TOPSITES +
                            // We need to LIMIT _after_ selecting the relevant suggested sites, which requires us to
                            // use an additional internal subquery, since we cannot LIMIT a subquery that is part of UNION ALL.
--- a/mobile/android/base/java/org/mozilla/gecko/db/LocalBrowserDB.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/LocalBrowserDB.java
@@ -571,17 +571,17 @@ public class LocalBrowserDB implements B
 
         // Our version of frecency is computed by scaling the number of visits by a multiplier
         // that approximates Gaussian decay, based on how long ago the entry was last visited.
         // Since we're limited by the math we can do with sqlite, we're calculating this
         // approximation using the Cauchy distribution: multiplier = 15^2 / (age^2 + 15^2).
         // Using 15 as our scale parameter, we get a constant 15^2 = 225. Following this math,
         // frecencyScore = numVisits * max(1, 100 * 225 / (age*age + 225)). (See bug 704977)
         // We also give bookmarks an extra bonus boost by adding 100 points to their frecency score.
-        final String sortOrder = BrowserContract.getFrecencySortOrder(true, false);
+        final String sortOrder = BrowserContract.getCombinedFrecencySortOrder(true, false);
 
         return cr.query(combinedUriWithLimit(limit),
                         projection,
                         selection,
                         selectionArgs,
                         sortOrder);
     }