Bug 1300543 - LegacyLoader: Skip loading from legacy storage if network download is permitted. r=ahunt
authorSebastian Kaspari <s.kaspari@gmail.com>
Mon, 05 Sep 2016 15:38:27 +0200
changeset 312976 c5f01980a16a9cd6662868714c03228622bef2fa
parent 312975 2269f770b2e7a2d103077ca1ada9af1785b648b2
child 312977 ceb8ca8e9b2dd446ee8bbe2417baed20ac38f4c6
push id81509
push usercbook@mozilla.com
push dateWed, 07 Sep 2016 15:23:10 +0000
treeherdermozilla-inbound@80dccdd8c94a [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: Skip loading from legacy storage if network download is permitted. r=ahunt If we are allowed to load the icon from the network then skip loading from the legacy storage and just load a fresh icon. This will avoid touching the legacy storage (disk) every time before downloading an icon. MozReview-Commit-ID: C9hYqISno6U
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
@@ -20,33 +20,39 @@ import org.mozilla.gecko.icons.IconRespo
  * for a couple of releases and be removed afterwards.
  *
  * When updating to an app version with the new loaders our initial storage won't have any data so
  * we need to continue loading from the database storage until the new storage has a good set of data.
  */
 public class LegacyLoader implements IconLoader {
     @Override
     public IconResponse load(IconRequest request) {
+        if (!request.shouldSkipNetwork()) {
+            // If we are allowed to load from the network for this request then just ommit the legacy
+            // loader and fetch a fresh new icon.
+            return null;
+        }
+
         if (request.shouldSkipDisk()) {
             return null;
         }
 
         final Bitmap bitmap = loadBitmapFromDatabase(request);
 
         if (bitmap == null) {
             return null;
         }
 
         return IconResponse.create(bitmap);
     }
 
     /* package-private */ Bitmap loadBitmapFromDatabase(IconRequest request) {
         final Context context = request.getContext();
         final ContentResolver contentResolver = context.getContentResolver();
-        final BrowserDB db = GeckoProfile.get(request.getContext()).getDB();
+        final BrowserDB db = GeckoProfile.get(context).getDB();
 
         // We ask the database for the favicon URL and ignore the icon URL in the request object:
         // As we are not updating the database anymore the icon might be stored under a different URL.
         final String legacyFaviconUrl = db.getFaviconURLFromPageURL(contentResolver, request.getPageUrl());
         if (legacyFaviconUrl == null) {
             // No URL -> Nothing to load.
             return null;
         }
--- 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
@@ -22,26 +22,42 @@ 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";
 
     @Test
-    public void testDatabaseIsQueriesForNormalRequests() {
+    public void testDatabaseIsQueriesForNormalRequestsWithNetworkSkipped() {
+        final IconRequest request = Icons.with(RuntimeEnvironment.application)
+                .pageUrl(TEST_PAGE_URL)
+                .icon(IconDescriptor.createGenericIcon(TEST_ICON_URL))
+                .skipNetwork()
+                .build();
+
+        final LegacyLoader loader = spy(new LegacyLoader());
+        final IconResponse response = loader.load(request);
+
+        verify(loader).loadBitmapFromDatabase(request);
+
+        Assert.assertNull(response);
+    }
+
+    @Test
+    public void testNothingIsLoadedIfNetworkIsNotSkipped() {
         final IconRequest request = Icons.with(RuntimeEnvironment.application)
                 .pageUrl(TEST_PAGE_URL)
                 .icon(IconDescriptor.createGenericIcon(TEST_ICON_URL))
                 .build();
 
         final LegacyLoader loader = spy(new LegacyLoader());
         final IconResponse response = loader.load(request);
 
-        verify(loader).loadBitmapFromDatabase(request);
+        verify(loader, never()).loadBitmapFromDatabase(request);
 
         Assert.assertNull(response);
     }
 
     @Test
     public void testNothingIsLoadedIfDiskSHouldBeSkipped() {
         final IconRequest request = Icons.with(RuntimeEnvironment.application)
                 .pageUrl(TEST_PAGE_URL)
@@ -57,16 +73,17 @@ public class TestLegacyLoader {
         Assert.assertNull(response);
     }
 
     @Test
     public void testLoadedBitmapIsReturnedAsResponse() {
         final IconRequest request = Icons.with(RuntimeEnvironment.application)
                 .pageUrl(TEST_PAGE_URL)
                 .icon(IconDescriptor.createGenericIcon(TEST_ICON_URL))
+                .skipNetwork()
                 .build();
 
         final Bitmap bitmap = mock(Bitmap.class);
 
         final LegacyLoader loader = spy(new LegacyLoader());
         doReturn(bitmap).when(loader).loadBitmapFromDatabase(request);
 
         final IconResponse response = loader.load(request);