Bug 1340957 - Don't rely on SuggestedSites being loaded r?sebastian draft
authorAndrzej Hunt <ahunt@mozilla.com>
Tue, 21 Feb 2017 08:21:56 -0800
changeset 488749 73a75459a595948f718ce49cba1f8696f3762605
parent 488741 22ec1dab9e821676f4204d36ce9801803032f504
child 488773 0db9eabd7acc97c124ce302e274c0046b12cb232
push id46617
push userahunt@mozilla.com
push dateThu, 23 Feb 2017 17:29:30 +0000
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();