Bug 1300543 - LegacyLoader: Only load if there's one icon URL left. r?ahunt draft
authorSebastian Kaspari <s.kaspari@gmail.com>
Mon, 05 Sep 2016 16:44:33 +0200
changeset 410110 cc3324be0696d66e7d3be3cdb228552fbf59920d
parent 410109 3f3ede030796ec71d17a03bd2a7c415d2c1ebac6
child 410111 60990f5ae8e723866b538f53d55384f1072c02d1
push id28648
push users.kaspari@gmail.com
push dateTue, 06 Sep 2016 08:42:47 +0000
reviewersahunt
bugs1300543
milestone51.0a1
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();
+        }
+    }
 }