Bug 1388396: Add MinimumSizeProcessor; .forActivityStream takes effect. r=sebastian
☠☠ backed out by dec35bc5233c ☠ ☠
authorMichael Comella <michael.l.comella@gmail.com>
Mon, 11 Sep 2017 17:41:33 -0700
changeset 429937 44fa7b379742377244961b988bfbf56dd92965dc
parent 429936 77f982a2f17ba33e49f56ee87dfff5be7ced8fb5
child 429938 3300c15011d3a6661aaf8eee254b693d7e99f392
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssebastian
bugs1388396
milestone57.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 1388396: Add MinimumSizeProcessor; .forActivityStream takes effect. r=sebastian This patch is necessary for the previous changesets to take effect. MozReview-Commit-ID: 98IemAEgbmi
mobile/android/base/java/org/mozilla/gecko/icons/IconRequestExecutor.java
mobile/android/base/java/org/mozilla/gecko/icons/loader/IconGenerator.java
mobile/android/base/java/org/mozilla/gecko/icons/processing/MinimumSizeProcessor.java
mobile/android/base/moz.build
--- a/mobile/android/base/java/org/mozilla/gecko/icons/IconRequestExecutor.java
+++ b/mobile/android/base/java/org/mozilla/gecko/icons/IconRequestExecutor.java
@@ -23,16 +23,17 @@ import org.mozilla.gecko.icons.preparati
 import org.mozilla.gecko.icons.preparation.FilterMimeTypes;
 import org.mozilla.gecko.icons.preparation.FilterPrivilegedUrls;
 import org.mozilla.gecko.icons.preparation.LookupIconUrl;
 import org.mozilla.gecko.icons.preparation.Preparer;
 import org.mozilla.gecko.icons.preparation.SuggestedSitePreparer;
 import org.mozilla.gecko.icons.processing.ColorProcessor;
 import org.mozilla.gecko.icons.processing.DiskProcessor;
 import org.mozilla.gecko.icons.processing.MemoryProcessor;
+import org.mozilla.gecko.icons.processing.MinimumSizeProcessor;
 import org.mozilla.gecko.icons.processing.Processor;
 import org.mozilla.gecko.icons.processing.ResizingProcessor;
 
 import java.util.Arrays;
 import java.util.List;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Future;
 import java.util.concurrent.LinkedBlockingQueue;
@@ -121,17 +122,21 @@ import java.util.concurrent.TimeUnit;
 
             // Resize the icon to match the target size (if possible)
             new ResizingProcessor(),
 
             // Extract the dominant color from the icon
             new ColorProcessor(),
 
             // Store the icon in the memory cache
-            new MemoryProcessor()
+            new MemoryProcessor(),
+
+            // Substitute a generated icon if the final result is not large enough.
+            // Expected to be called after ResizingProcessor.
+            new MinimumSizeProcessor()
     );
 
     private static final ExecutorService EXECUTOR;
     static {
         final ThreadFactory factory = new ThreadFactory() {
             @Override
             public Thread newThread(@NonNull Runnable runnable) {
                 Thread thread = new Thread(runnable, "GeckoIconTask");
--- a/mobile/android/base/java/org/mozilla/gecko/icons/loader/IconGenerator.java
+++ b/mobile/android/base/java/org/mozilla/gecko/icons/loader/IconGenerator.java
@@ -50,17 +50,17 @@ public class IconGenerator implements Ic
         }
 
         return generate(request.getContext(), request.getPageUrl());
     }
 
     /**
      * Generate default favicon for the given page URL.
      */
-    @VisibleForTesting static IconResponse generate(Context context, String pageURL) {
+    public static IconResponse generate(Context context, String pageURL) {
         final Resources resources = context.getResources();
         final int widthAndHeight = resources.getDimensionPixelSize(R.dimen.favicon_bg);
         final int roundedCorners = resources.getDimensionPixelOffset(R.dimen.favicon_corner_radius);
 
         final Bitmap favicon = Bitmap.createBitmap(widthAndHeight, widthAndHeight, Bitmap.Config.ARGB_8888);
         final Canvas canvas = new Canvas(favicon);
 
         final int color = pickColor(pageURL);
new file mode 100644
--- /dev/null
+++ b/mobile/android/base/java/org/mozilla/gecko/icons/processing/MinimumSizeProcessor.java
@@ -0,0 +1,37 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package org.mozilla.gecko.icons.processing;
+
+import org.mozilla.gecko.icons.IconRequest;
+import org.mozilla.gecko.icons.IconResponse;
+import org.mozilla.gecko.icons.loader.IconGenerator;
+
+/**
+ * Substitutes a generated image for the given icon if it doesn't not meet the minimum size requirement.
+ *
+ * Ideally, we would discard images while we're loading them: if we found an icon that was too small, we
+ * would have the opportunity to search other places before generating an icon and we wouldn't duplicate
+ * the generate icon call. However, this turned out to be non-trivial: a single icon will appear as two
+ * different sizes when given to the loader, the original size the first time it's loaded and the size after
+ * {@link ResizingProcessor} the second time, when it's loaded from the cache. It turned out to be much simpler
+ * to enforce the requirement that...
+ *
+ * This processor is expected to be called after {@link ResizingProcessor}.
+ */
+public class MinimumSizeProcessor implements Processor {
+
+    @Override
+    public void process(final IconRequest request, final IconResponse response) {
+        // We expect that this bitmap has already been scaled by ResizingProcessor.
+        if (response.getBitmap().getWidth() >= request.getMinimumSizePxAfterScaling()) {
+            return;
+        }
+
+        // This is fragile: ideally, we can return the generated response but instead we're mutating the argument.
+        final IconResponse generatedResponse = IconGenerator.generate(request.getContext(), request.getPageUrl());
+        response.updateBitmap(generatedResponse.getBitmap());
+        response.updateColor(generatedResponse.getColor());
+    }
+}
--- a/mobile/android/base/moz.build
+++ b/mobile/android/base/moz.build
@@ -745,16 +745,17 @@ gbjar.sources += ['java/org/mozilla/geck
     'icons/preparation/FilterMimeTypes.java',
     'icons/preparation/FilterPrivilegedUrls.java',
     'icons/preparation/LookupIconUrl.java',
     'icons/preparation/Preparer.java',
     'icons/preparation/SuggestedSitePreparer.java',
     'icons/processing/ColorProcessor.java',
     'icons/processing/DiskProcessor.java',
     'icons/processing/MemoryProcessor.java',
+    'icons/processing/MinimumSizeProcessor.java',
     'icons/processing/Processor.java',
     'icons/processing/ResizingProcessor.java',
     'icons/storage/DiskStorage.java',
     'icons/storage/FailureCache.java',
     'icons/storage/MemoryStorage.java',
     'IntentHelper.java',
     'LauncherActivity.java',
     'lwt/LightweightTheme.java',