Bug 1300543 - LegacyLoader: Skip loading from legacy storage if network download is permitted. r?ahunt draft
authorSebastian Kaspari <s.kaspari@gmail.com>
Mon, 05 Sep 2016 15:38:27 +0200
changeset 410109 3f3ede030796ec71d17a03bd2a7c415d2c1ebac6
parent 410108 117eba0ba79c828027ece1770db700e21513bffa
child 410110 cc3324be0696d66e7d3be3cdb228552fbf59920d
push id28648
push users.kaspari@gmail.com
push dateTue, 06 Sep 2016 08:42:47 +0000
reviewersahunt
bugs1300543
milestone51.0a1
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);