Bug 1403755: Rm code to insert blanks into top sites. r=liuche draft
authorMichael Comella <michael.l.comella@gmail.com>
Thu, 28 Sep 2017 14:09:48 -0700
changeset 672179 24a1c13c077ff73b0699219074d5b7c7e97041ef
parent 671268 756e10aa8bbd416cbc49b7739f78fb81d5525477
child 733740 c90b632d038b2e2349b539472cb945f3caea61dd
push id82179
push usermichael.l.comella@gmail.com
push dateThu, 28 Sep 2017 21:11:19 +0000
reviewersliuche
bugs1403755
milestone58.0a1
Bug 1403755: Rm code to insert blanks into top sites. r=liuche This code was being mistakenly activated when getting top sites for Activity Stream. This is the first removal of old top sites code and will mean we can't go back to old top sites by flipping the `ActivityStream.isEnabled` flag. Since we're planning to ship AS, this shouldn't matter. If we wanted to preserve support, we could create a branch but deleting the code is much simpler. MozReview-Commit-ID: 9VB0RqNHmE0
mobile/android/base/java/org/mozilla/gecko/db/BrowserProvider.java
--- a/mobile/android/base/java/org/mozilla/gecko/db/BrowserProvider.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/BrowserProvider.java
@@ -1065,56 +1065,21 @@ public class BrowserProvider extends Sha
                                                             new String[] {
                                                                     suggestedSitesCursor.getString(idColumnIndex),
                                                                     suggestedSitesCursor.getString(urlColumnIndex),
                                                                     suggestedSitesCursor.getString(titleColumnIndex)
                                                             });
         }
         suggestedSitesCursor.close();
 
-        boolean hasPreparedBlankTiles = false;
-
-        // We can somewhat reduce the number of blanks we produce by eliminating suggested sites.
-        // We do the actual limit calculation in SQL (since we need to take into account the number
-        // of pinned sites too), but this might avoid producing 5 or so additional blank tiles
-        // that would then need to be filtered out.
-        final int maxBlanksNeeded = suggestedGridLimit - suggestedSitesCursor.getCount();
-
-        final StringBuilder blanksBuilder = new StringBuilder();
-        for (int i = 0; i < maxBlanksNeeded; i++) {
-            if (hasPreparedBlankTiles) {
-                blanksBuilder.append(" UNION ALL");
-            } else {
-                hasPreparedBlankTiles = true;
-            }
-
-            blanksBuilder.append(" SELECT" +
-                                 " -1 AS " + Bookmarks._ID + "," +
-                                 " '' AS " + Bookmarks.URL + "," +
-                                 " '' AS " + Bookmarks.TITLE);
-        }
-
-
-
         // To restrict suggested sites to the grid, we simply subtract the number of topsites (which have already had
         // the pinned sites filtered out), and the number of pinned sites.
         // SQLite completely ignores negative limits, hence we need to manually limit to 0 in this case.
         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 + ")" +
-                            ")";
-
         db.beginTransaction();
         try {
             db.execSQL("DROP TABLE IF EXISTS " + TABLE_TOPSITES);
 
             db.execSQL("CREATE TEMP TABLE " + TABLE_TOPSITES + " AS" +
                        " SELECT " +
                        Bookmarks._ID + ", " +
                        Combined.BOOKMARK_ID + ", " +
@@ -1150,34 +1115,16 @@ public class BrowserProvider extends Sha
                            Bookmarks.URL + " NOT IN (SELECT url FROM " + TABLE_TOPSITES + ")" +
                            " AND " +
                            Bookmarks.URL + " NOT IN (SELECT url " + pinnedSitesFromClause + ")" +
                            suggestedLimitClause + " )",
 
                            suggestedSiteArgs);
             }
 
-            if (hasPreparedBlankTiles) {
-                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.
-                           // Hence the weird SELECT * FROM (SELECT ...relevant suggested sites... LIMIT ?)
-                           " SELECT * FROM (SELECT " +
-                           Bookmarks._ID + ", " +
-                           Bookmarks._ID + " AS " + Combined.BOOKMARK_ID + ", " +
-                           " -1 AS " + Combined.HISTORY_ID + ", " +
-                           " NULL AS " + Combined.HISTORY_GUID + ", " +
-                           Bookmarks.URL + ", " +
-                           Bookmarks.TITLE + ", " +
-                           "NULL AS " + Combined.HISTORY_ID + ", " +
-                           TopSites.TYPE_BLANK + " as " + TopSites.TYPE +
-                           " FROM ( " + blanksBuilder.toString() + " )" +
-                           blanksLimitClause + " )");
-            }
-
             // If we retrieve more topsites than we have free positions for in the freeIdSubquery,
             // we will have topsites that don't receive a position when joining TABLE_TOPSITES
             // with freeIdSubquery. Hence we need to coalesce the position with a generated position.
             // We know that the difference in positions will be at most suggestedGridLimit, hence we
             // can add that to the rowid to generate a safe position.
             // I.e. if we have 6 pinned sites then positions 0..5 are filled, the JOIN results in
             // the first N rows having positions 6..(N+6), so row N+1 should receive a position that is at
             // least N+1+6, which is equal to rowid + 6.