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

BUG=656015

Review-Url: https://codereview.chromium.org/2816733002
Cr-Commit-Position: refs/heads/master@{#468209}

chrome/android/java/AndroidManifest.xml

@@ -632,6 +632,14 @@ by a child template that "extends" this file.
             android:exported="false">
         </service>
 
+        <!-- Service for decoding images in a sandboxed process. -->
+        <service
+            android:description="@string/decoder_description"
+            android:name="org.chromium.chrome.browser.photo_picker.DecoderService"
+            android:exported="false"
+            android:isolatedProcess="true"
+            android:process=":decoder_service" />
+
         <!-- Providers for chrome data. -->
         <provider android:name="org.chromium.chrome.browser.provider.ChromeBrowserProvider"
             android:authorities="{{ manifest_package }}.ChromeBrowserProvider;{{ manifest_package }}.browser;{{ manifest_package }}"

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

+// 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;
+
+/**
+ * A collection of utility functions for dealing with bitmaps.
+ */
+class BitmapUtils {
+    /**
+     * Takes a |bitmap| and returns a square thumbnail of |size|x|size| from the center of the
+     * bitmap specified.
+     * @param bitmap The bitmap to adjust.
+     * @param size The desired size (width and height).
+     * @return The new bitmap thumbnail.
+     */
+    private static Bitmap sizeBitmap(Bitmap bitmap, int size) {
+        // TODO(finnur): Investigate options that require fewer bitmaps to be created.
+        bitmap = ensureMinSize(bitmap, size);
+        bitmap = cropToSquare(bitmap, size);
+        return bitmap;
+    }
+
+    /**
+     * Given a FileDescriptor, decodes the contents and returns a bitmap of
+     * dimensions |size|x|size|.
+     * @param descriptor The FileDescriptor for the file to read.
+     * @param size The width and height of the bitmap to return.
+     * @return The resulting bitmap.
+     */
+    public static Bitmap decodeBitmapFromFileDescriptor(FileDescriptor descriptor, int size) {
+        BitmapFactory.Options options = new BitmapFactory.Options();
+        options.inJustDecodeBounds = true;
+        BitmapFactory.decodeFileDescriptor(descriptor, null, options);
+        options.inSampleSize = calculateInSampleSize(options.outWidth, options.outHeight, size);
+        options.inJustDecodeBounds = false;
+        Bitmap bitmap = BitmapFactory.decodeFileDescriptor(descriptor, null, options);
+
+        if (bitmap == null) return null;
+
+        return sizeBitmap(bitmap, size);
+    }
+
+    /**
+     * Calculates the sub-sampling factor {@link BitmapFactory#inSampleSize} option for a given
+     * image dimensions, which will be used to create a bitmap of a pre-determined size (as small as
+     * possible without either dimension shrinking below |minSize|.
+     * @param width The calculated width of the image to decode.
+     * @param height The calculated height of the image to decode.
+     * @param minSize The maximum size the image should be (in either dimension).
+     * @return The sub-sampling factor (power of two: 1 = no change, 2 = half-size, etc).
+     */
+    private static int calculateInSampleSize(int width, int height, int minSize) {
+        int inSampleSize = 1;
+        if (width > minSize && height > minSize) {
+            inSampleSize = Math.min(width, height) / minSize;
+        }
+        return inSampleSize;
+    }
+
+    /**
+     * 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 && height >= size) return bitmap;
+
+        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 && height == size) return bitmap;
+
+        if (width > size) x = (width - size) / 2;
+        if (height > size) y = (height - size) / 2;
+        return Bitmap.createBitmap(bitmap, x, y, size, size);
+    }
+}

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

+// 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.app.Service;
+import android.content.Intent;
+import android.graphics.Bitmap;
+import android.os.Bundle;
+import android.os.Handler;
+import android.os.IBinder;
+import android.os.Message;
+import android.os.Messenger;
+import android.os.ParcelFileDescriptor;
+import android.os.RemoteException;
+
+import org.chromium.base.Log;
+
+import java.io.FileDescriptor;
+import java.io.IOException;
+
+/**
+ * A service to accept requests to take image file contents and decode them.
+ */
+public class DecoderService extends Service {
+    // Message ids for communicating with the client.
+
+    // A message sent by the client to decode an image.
+    static final int MSG_DECODE_IMAGE = 1;
+    // A message sent by the server to notify the client of the results of the decoding.
+    static final int MSG_IMAGE_DECODED_REPLY = 2;
+
+    // The keys for the bundle when passing data to and from this service.
+    static final String KEY_FILE_DESCRIPTOR = "file_descriptor";
+    static final String KEY_FILE_PATH = "file_path";
+    static final String KEY_IMAGE_BITMAP = "image_bitmap";
+    static final String KEY_IMAGE_BYTE_COUNT = "image_byte_count";
+    static final String KEY_IMAGE_DESCRIPTOR = "image_descriptor";
+    static final String KEY_SIZE = "size";
+    static final String KEY_SUCCESS = "success";
+
+    // A tag for logging error messages.
+    private static final String TAG = "ImageDecoder";
+
+    /**
+     * Handler for incoming messages from clients.
+     */
+    static class IncomingHandler extends Handler {
+        @Override
+        public void handleMessage(Message msg) {
+            switch (msg.what) {
+                case MSG_DECODE_IMAGE:
+                    Bundle bundle = null;
+                    Messenger client = null;
+                    String filePath = "";
+                    int size = 0;
+                    try {
+                        Bundle payload = msg.getData();
+                        client = msg.replyTo;
+
+                        filePath = payload.getString(KEY_FILE_PATH);
+                        ParcelFileDescriptor pfd = payload.getParcelable(KEY_FILE_DESCRIPTOR);
+                        size = payload.getInt(KEY_SIZE);
+
+                        // Setup a minimum viable response to parent process. Will be fleshed out
+                        // further below.
+                        bundle = new Bundle();
+                        bundle.putString(KEY_FILE_PATH, filePath);
+                        bundle.putBoolean(KEY_SUCCESS, false);
+
+                        FileDescriptor fd = pfd.getFileDescriptor();
+                        Bitmap bitmap = BitmapUtils.decodeBitmapFromFileDescriptor(fd, size);
+                        try {
+                            pfd.close();
+                        } catch (IOException e) {
+                            Log.e(TAG, "Closing failed " + filePath + " (size: " + size + ") " + e);
+                        }
+
+                        if (bitmap == null) {
+                            Log.e(TAG, "Decode failed " + filePath + " (size: " + size + ")");
+                            sendReply(client, bundle); // Sends SUCCESS == false;
+                            return;
+                        }
+
+                        // The most widely supported, easiest, and reasonably efficient method is to
+                        // decode to an immutable bitmap and just return the bitmap over binder. It
+                        // will internally memcpy itself to ashmem and then just send over the file
+                        // descriptor. In the receiving process it will just leave the bitmap on
+                        // ashmem since it's immutable and carry on.
+                        bundle.putParcelable(KEY_IMAGE_BITMAP, bitmap);
+                        bundle.putBoolean(KEY_SUCCESS, true);
+                        sendReply(client, bundle);
+                        bitmap.recycle();
+                    } catch (Exception e) {
+                        // This service has no UI and maintains no state so if it crashes on
+                        // decoding a photo, it is better UX to eat the exception instead of showing
+                        // a crash dialog and discarding other requests that have already been sent.
+                        Log.e(TAG,
+                                "Unexpected error during decoding " + filePath + " (size: " + size
+                                        + ") " + e);
+
+                        if (bundle != null && client != null) sendReply(client, bundle);
+                    }
+                    break;
+                default:
+                    super.handleMessage(msg);
+            }
+        }
+
+        private void sendReply(Messenger client, Bundle bundle) {
+            Message reply = Message.obtain(null, MSG_IMAGE_DECODED_REPLY);
+            reply.setData(bundle);
+            try {
+                client.send(reply);
+            } catch (RemoteException remoteException) {
+                Log.e(TAG, "Remote error while replying: " + remoteException);
+            }
+        }
+    }
+
+    /**
+     * The target we publish for clients to send messages to IncomingHandler.
+     */
+    final Messenger mMessenger = new Messenger(new IncomingHandler());
+
+    /**
+     * When binding to the service, we return an interface to our messenger
+     * for sending messages to the service.
+     */
+    @Override
+    public IBinder onBind(Intent intent) {
+        return mMessenger.getBinder();
+    }
+}

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

@@ -10,6 +10,7 @@ import android.webkit.MimeTypeMap;
 import java.io.File;
 import java.io.FileFilter;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Locale;
 
 /**
@@ -23,13 +24,12 @@ class MimeTypeFileFilter implements FileFilter {
 
     /**
      * Contructs a MimeTypeFileFilter object.
-     * @param acceptAttr A comma seperated list of MIME types this filter accepts.
-     *                   For example: images/gif, video/*.
+     * @param mimeTypes A list of MIME types this filter accepts.
+     *                  For example: images/gif, video/*.
      */
-    // TODO(finnur): Convert param to List.
-    public MimeTypeFileFilter(@NonNull String acceptAttr) {
-        for (String field : acceptAttr.toLowerCase(Locale.US).split(",")) {
-            field = field.trim();
+    public MimeTypeFileFilter(@NonNull List<String> mimeTypes) {
+        for (String field : mimeTypes) {
+            field = field.trim().toLowerCase(Locale.US);
             if (field.startsWith(".")) {
                 mExtensions.add(field.substring(1));
             } else if (field.endsWith("/*")) {

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

@@ -189,8 +189,13 @@ public class PickerBitmapView extends SelectableItemView<PickerBitmap> {
 
         mBitmapDetails = bitmapDetails;
         setItem(bitmapDetails);
-        setThumbnailBitmap(thumbnail);
-        mImageLoaded = !placeholder;
+        if (isCameraTile() || isGalleryTile()) {
+            initializeSpecialTile(mBitmapDetails);
+            mImageLoaded = true;
+        } else {
+            setThumbnailBitmap(thumbnail);
+            mImageLoaded = !placeholder;
+        }
 
         updateSelectionState();
     }
@@ -217,8 +222,6 @@ public class PickerBitmapView extends SelectableItemView<PickerBitmap> {
                 mSpecialTile, null, image, null, null);
         mSpecialTile.setText(labelStringId);
 
-        initialize(bitmapDetails, null, false);
-
         // Reset visibility, since #initialize() sets mSpecialTile visibility to GONE.
         mSpecialTile.setVisibility(View.VISIBLE);
     }
@@ -266,6 +269,7 @@ public class PickerBitmapView extends SelectableItemView<PickerBitmap> {
      * re-used.
      */
     private void resetTile() {
+        mIconView.setImageBitmap(null);
         mUnselectedView.setVisibility(View.GONE);
         mSelectedView.setVisibility(View.GONE);
         mScrim.setVisibility(View.GONE);
@@ -323,15 +327,14 @@ public class PickerBitmapView extends SelectableItemView<PickerBitmap> {
     }
 
     private boolean isGalleryTile() {
-        // TODO(finnur): Remove the null checks here and below.
-        return mBitmapDetails != null && mBitmapDetails.type() == PickerBitmap.GALLERY;
+        return mBitmapDetails.type() == PickerBitmap.GALLERY;
     }
 
     private boolean isCameraTile() {
-        return mBitmapDetails != null && mBitmapDetails.type() == PickerBitmap.CAMERA;
+        return mBitmapDetails.type() == PickerBitmap.CAMERA;
     }
 
     private boolean isPictureTile() {
-        return mBitmapDetails == null || mBitmapDetails.type() == PickerBitmap.PICTURE;
+        return mBitmapDetails.type() == PickerBitmap.PICTURE;
     }
 }

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

@@ -5,9 +5,6 @@
 package org.chromium.chrome.browser.photo_picker;
 
 import android.graphics.Bitmap;
-import android.graphics.Canvas;
-import android.graphics.Color;
-import android.graphics.Paint;
 import android.support.v7.widget.RecyclerView.ViewHolder;
 import android.text.TextUtils;
 
@@ -16,7 +13,8 @@ import java.util.List;
 /**
  * Holds on to a {@link PickerBitmapView} that displays information about a picker bitmap.
  */
-public class PickerBitmapViewHolder extends ViewHolder {
+public class PickerBitmapViewHolder
+        extends ViewHolder implements DecoderServiceHost.ImageDecodedCallback {
     // Our parent category.
     private PickerCategoryView mCategoryView;
 
@@ -35,11 +33,9 @@ public class PickerBitmapViewHolder extends ViewHolder {
         mItemView = itemView;
     }
 
-    /**
-     * The notification handler for when an image has been decoded.
-     * @param filePath The file path for the newly decoded image.
-     * @param bitmap The results of the decoding (or placeholder image, if failed).
-     */
+    // DecoderServiceHost.ImageDecodedCallback
+
+    @Override
     public void imageDecodedCallback(String filePath, Bitmap bitmap) {
         if (bitmap == null || bitmap.getWidth() == 0 || bitmap.getHeight() == 0) {
             return;
@@ -65,31 +61,16 @@ public class PickerBitmapViewHolder extends ViewHolder {
 
         if (mBitmapDetails.type() == PickerBitmap.CAMERA
                 || mBitmapDetails.type() == PickerBitmap.GALLERY) {
-            mItemView.initializeSpecialTile(mBitmapDetails);
+            mItemView.initialize(mBitmapDetails, null, false);
             return;
         }
 
         // TODO(finnur): Use cached image, if available.
 
-        // TODO(finnur): Use decoder instead.
-        int size = mCategoryView.getImageSize();
-        imageDecodedCallback(mBitmapDetails.getFilePath(), createPlaceholderBitmap(size, size));
-    }
+        mItemView.initialize(mBitmapDetails, null, true);
 
-    /**
-     * Creates a placeholder bitmap.
-     * @param width The requested width of the resulting bitmap.
-     * @param height The requested height of the resulting bitmap.
-     * @return Placeholder bitmap.
-     */
-    // TODO(finnur): Remove once the decoder is in place.
-    private Bitmap createPlaceholderBitmap(int width, int height) {
-        Bitmap placeholder = Bitmap.createBitmap(width, height, Bitmap.Config.ARGB_8888);
-        Canvas canvas = new Canvas(placeholder);
-        Paint paint = new Paint();
-        paint.setColor(Color.GRAY);
-        canvas.drawRect(0, 0, (float) width, (float) height, paint);
-        return placeholder;
+        int size = mCategoryView.getImageSize();
+        mCategoryView.getDecoderServiceHost().decodeImage(mBitmapDetails.getFilePath(), size, this);
     }
 
     /**

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

@@ -20,6 +20,7 @@ import org.chromium.chrome.browser.widget.selection.SelectableListLayout;
 import org.chromium.chrome.browser.widget.selection.SelectionDelegate;
 import org.chromium.ui.PhotoPickerListener;
 
+import java.util.Arrays;
 import java.util.List;
 
 /**
@@ -27,7 +28,8 @@ import java.util.List;
  * the photo picker, for example the RecyclerView and the bitmap caches.
  */
 public class PickerCategoryView extends RelativeLayout
-        implements FileEnumWorkerTask.FilesEnumeratedCallback, OnMenuItemClickListener {
+        implements FileEnumWorkerTask.FilesEnumeratedCallback, RecyclerView.RecyclerListener,
+                   DecoderServiceHost.ServiceReadyCallback, OnMenuItemClickListener {
     // The dialog that owns us.
     private PhotoPickerDialog mDialog;
 
@@ -46,6 +48,9 @@ public class PickerCategoryView extends RelativeLayout
     // The callback to notify the listener of decisions reached in the picker.
     private PhotoPickerListener mListener;
 
+    // The host class for the decoding service.
+    private DecoderServiceHost mDecoderServiceHost;
+
     // The RecyclerView showing the images.
     private RecyclerView mRecyclerView;
 
@@ -71,6 +76,9 @@ public class PickerCategoryView extends RelativeLayout
     // A worker task for asynchronously enumerating files off the main thread.
     private FileEnumWorkerTask mWorkerTask;
 
+    // Whether the connection to the service has been established.
+    private boolean mServiceReady;
+
     public PickerCategoryView(Context context) {
         super(context);
         postConstruction(context);
@@ -84,6 +92,11 @@ public class PickerCategoryView extends RelativeLayout
     private void postConstruction(Context context) {
         mContext = context;
 
+        mDecoderServiceHost = new DecoderServiceHost(this);
+        mDecoderServiceHost.bind(mContext);
+
+        enumerateBitmaps();
+
         mSelectionDelegate = new SelectionDelegate<PickerBitmap>();
 
         View root = LayoutInflater.from(context).inflate(R.layout.photo_picker_dialog, this);
@@ -107,18 +120,21 @@ public class PickerCategoryView extends RelativeLayout
         mRecyclerView.addItemDecoration(new GridSpacingItemDecoration(mColumns, mPadding));
 
         // TODO(finnur): Implement caching.
-        // TODO(finnur): Remove this once the decoder service is in place.
-        prepareBitmaps();
     }
 
     /**
-     * Cancels any outstanding requests.
+     * Severs the connection to the decoding utility process and cancels any outstanding requests.
      */
     public void onDialogDismissed() {
         if (mWorkerTask != null) {
             mWorkerTask.cancel(true);
             mWorkerTask = null;
         }
+
+        if (mDecoderServiceHost != null) {
+            mDecoderServiceHost.unbind(mContext);
+            mDecoderServiceHost = null;
+        }
     }
 
     /**
@@ -141,8 +157,25 @@ public class PickerCategoryView extends RelativeLayout
     @Override
     public void filesEnumeratedCallback(List<PickerBitmap> files) {
         mPickerBitmaps = files;
-        if (files != null && files.size() > 0) {
-            mPickerAdapter.notifyDataSetChanged();
+        processBitmaps();
+    }
+
+    // DecoderServiceHost.ServiceReadyCallback:
+
+    @Override
+    public void serviceReady() {
+        mServiceReady = true;
+        processBitmaps();
+    }
+
+    // RecyclerView.RecyclerListener:
+
+    @Override
+    public void onViewRecycled(RecyclerView.ViewHolder holder) {
+        PickerBitmapViewHolder bitmapHolder = (PickerBitmapViewHolder) holder;
+        String filePath = bitmapHolder.getFilePath();
+        if (filePath != null) {
+            getDecoderServiceHost().cancelDecodeImage(filePath);
         }
     }
 
@@ -162,6 +195,16 @@ public class PickerCategoryView extends RelativeLayout
         return false;
     }
 
+    /**
+     * Start loading of bitmaps, once files have been enumerated and service is
+     * ready to decode.
+     */
+    private void processBitmaps() {
+        if (mServiceReady && mPickerBitmaps != null) {
+            mPickerAdapter.notifyDataSetChanged();
+        }
+    }
+
     // Simple accessors:
 
     public int getImageSize() {
@@ -176,6 +219,10 @@ public class PickerCategoryView extends RelativeLayout
         return mPickerBitmaps;
     }
 
+    public DecoderServiceHost getDecoderServiceHost() {
+        return mDecoderServiceHost;
+    }
+
     public boolean isMultiSelectAllowed() {
         return mMultiSelectionAllowed;
     }
@@ -212,14 +259,15 @@ public class PickerCategoryView extends RelativeLayout
     }
 
     /**
-     * Prepares bitmaps for loading.
+     * Asynchronously enumerates bitmaps on disk.
      */
-    private void prepareBitmaps() {
+    private void enumerateBitmaps() {
         if (mWorkerTask != null) {
             mWorkerTask.cancel(true);
         }
 
-        mWorkerTask = new FileEnumWorkerTask(this, new MimeTypeFileFilter("image/*"));
+        mWorkerTask =
+                new FileEnumWorkerTask(this, new MimeTypeFileFilter(Arrays.asList("image/*")));
         mWorkerTask.execute();
     }
 

chrome/android/java/strings/android_chrome_strings.grd

@@ -2816,6 +2816,11 @@ You must have Bluetooth and Location turned on in order to use the Physical Web.
         <ph name="BEGIN_LINK">&lt;link&gt;</ph>Get help<ph name="END_LINK">&lt;/link&gt;</ph>
       </message>
 
+      <!-- Photo Picker strings -->
+      <message name="IDS_DECODER_DESCRIPTION" desc="The title for the image decoder utility service.">
+        Image decoder
+      </message>
+
       <!-- Migration strings -->
       <message name="IDS_TAB_SWITCHER_CALLOUT_BODY" desc="Indicates that clicking the tab switcher button gives you quick access to your tabs.">
         Tap this button for quick access to your tabs.

chrome/android/java_sources.gni

@@ -784,6 +784,9 @@ chrome_java_sources = [
   "java/src/org/chromium/chrome/browser/permissions/PermissionDialogController.java",
   "java/src/org/chromium/chrome/browser/permissions/PermissionDialogDelegate.java",
   "java/src/org/chromium/chrome/browser/physicalweb/BitmapHttpRequest.java",
+  "java/src/org/chromium/chrome/browser/photo_picker/BitmapUtils.java",
+  "java/src/org/chromium/chrome/browser/photo_picker/DecoderService.java",
+  "java/src/org/chromium/chrome/browser/photo_picker/DecoderServiceHost.java",
   "java/src/org/chromium/chrome/browser/photo_picker/FileEnumWorkerTask.java",
   "java/src/org/chromium/chrome/browser/photo_picker/MimeTypeFileFilter.java",
   "java/src/org/chromium/chrome/browser/photo_picker/PhotoPickerDialog.java",