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 165494 3f69e71fc0cff912d5cfbe7fa4f6aa2e76572915
parent 165493 8804ec54fbcbde6e92df7c3464f34a75962ef5b8
child 165495 d25fd4a6657da5ccfe213c55a9b0a7c9f4e399d6
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)
reviewersrnewman
bugs926497
milestone27.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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();
                 }