Bug 1340957 - Don't rely on SuggestedSites being loaded r=sebastian
authorAndrzej Hunt <ahunt@mozilla.com>
Tue, 21 Feb 2017 08:21:56 -0800
changeset 373763 ea22d7caa61894c693987966aa2c2a5c984f3ed9
parent 373762 be46bf0c2fd6c7526a304d96efa2ecfb16ee5676
child 373764 23114a28082a06937c9ecb104f3b45c8f9f3ab05
push id10863
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 23:02:23 +0000
treeherdermozilla-aurora@0931190cd725 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssebastian
bugs1340957
milestone54.0a1
Bug 1340957 - Don't rely on SuggestedSites being loaded r=sebastian MozReview-Commit-ID: JjWurcyDoWQ
mobile/android/base/java/org/mozilla/gecko/icons/loader/SuggestedSiteLoader.java
mobile/android/base/java/org/mozilla/gecko/icons/preparation/SuggestedSitePreparer.java
--- a/mobile/android/base/java/org/mozilla/gecko/icons/loader/SuggestedSiteLoader.java
+++ b/mobile/android/base/java/org/mozilla/gecko/icons/loader/SuggestedSiteLoader.java
@@ -49,16 +49,23 @@ public class SuggestedSiteLoader impleme
                 .load(iconURL)
                 .noFade()
                 .get();
     }
 
     private IconResponse buildIcon(final Context context, final String siteURL, final int targetSize) {
         final SuggestedSites suggestedSites = BrowserDB.from(context).getSuggestedSites();
 
+        if (suggestedSites == null) {
+            // See longer explanation in SuggestedSitePreparer: suggested sites aren't always loaded.
+            // If they aren't, SuggestedSitePreparer won't suggest any bundled icons so we should
+            // never try to load them anyway, but we should double check here to be completely safe.
+            return null;
+        }
+
         final String iconLocation = suggestedSites.getImageUrlForUrl(siteURL);
         final String backgroundColorString = suggestedSites.getBackgroundColorForUrl(siteURL);
 
         if (iconLocation == null || backgroundColorString == null) {
             // There's little we can do if loading a bundled resource fails: this failure could
             // be caused by a distribution (as opposed to Gecko), so we should just shout loudly,
             // as opposed to crashing:
             Log.e(LOGTAG, "Unable to find tile data definitions for site:" + siteURL);
--- a/mobile/android/base/java/org/mozilla/gecko/icons/preparation/SuggestedSitePreparer.java
+++ b/mobile/android/base/java/org/mozilla/gecko/icons/preparation/SuggestedSitePreparer.java
@@ -4,56 +4,74 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 package org.mozilla.gecko.icons.preparation;
 
 import android.content.Context;
 import android.database.Cursor;
 
 import org.mozilla.gecko.db.BrowserContract;
 import org.mozilla.gecko.db.BrowserDB;
+import org.mozilla.gecko.db.SuggestedSites;
 import org.mozilla.gecko.icons.IconDescriptor;
 import org.mozilla.gecko.icons.IconRequest;
 import org.mozilla.gecko.icons.loader.SuggestedSiteLoader;
 
 import java.util.HashSet;
 import java.util.Set;
 
 public class SuggestedSitePreparer implements Preparer {
 
     private boolean initialised = false;
     private final Set<String> siteFaviconMap = new HashSet<>();
 
     // Loading suggested sites (and iterating over them) is potentially slow. The number of suggested
     // sites is low, and a HashSet containing them is therefore likely to be exceedingly small.
     // Hence we opt to iterate over the list once, and do an immediate lookup every time a favicon
     // is requested:
-    private void initialise(final Context context) {
-        final Cursor cursor = BrowserDB.from(context).getSuggestedSites().get(Integer.MAX_VALUE);
+    // Loading can fail if suggested sites haven't been initialised yet, only proceed if we return true.
+    private boolean initialise(final Context context) {
+        final SuggestedSites suggestedSites = BrowserDB.from(context).getSuggestedSites();
+
+        // suggestedSites may not have been initialised yet if BrowserApp isn't running yet. Suggested
+        // Sites are initialised in BrowserApp.onCreate(), but we might need to load icons when running
+        // custom tabs, as geckoview, etc. (But we don't care as much about the bundled icons in those
+        // scenarios.)
+        if (suggestedSites == null) {
+            return false;
+        }
+
+        final Cursor cursor = suggestedSites.get(Integer.MAX_VALUE);
         try {
             final int urlColumnIndex = cursor.getColumnIndexOrThrow(BrowserContract.Bookmarks.URL);
 
             while (cursor.moveToNext()) {
                 final String url = cursor.getString(urlColumnIndex);
                 siteFaviconMap.add(url);
             }
         } finally {
             cursor.close();
         }
+
+        return true;
     }
 
     @Override
     public void prepare(final IconRequest request) {
         if (request.shouldSkipDisk()) {
             return;
         }
 
         if (!initialised) {
-            initialise(request.getContext());
+            initialised = initialise(request.getContext());
 
-            initialised = true;
+            if (!initialised) {
+                // Early return: if we were unable to load suggested sites metdata, we won't be able
+                // to provide sites (but we'll still try again next time).
+                return;
+            }
         }
 
         final String siteURL = request.getPageUrl();
 
         if (siteFaviconMap.contains(siteURL)) {
             request.modify()
                     .icon(IconDescriptor.createBundledTileIcon(SuggestedSiteLoader.SUGGESTED_SITE_TOUCHTILE + request.getPageUrl()))
                     .deferBuild();