author | Sebastian Kaspari <s.kaspari@gmail.com> |
Mon, 05 Sep 2016 16:44:33 +0200 | |
changeset 312864 | ceb8ca8e9b2dd446ee8bbe2417baed20ac38f4c6 |
parent 312863 | c5f01980a16a9cd6662868714c03228622bef2fa |
child 312865 | 714566fb31ec58b36489c6cbd9a4904fac3da778 |
push id | 30663 |
push user | cbook@mozilla.com |
push date | Wed, 07 Sep 2016 15:12:31 +0000 |
treeherder | mozilla-central@3d0b41fdd93b [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | ahunt |
bugs | 1300543 |
milestone | 51.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
|
--- 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(); + } + } }