Photo Picker Dialog: Use sandboxed utility process for decoding images.

chrome/android/java/src/org/chromium/chrome/browser/photo_picker/BitmapUtils.java

Index: chrome/android/java/src/org/chromium/chrome/browser/photo_picker/BitmapUtils.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/BitmapUtils.java b/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/BitmapUtils.java
new file mode 100644
index 0000000000000000000000000000000000000000..3b80472a31da673a70a4cadadef1f31cfb42efd1
--- /dev/null
+++ b/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/BitmapUtils.java
@@ -0,0 +1,125 @@
+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chrome.browser.photo_picker;
+
+import android.graphics.Bitmap;
+import android.graphics.BitmapFactory;
+
+import java.io.FileDescriptor;
+
+class BitmapUtils {

[Theresa, 2017/04/13 02:25:26]

nit: JavaDoc for the class

[Finnur, 2017/04/18 17:21:14]

Done.

+    /**
+     * Takes a |bitmap| and returns a square thumbnail of |width| from the center of the bitmap
+     * specified.
+     * @param bitmap The bitmap to adjust.
+     * @param width The desired width.
+     * @return The new bitmap thumbnail.
+     */
+    public static Bitmap sizeBitmap(Bitmap bitmap, int width) {
+        bitmap = ensureMinSize(bitmap, width);
+        bitmap = cropToSquare(bitmap, width);

[Theresa, 2017/04/13 02:25:27]

I think that instead of manipulating the Bitmap here to ensure a minimum size
and crop to a square, we can rely on the Android framework to do the
scaling/centering for us. Android's ImageView has an scaleType property that
controls how the image is scaled and positioned within the view. If the
ImageView that contains the bitmaps is set to the desired size and the scaleType
is set to centerCrop then I think you'll achieve the same visual affect with
less programatic manipulation of the bitmaps.

[Finnur, 2017/04/18 17:21:14]

That's a good point. I'll take a look at this tomorrow (ran out of time).

+        return bitmap;
+    }
+
+    /**
+     * Given a FileDescriptor, decodes the contents and returns a bitmap of size |width|x|width|.
+     * @param descriptor The FileDescriptor for the file to read.
+     * @param width The width of the bitmap to return.
+     * @return The resulting bitmap.
+     */
+    public static Bitmap decodeBitmapFromFileDescriptor(FileDescriptor descriptor, int width) {
+        BitmapFactory.Options options = new BitmapFactory.Options();
+        options.inJustDecodeBounds = true;
+        BitmapFactory.decodeFileDescriptor(descriptor, null, options);
+        options.inSampleSize = calculateInSampleSize(options, width, width);
+        options.inJustDecodeBounds = false;
+        Bitmap bitmap = BitmapFactory.decodeFileDescriptor(descriptor, null, options);
+
+        if (bitmap == null) return null;
+
+        return sizeBitmap(bitmap, width);
+    }
+
+    /**
+     * Calculates the sub-sampling factor {@link BitmapFactory#inSampleSize} option for a given
+     * bitmap option, which will be used to create a bitmap of a pre-determined size (no larger than
+     * |width| and |height|).
+     * @param options The bitmap options to consider.
+     * @param width The requested width.
+     * @param height The requested height.
+     * @return The sub-sampling factor (1 = no change, 2 = half-size, etc).
+     */
+    private static int calculateInSampleSize(BitmapFactory.Options options, int width, int height) {

[Theresa, 2017/04/13 02:25:26]

nit: this method can be private

Also, since the width and height are the same can the two params be condensed to
one "size" or "desiredSize" param?

[Finnur, 2017/04/18 17:21:14]

Not sure I understand. It already is...
If you mean: Remove |static| then I'd point out that it is used from static
content, so that produces a compile error...
Done.

+        int inSampleSize = 1;
+
+        if (options.outHeight > height || options.outWidth > width) {
+            final int halfHeight = options.outHeight / 2;
+            final int halfWidth = options.outWidth / 2;
+
+            while ((halfHeight / inSampleSize) >= height || (halfWidth / inSampleSize) >= width) {

[Theresa, 2017/04/13 02:25:26]

I think the math for calculating the inSampleSize can be simplified. If the
image's real size is 4,000 x 4,000 and the desired size is 1,000, the
inSampleSize should be 4.

Looking at another example, if the image is 4,000 width x 6,000 height, based on
the ensureMinSize() method, I think the inSampleSize should be 4. The decoded
image will 1,000 x 1,5000 and the cropToSquare() method (or Android's ImageView
scaleType=centerCrop) would chop off 250 from the top and bottom of the bitmap.

I think this code will produce the correct inSampleSize:
if (options.outHeight > size && options.outWidth > size) {
    inSampleSize = Math.min(options.outHeight, options.outWidth) / size;
}

[Finnur, 2017/04/18 17:21:14]

The idea is to produce a photo that is no larger in both dimensions than |size|.
Lets take a real life photo from my Nexus 6, which is 4160x3120 (the photo, not
my Nexus 6). The real-estate space on that phone allows for thumbnails of size
461x461.

inSampleSize = Math.min(options.outHeight, options.outWidth) / size;
             = Math.min(3120, 4160) / 461;
             = 3120 / 461
             = 6 (integer division)
             
inSampleSize is rounded down to the nearest power-of two, so this would be
rounded to 4, I guess. The end result is a 1040x780 image, which is larger than
the requested size (461x461 max).

We want inSampleSize to hit a certain sweet-spot -- be as large as possible to
minimize CPU time and memory usage in decoding and transferring, but not so
large as to overly degrade the image quality. Therefore, I don't think we should
use Androids View here to crop, because then we're decoding to a bigger image
size than we can show and wasting both CPU and memory in the process.

However, looking at this again, I see this function can be simplified a bit...

[Theresa, 2017/04/18 19:06:20]

Shouldn't we ensure that the photo is at least |size| in both dimensions, while
minimizing how much larger than |size| it is?
With the method in the latest patchset, the inSampleSize would actually end up
being 16, right?

while ((height / inSampleSize) >= maxSize || (width / inSampleSize) >= maxSize)
{
    inSampleSize *= 2;
}

  4160 / 1 >= 461, inSampleSize = 2
  4160 / 2 = 2080, inSampleSize = 4
  4160 / 4 = 1040, inSampleSize = 8
  4160 / 8 = 520, inSampleSize = 16
  4160 / 16 = 260 -- DONE, image will be 195 x 260

Upscaling thumbnails generally doesn't look good. What's the sweet spot in your
mind? In mine, it's at sample size 4, which ensures there is no up-scaling, but
I could be talked into 8, which would be 390 x 520. There are some image sizes
that are less-borderline where using a too-high sample size is going to result
in upscaling, but that's a tradeoff we should consider.
I was only suggesting using Android views in place of ensureMinSize() and
cropToSquare(). Using the Android View to crop/up-scale images that start out
too small vs using a Bitmap crop/up-scale in this class doesn't affect the size
of the image being decoded since the Bitmap scaling/cropping happens after we've
decoded the image. I would expect that Bitmap operations are more costly than
letting the Android View system layout the image (I don't have any data to back
that up, so that assumption could be wrong), but I can see an argument for
minimizing the number of bits being sent back in the response for the bitmap. 
 

[Finnur, 2017/04/19 16:01:35]

The sweet spot in my mind is the highest inSampleSize number we can use without
noticing much degradation. But that's subjective, I guess. I'm OK with trying it
this way (effectively changing the param from maxSize to minSize, so as to end
with an inSampleSize of 4 in the case above). We can always revisit it, if
memory constraints or speed of decoding are a problem.

However, I'm having a problem getting the Android View system to work the way I
want it. There's two problems.

1) If the image is _smaller_ than minSize, without ensureMinSize the View system
does not seem to scale the image up to fill the available space provided (tried
various combinations of xml params for the ImageView). I need all images, large
and small to fully use up the square given.

2) If the image is _larger_ than the available space, the View system will
correctly only show the portions of the image that fit within the boundary
given. However, it doesn't actually crop the photo to fit, so the selection
scale animation (that is supposed to make the image shrink) will make parts of
the image (that were hidden) visible because the view system thinks -- hey,
we've got all these extra bits we aren't showing, now is our chance! :)
cropToSquare eliminates this by making sure that the image is always rectangular
and fits exactly in the square given (and uses less memory while it lives on
screen).

[Theresa, 2017/04/24 16:57:38]

That's a bit surprising since CENTER_CROP's documentation says "Scale the image
uniformly (maintain the image's aspect ratio) so that both dimensions (width and
height) of the image will be equal to or larger than the corresponding dimension
of the view (minus padding)."

But if you can't get it to work then scaling here is okay.
Thank you for the explanation, that makes sense.

+                inSampleSize *= 2;
+            }
+        }
+
+        return inSampleSize;
+    }
+
+    /**
+     * Scales a |bitmap| to a certain size.
+     * @param bitmap The bitmap to scale.
+     * @param scaleMaxSize What to scale it to.
+     * @param filter True if the source should be filtered.
+     * @return The resulting scaled bitmap.
+     */
+    public static Bitmap scale(Bitmap bitmap, float scaleMaxSize, boolean filter) {

[Theresa, 2017/04/13 02:25:26]

Where is this method called from?

[Finnur, 2017/04/18 17:21:14]

Hmm... it isn't (yet)... Removed.

+        float ratio = Math.min((float) scaleMaxSize / bitmap.getWidth(),
+                (float) scaleMaxSize / bitmap.getHeight());
+        int height = Math.round(ratio * bitmap.getHeight());
+        int width = Math.round(ratio * bitmap.getWidth());
+
+        return Bitmap.createScaledBitmap(bitmap, width, height, filter);
+    }
+
+    /**
+     * Ensures a |bitmap| is at least |size| in both width and height.
+     * @param bitmap The bitmap to modify.
+     * @param size The minimum size (width and height).
+     * @return The resulting (scaled) bitmap.
+     */
+    private static Bitmap ensureMinSize(Bitmap bitmap, int size) {
+        int width = bitmap.getWidth();
+        int height = bitmap.getHeight();
+        if (width < size) {
+            float scale = (float) size / width;
+            width = size;
+            height *= scale;
+        }
+
+        if (height < size) {
+            float scale = (float) size / height;
+            height = size;
+            width *= scale;
+        }
+
+        return Bitmap.createScaledBitmap(bitmap, width, height, true);
+    }
+
+    /**
+     * Crops a |bitmap| to a certain square |size|
+     * @param bitmap The bitmap to crop.
+     * @param size The size desired (width and height).
+     * @return The resulting (square) bitmap.
+     */
+    private static Bitmap cropToSquare(Bitmap bitmap, int size) {
+        int x = 0;
+        int y = 0;
+        int width = bitmap.getWidth();
+        int height = bitmap.getHeight();
+
+        if (width > size) x = (width - size) / 2;
+        if (height > size) y = (height - size) / 2;
+        return Bitmap.createBitmap(bitmap, x, y, size, size);
+    }
+}