Bug 931918 - Part 3: Rework failure case handling in download. r=bnicholson, a=bajaj
authorRichard Newman <rnewman@mozilla.com>
Thu, 31 Oct 2013 10:35:17 -0700
changeset 167335 a5dbb1214ea908f89b2ab24e59e15b14827061a3
parent 167334 9e54369efcb059e0bc9b293d2b7a8c21685d448c
child 167336 f8f0cd453b9539f9a764166c6d7cc5bbe218f4c6
push id428
push userbbajaj@mozilla.com
push dateTue, 28 Jan 2014 00:16:25 +0000
treeherdermozilla-release@cd72a7ff3a75 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbnicholson, bajaj
bugs931918
milestone27.0a2
Bug 931918 - Part 3: Rework failure case handling in download. r=bnicholson, a=bajaj
mobile/android/base/favicons/LoadFaviconTask.java
--- a/mobile/android/base/favicons/LoadFaviconTask.java
+++ b/mobile/android/base/favicons/LoadFaviconTask.java
@@ -164,39 +164,42 @@ public class LoadFaviconTask extends UiA
      * a JAR URI.
      */
     private static Bitmap fetchJARFavicon(String uri) {
         if (uri == null) {
             return null;
         }
         if (uri.startsWith("jar:jar:")) {
             Log.d(LOGTAG, "Fetching favicon from JAR.");
-            return GeckoJarReader.getBitmap(sContext.getResources(), uri);
+            try {
+                return GeckoJarReader.getBitmap(sContext.getResources(), uri);
+            } catch (Exception e) {
+                // Just about anything could happen here.
+                Log.w(LOGTAG, "Error fetching favicon from JAR.", e);
+                return null;
+            }
         }
         return null;
     }
 
     // Runs in background thread.
+    // Does not attempt to fetch from JARs.
     private Bitmap downloadFavicon(URI targetFaviconURI) {
         if (targetFaviconURI == null) {
             return null;
         }
 
-        final String uriString = targetFaviconURI.toString();
-        Bitmap image = fetchJARFavicon(uriString);
-        if (image != null) {
-            return image;
-        }
-
         // Only get favicons for HTTP/HTTPS.
         String scheme = targetFaviconURI.getScheme();
         if (!"http".equals(scheme) && !"https".equals(scheme)) {
             return null;
         }
 
+        Bitmap image = null;
+
         // skia decoder sometimes returns null; workaround is to use BufferedHttpEntity
         // http://groups.google.com/group/android-developers/browse_thread/thread/171b8bf35dbbed96/c3ec5f45436ceec8?lnk=raot
         try {
             // Try the URL we were given.
             HttpResponse response = tryDownload(targetFaviconURI);
             if (response == null) {
                 return null;
             }
@@ -292,17 +295,17 @@ public class LoadFaviconTask extends UiA
             loadsInFlight.put(mFaviconUrl, this);
         }
 
         if (isCancelled()) {
             return null;
         }
 
         image = loadFaviconFromDb();
-        if (image != null && image.getWidth() > 0 && image.getHeight() > 0) {
+        if (imageIsValid(image)) {
             return image;
         }
 
         if (mOnlyFromLocal || isCancelled()) {
             return null;
         }
 
         // Let's see if it's in a JAR.
@@ -316,32 +319,58 @@ public class LoadFaviconTask extends UiA
             image = downloadFavicon(new URI(mFaviconUrl));
         } catch (URISyntaxException e) {
             Log.e(LOGTAG, "The provided favicon URL is not valid");
             return null;
         } catch (Exception e) {
             Log.e(LOGTAG, "Couldn't download favicon.", e);
         }
 
+        if (imageIsValid(image)) {
+            saveFaviconToDb(image);
+            return image;
+        }
+
+        if (isUsingDefaultURL) {
+            Favicons.putFaviconInFailedCache(mFaviconUrl);
+            return null;
+        }
+
         // If we're not already trying the default URL, try it now.
-        if (image == null && !isUsingDefaultURL) {
-            try {
-                image = downloadFavicon(new URI(Favicons.guessDefaultFaviconURL(mPageUrl)));
-            } catch (URISyntaxException e){
-                // Not interesting. It was an educated guess, anyway.
-            }
+        final String guessed = Favicons.guessDefaultFaviconURL(mPageUrl);
+        if (guessed == null) {
+            Favicons.putFaviconInFailedCache(mFaviconUrl);
+            return null;
         }
 
-        if (image != null && image.getWidth() > 0 && image.getHeight() > 0) {
-            saveFaviconToDb(image);
-        } else {
-            Favicons.putFaviconInFailedCache(mFaviconUrl);
+        image = fetchJARFavicon(guessed);
+        if (imageIsValid(image)) {
+            // We don't want to put this into the DB.
+            return image;
+        }
+
+        try {
+            image = downloadFavicon(new URI(guessed));
+        } catch (Exception e) {
+            // Not interesting. It was an educated guess, anyway.
+            return null;
         }
 
-        return image;
+        if (imageIsValid(image)) {
+            saveFaviconToDb(image);
+            return image;
+        }
+
+        return null;
+    }
+
+    private static boolean imageIsValid(final Bitmap image) {
+        return image != null &&
+               image.getWidth() > 0 &&
+               image.getHeight() > 0;
     }
 
     @Override
     protected void onPostExecute(Bitmap image) {
         if (mIsChaining) {
             return;
         }