Bug 826400 - Part 1: Add touchIcon to URLMetadataTable r=nalexander draft
authorAndrzej Hunt <ahunt@mozilla.com>
Mon, 21 Dec 2015 17:26:52 -0800
changeset 319138 854144400164d80114a84dcd77e05cf27647280f
parent 319137 c342816f76c048db725e4c44e64bb3cfdcde7aa9
child 319139 074a36a6acc9e3aa8f63c71403a7cc4afdb31032
push id8984
push userahunt@mozilla.com
push dateTue, 05 Jan 2016 23:06:12 +0000
reviewersnalexander
bugs826400
milestone46.0a1
Bug 826400 - Part 1: Add touchIcon to URLMetadataTable r=nalexander
mobile/android/base/java/org/mozilla/gecko/db/BrowserDatabaseHelper.java
mobile/android/base/java/org/mozilla/gecko/db/LocalURLMetadata.java
mobile/android/base/java/org/mozilla/gecko/db/URLMetadataTable.java
--- a/mobile/android/base/java/org/mozilla/gecko/db/BrowserDatabaseHelper.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/BrowserDatabaseHelper.java
@@ -34,17 +34,17 @@ import android.database.sqlite.SQLiteOpe
 import android.net.Uri;
 import android.os.Build;
 import android.util.Log;
 
 
 final class BrowserDatabaseHelper extends SQLiteOpenHelper {
     private static final String LOGTAG = "GeckoBrowserDBHelper";
 
-    public static final int DATABASE_VERSION = 26; // Bug 1128675
+    public static final int DATABASE_VERSION = 27; // Bug 1128675
     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_FAVICONS = Favicons.TABLE_NAME;
     static final String TABLE_THUMBNAILS = Thumbnails.TABLE_NAME;
--- a/mobile/android/base/java/org/mozilla/gecko/db/LocalURLMetadata.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/LocalURLMetadata.java
@@ -35,16 +35,17 @@ public class LocalURLMetadata implements
 
     // This returns a list of columns in the table. It's used to simplify some loops for reading/writing data.
     @SuppressWarnings("serial")
     private final Set<String> getModel() {
         return new HashSet<String>() {{
             add(URLMetadataTable.URL_COLUMN);
             add(URLMetadataTable.TILE_IMAGE_URL_COLUMN);
             add(URLMetadataTable.TILE_COLOR_COLUMN);
+            add(URLMetadataTable.TOUCH_ICON_COLUMN);
         }};
     }
 
     // Store a cache of recent results. This number is chosen to match the max number of tiles on about:home
     private static final int CACHE_SIZE = 9;
     // Note: Members of this cache are unmodifiable.
     private final LruCache<String, Map<String, Object>> cache = new LruCache<String, Map<String, Object>>(CACHE_SIZE);
 
@@ -91,50 +92,68 @@ public class LocalURLMetadata implements
 
     /**
      * Returns an unmodifiable Map of url->Metadata (i.e. A second HashMap) for a list of urls.
      * Must not be called from UI or Gecko threads.
      */
     @Override
     public Map<String, Map<String, Object>> getForURLs(final ContentResolver cr,
                                                        final List<String> urls,
-                                                       final List<String> columns) {
+                                                       final List<String> requestedColumns) {
         ThreadUtils.assertNotOnUiThread();
         ThreadUtils.assertNotOnGeckoThread();
 
         final Map<String, Map<String, Object>> data = new HashMap<String, Map<String, Object>>();
 
         // Nothing to query for
-        if (urls.isEmpty() || columns.isEmpty()) {
+        if (urls.isEmpty() || requestedColumns.isEmpty()) {
             Log.e(LOGTAG, "Queried metadata for nothing");
             return data;
         }
 
         // Search the cache for any of these urls
         List<String> urlsToQuery = new ArrayList<String>();
         for (String url : urls) {
             final Map<String, Object> hit = cache.get(url);
             if (hit != null) {
-                // Cache hit!
-                data.put(url, hit);
+                // Cache hit: we've found the URL in the cache, however we may not have cached the desired columns
+                // for that URL. Hence we need to check whether our cache hit contains those columns, and directly
+                // retrieve the desired data if not. (E.g. the top sites panel retrieves the tile, and tilecolor. If
+                // we later try to retrieve the touchIcon for a top-site the cache hit will only point to
+                // tile+tilecolor, and not the required touchIcon. In this case we don't want to use the cache.)
+                boolean useCache = true;
+                for (String c: requestedColumns) {
+                    if (!hit.containsKey(c)) {
+                        useCache = false;
+                    }
+                }
+                if (useCache) {
+                    data.put(url, hit);
+                } else {
+                    urlsToQuery.add(url);
+                }
             } else {
                 urlsToQuery.add(url);
             }
         }
 
         Telemetry.addToHistogram("FENNEC_TILES_CACHE_HIT", data.size());
 
         // If everything was in the cache, we're done!
         if (urlsToQuery.size() == 0) {
             return Collections.unmodifiableMap(data);
         }
 
         final String selection = DBUtils.computeSQLInClause(urlsToQuery.size(), URLMetadataTable.URL_COLUMN);
+        List<String> columns = requestedColumns;
         // We need the url to build our final HashMap, so we force it to be included in the query.
         if (!columns.contains(URLMetadataTable.URL_COLUMN)) {
+            // The requestedColumns may be immutable (e.g. if the caller used Collections.singletonList), hence
+            // we have to create a copy.
+            columns = new ArrayList<String>(columns);
             columns.add(URLMetadataTable.URL_COLUMN);
         }
 
         final Cursor cursor = cr.query(uriWithProfile,
                                        columns.toArray(new String[columns.size()]), // columns,
                                        selection, // selection
                                        urlsToQuery.toArray(new String[urlsToQuery.size()]), // selectionargs
                                        null);
--- a/mobile/android/base/java/org/mozilla/gecko/db/URLMetadataTable.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/URLMetadataTable.java
@@ -22,45 +22,55 @@ public class URLMetadataTable extends Ba
     // Uri for querying this table
     static final Uri CONTENT_URI = Uri.withAppendedPath(BrowserContract.AUTHORITY_URI, "metadata");
 
     // Columns in the table
     public static final String ID_COLUMN = "id";
     public static final String URL_COLUMN = "url";
     public static final String TILE_IMAGE_URL_COLUMN = "tileImage";
     public static final String TILE_COLOR_COLUMN = "tileColor";
+    public static final String TOUCH_ICON_COLUMN = "touchIcon";
 
     URLMetadataTable() { }
 
     @Override
     protected String getTable() {
         return TABLE;
     }
 
     @Override
     public void onCreate(SQLiteDatabase db) {
         String create = "CREATE TABLE " + TABLE + " (" +
             ID_COLUMN + " INTEGER PRIMARY KEY, " +
             URL_COLUMN + " TEXT NON NULL UNIQUE, " +
             TILE_IMAGE_URL_COLUMN + " STRING, " +
-            TILE_COLOR_COLUMN + " STRING);";
+            TILE_COLOR_COLUMN + " STRING, " +
+            TOUCH_ICON_COLUMN + " STRING);";
         db.execSQL(create);
     }
 
+    private void upgradeDatabaseFrom26To27(SQLiteDatabase db) {
+        db.execSQL("ALTER TABLE " + TABLE +
+                   " ADD COLUMN " + TOUCH_ICON_COLUMN + " STRING");
+    }
+
     @Override
     public void onUpgrade(SQLiteDatabase db, int oldVersion, int newVersion) {
-        // This table was added in v19 of the db. Force its creation if we're coming from an earlier version
+        // This table was added in v21 of the db. Force its creation if we're coming from an earlier version
         if (newVersion >= 21 && oldVersion < 21) {
             onCreate(db);
         }
 
         // Removed the redundant metadata_url_idx index in version 26
         if (newVersion >= 26 && oldVersion < 26) {
             db.execSQL("DROP INDEX IF EXISTS metadata_url_idx");
         }
+        if (newVersion >= 27 && oldVersion < 27) {
+            upgradeDatabaseFrom26To27(db);
+        }
     }
 
     @Override
     public Table.ContentProviderInfo[] getContentProviderInfo() {
         return new Table.ContentProviderInfo[] {
             new Table.ContentProviderInfo(TABLE_ID_NUMBER, TABLE)
         };
     }