Bug 1300543 - LegacyLoader: Only load if there's one icon URL left. r=ahunt
authorSebastian Kaspari <s.kaspari@gmail.com>
Mon, 05 Sep 2016 16:44:33 +0200
changeset 312864 ceb8ca8e9b2dd446ee8bbe2417baed20ac38f4c6
parent 312863 c5f01980a16a9cd6662868714c03228622bef2fa
child 312865 714566fb31ec58b36489c6cbd9a4904fac3da778
push id30663
push usercbook@mozilla.com
push dateWed, 07 Sep 2016 15:12:31 +0000
treeherdermozilla-central@3d0b41fdd93b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersahunt
bugs1300543
milestone51.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 1300543 - LegacyLoader: Only load if there's one icon URL left. r=ahunt Let's try to load from the legacy loader only if there's one icon left and the other loads have failed. We will ignore the icon URL anyways and try to receive the legacy icon URL from the database. MozReview-Commit-ID: Kr7gHXBuAs7
mobile/android/base/java/org/mozilla/gecko/icons/loader/LegacyLoader.java
mobile/android/tests/background/junit4/src/org/mozilla/gecko/icons/loader/TestLegacyLoader.java
--- a/mobile/android/base/java/org/mozilla/gecko/icons/loader/LegacyLoader.java
+++ b/mobile/android/base/java/org/mozilla/gecko/icons/loader/LegacyLoader.java
@@ -30,16 +30,23 @@ public class LegacyLoader implements Ico
             // loader and fetch a fresh new icon.
             return null;
         }
 
         if (request.shouldSkipDisk()) {
             return null;
         }
 
+        if (request.getIconCount() > 1) {
+            // There are still other icon URLs to try. Let's try to load from the legacy loader only
+            // if there's one icon left and the other loads have failed. We will ignore the icon URL
+            // anyways and try to receive the legacy icon URL from the database.
+            return null;
+        }
+
         final Bitmap bitmap = loadBitmapFromDatabase(request);
 
         if (bitmap == null) {
             return null;
         }
 
         return IconResponse.create(bitmap);
     }
--- a/mobile/android/tests/background/junit4/src/org/mozilla/gecko/icons/loader/TestLegacyLoader.java
+++ b/mobile/android/tests/background/junit4/src/org/mozilla/gecko/icons/loader/TestLegacyLoader.java
@@ -10,26 +10,30 @@ import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mozilla.gecko.background.testhelpers.TestRunner;
 import org.mozilla.gecko.icons.IconDescriptor;
 import org.mozilla.gecko.icons.IconRequest;
 import org.mozilla.gecko.icons.IconResponse;
 import org.mozilla.gecko.icons.Icons;
 import org.robolectric.RuntimeEnvironment;
 
+import java.util.Iterator;
+
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
 
 @RunWith(TestRunner.class)
 public class TestLegacyLoader {
     private static final String TEST_PAGE_URL = "http://www.mozilla.org";
     private static final String TEST_ICON_URL = "https://example.org/favicon.ico";
+    private static final String TEST_ICON_URL_2 = "https://example.com/page/favicon.ico";
+    private static final String TEST_ICON_URL_3 = "https://example.net/icon/favicon.ico";
 
     @Test
     public void testDatabaseIsQueriesForNormalRequestsWithNetworkSkipped() {
         final IconRequest request = Icons.with(RuntimeEnvironment.application)
                 .pageUrl(TEST_PAGE_URL)
                 .icon(IconDescriptor.createGenericIcon(TEST_ICON_URL))
                 .skipNetwork()
                 .build();
@@ -86,9 +90,42 @@ public class TestLegacyLoader {
         final LegacyLoader loader = spy(new LegacyLoader());
         doReturn(bitmap).when(loader).loadBitmapFromDatabase(request);
 
         final IconResponse response = loader.load(request);
 
         Assert.assertNotNull(response);
         Assert.assertEquals(bitmap, response.getBitmap());
     }
+
+    @Test
+    public void testLoaderOnlyLoadsIfThereIsOneIconLeft() {
+        final IconRequest request = Icons.with(RuntimeEnvironment.application)
+                .pageUrl(TEST_PAGE_URL)
+                .icon(IconDescriptor.createGenericIcon(TEST_ICON_URL))
+                .icon(IconDescriptor.createGenericIcon(TEST_ICON_URL_2))
+                .icon(IconDescriptor.createGenericIcon(TEST_ICON_URL_3))
+                .skipNetwork()
+                .build();
+
+        final LegacyLoader loader = spy(new LegacyLoader());
+        doReturn(mock(Bitmap.class)).when(loader).loadBitmapFromDatabase(request);
+
+        // First load doesn't load an icon.
+        Assert.assertNull(loader.load(request));
+
+        // Second load doesn't load an icon.
+        removeFirstIcon(request);
+        Assert.assertNull(loader.load(request));
+
+        // Now only one icon is left and a response will be returned.
+        removeFirstIcon(request);
+        Assert.assertNotNull(loader.load(request));
+    }
+
+    private void removeFirstIcon(IconRequest request) {
+        final Iterator<IconDescriptor> iterator = request.getIconIterator();
+        if (iterator.hasNext()) {
+            iterator.next();
+            iterator.remove();
+        }
+    }
 }