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
--- 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();
+ }
+ }
}