Bug 926497 - Prevent the favicon cache from throwing NPEs at odd moments. r=rnewman
authorChris Kitching <chriskitching@linux.com>
Mon, 14 Oct 2013 10:37:03 -0700
changeset 150631 3f69e71fc0cff912d5cfbe7fa4f6aa2e76572915
parent 150630 8804ec54fbcbde6e92df7c3464f34a75962ef5b8
child 150632 d25fd4a6657da5ccfe213c55a9b0a7c9f4e399d6
push id3024
push userrnewman@mozilla.com
push dateMon, 14 Oct 2013 17:37:13 +0000
treeherderfx-team@3f69e71fc0cf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrnewman
bugs926497
milestone27.0a1
Bug 926497 - Prevent the favicon cache from throwing NPEs at odd moments. r=rnewman
mobile/android/base/favicons/Favicons.java
mobile/android/base/favicons/LoadFaviconTask.java
mobile/android/base/favicons/cache/FaviconCache.java
--- a/mobile/android/base/favicons/Favicons.java
+++ b/mobile/android/base/favicons/Favicons.java
@@ -346,16 +346,17 @@ public class Favicons {
         try {
             // Fall back to trying "someScheme:someDomain.someExtension/favicon.ico".
             URI u = new URI(pageURL);
             return new URI(u.getScheme(),
                            u.getAuthority(),
                            "/favicon.ico", null,
                            null).toString();
         } catch (URISyntaxException e) {
+            Log.e(LOGTAG, "URISyntaxException getting default favicon URL", e);
             return null;
         }
     }
 
     public static void removeLoadTask(long taskId) {
         sLoadTasks.remove(taskId);
     }
 
--- a/mobile/android/base/favicons/LoadFaviconTask.java
+++ b/mobile/android/base/favicons/LoadFaviconTask.java
@@ -229,16 +229,20 @@ public class LoadFaviconTask extends UiA
             }
 
             // If we found a faviconURL - use it.
             if (storedFaviconUrl != null) {
                 mFaviconUrl = storedFaviconUrl;
             } else {
                 // If we don't have a stored one, fall back to the default.
                 mFaviconUrl = Favicons.guessDefaultFaviconURL(mPageUrl);
+
+                if (TextUtils.isEmpty(mFaviconUrl)) {
+                    return null;
+                }
                 isUsingDefaultURL = true;
             }
         }
 
         // Check if favicon has failed - if so, give up. We need this check because, sometimes, we
         // didn't know the real Favicon URL until we asked the database.
         if (Favicons.isFailedFavicon(mFaviconUrl)) {
             return null;
--- a/mobile/android/base/favicons/cache/FaviconCache.java
+++ b/mobile/android/base/favicons/cache/FaviconCache.java
@@ -200,16 +200,20 @@ public class FaviconCache {
     /**
      * Determine if the provided favicon URL is marked as a failure (Has failed to load before -
      * such icons get blacklisted for a time to prevent us endlessly retrying.)
      *
      * @param faviconURL Favicon URL to check if failed in memcache.
      * @return true if this favicon is blacklisted, false otherwise.
      */
     public boolean isFailedFavicon(String faviconURL) {
+        if (faviconURL == null) {
+            return true;
+        }
+
         startRead();
 
         boolean isExpired = false;
         boolean isAborting = false;
 
         try {
             // If we don't have it in the cache, it certainly isn't a known failure.
             if (!mBackingMap.containsKey(faviconURL)) {
@@ -236,17 +240,17 @@ public class FaviconCache {
             }
         } catch (Exception unhandled) {
             // Handle any exception thrown and return the locks to a sensible state.
             finishRead();
 
             // Flag to prevent finally from doubly-unlocking.
             isAborting = true;
             Log.e(LOGTAG, "FaviconCache exception!", unhandled);
-            return false;
+            return true;
         }  finally {
             if (!isAborting) {
                 if (isExpired) {
                     // No longer expired.
                     upgradeReadToWrite();
                 } else {
                     finishRead();
                 }