Photo Picker dialog: Convert to AIDL for easier security review.

chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderServiceHost.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.content.ComponentName;
 import android.content.Context;
 import android.content.Intent;
 import android.content.ServiceConnection;
 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 android.os.StrictMode;
 import android.os.SystemClock;
 
 import org.chromium.base.Log;
+import org.chromium.base.ThreadUtils;
 import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.chrome.browser.util.ConversionUtils;
 
 import java.io.File;
 import java.io.FileDescriptor;
 import java.io.FileInputStream;
 import java.io.IOException;
-import java.lang.ref.WeakReference;
 import java.util.LinkedHashMap;
 import java.util.concurrent.TimeUnit;
 
 /**
- * A class to communicate with the {@link DecoderService}.
+ * A class to communicate with the {@link DecoderServiceImpl}.
  */
-public class DecoderServiceHost {
+public class DecoderServiceHost extends IDecoderServiceListener.Stub {
     // A tag for logging error messages.
     private static final String TAG = "ImageDecoderHost";
 
+    IDecoderService mIRemoteService;
+    private ServiceConnection mConnection = new ServiceConnection() {
+        public void onServiceConnected(ComponentName className, IBinder service) {
+            mIRemoteService = IDecoderService.Stub.asInterface(service);
+            mBound = true;
+            mCallback.serviceReady();
+        }
+
+        public void onServiceDisconnected(ComponentName className) {
+            Log.e(TAG, "Service has unexpectedly disconnected");

[Robert Sesek, 2017/06/20 02:09:47]

This will be called if you ever unbindService(), which should be done to clean
up the process when it's no longer needed.

[Finnur, 2017/06/20 12:20:57]

I do call unbindService but I do not see this function being called as a result.


The onServiceDisconnected in the Android sample...
https://developer.android.com/guide/components/aidl.html
... also has an equivalent Log line.

+            mIRemoteService = null;
+            mBound = false;
+        }
+    };
+
     /**
      * Interface for notifying clients of the service being ready.
      */
     public interface ServiceReadyCallback {
         /**
          * A function to define to receive a notification once the service is up and running.
          */
         void serviceReady();
     }
 
     /**
      * An interface notifying clients when an image has finished decoding.
      */
     public interface ImageDecodedCallback {
         /**
          * A function to define to receive a notification that 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).
          */
         void imageDecodedCallback(String filePath, Bitmap bitmap);
     }
 
     /**
-     * Class for interacting with the main interface of the service.
-     */
-    private class DecoderServiceConnection implements ServiceConnection {
-        // The callback to use to notify the service being ready.
-        private ServiceReadyCallback mCallback;
-
-        public DecoderServiceConnection(ServiceReadyCallback callback) {
-            mCallback = callback;
-        }
-
-        // Called when a connection to the service has been established.
-        public void onServiceConnected(ComponentName name, IBinder service) {
-            mService = new Messenger(service);
-            mBound = true;
-            mCallback.serviceReady();
-        }
-
-        // Called when a connection to the service has been lost.
-        public void onServiceDisconnected(ComponentName name) {
-            mBound = false;
-        }
-    }
-
-    /**
      * Class for keeping track of the data involved with each request.
      */
     private static class DecoderServiceParams {
         // The path to the file containing the bitmap to decode.
         public String mFilePath;
 
         // The requested size (width and height) of the bitmap, once decoded.
         public int mSize;
 
         // The callback to use to communicate the results of the decoding.
@@ skipping 11 matching lines @@
 
     // Map of file paths to decoder parameters in order of request.
     private LinkedHashMap<String, DecoderServiceParams> mRequests = new LinkedHashMap<>();
     LinkedHashMap<String, DecoderServiceParams> getRequests() {
         return mRequests;
     }
 
     // The callback used to notify the client when the service is ready.
     private ServiceReadyCallback mCallback;
 
-    // Messenger for communicating with the remote service.
-    Messenger mService = null;
-
-    // Our service connection to the {@link DecoderService}.
-    private DecoderServiceConnection mConnection;
-
     // Flag indicating whether we are bound to the service.
     boolean mBound;
 
-    // The inbound messenger used by the remote service to communicate with us.
-    final Messenger mMessenger = new Messenger(new IncomingHandler(this));
+    private Context mContext;
 
     /**
      * The DecoderServiceHost constructor.
      * @param callback The callback to use when communicating back to the client.
      */
-    public DecoderServiceHost(ServiceReadyCallback callback) {
+    public DecoderServiceHost(ServiceReadyCallback callback, Context context) {
         mCallback = callback;
+        mContext = context;
     }
 
     /**
-     * Initiate binding with the {@link DecoderService}.
+     * Initiate binding with the {@link DecoderServiceImpl}.
      * @param context The context to use.
      */
     public void bind(Context context) {
-        mConnection = new DecoderServiceConnection(mCallback);
-        Intent intent = new Intent(context, DecoderService.class);
-        context.bindService(intent, mConnection, Context.BIND_AUTO_CREATE);
+        Intent intent = new Intent(mContext, DecoderServiceImpl.class);
+        intent.setAction(IDecoderService.class.getName());
+        mContext.bindService(intent, mConnection, Context.BIND_AUTO_CREATE);
     }
 
     /**
-     * Unbind from the {@link DecoderService}.
+     * Unbind from the {@link DecoderServiceImpl}.
      * @param context The context to use.
      */
     public void unbind(Context context) {
         if (mBound) {
             context.unbindService(mConnection);
             mBound = false;
         }
     }
 
     /**
@@ skipping 13 matching lines @@
      * Dispatches the next image for decoding (from the queue).
      */
     private void dispatchNextDecodeImageRequest() {
         if (mRequests.entrySet().iterator().hasNext()) {
             DecoderServiceParams params = mRequests.entrySet().iterator().next().getValue();
             params.mTimestamp = SystemClock.elapsedRealtime();
             dispatchDecodeImageRequest(params.mFilePath, params.mSize);
         }
     }
 
+    @Override
+    public void onDecodeImageDone(final Bundle payload) {
+        ThreadUtils.runOnUiThread(new Runnable() {
+            @Override
+            public void run() {
+                // Read the reply back from the service.
+                String filePath = payload.getString(DecoderServiceImpl.KEY_FILE_PATH);
+                Boolean success = payload.getBoolean(DecoderServiceImpl.KEY_SUCCESS);
+                Bitmap bitmap = success
+                        ? (Bitmap) payload.getParcelable(DecoderServiceImpl.KEY_IMAGE_BITMAP)
+                        : null;
+                long decodeTime = payload.getLong(DecoderServiceImpl.KEY_DECODE_TIME);
+                closeRequest(filePath, bitmap, decodeTime);
+            }
+        });
+    }
+
     /**
      * Ties up all the loose ends from the decoding request (communicates the results of the
      * decoding process back to the client, and takes care of house-keeping chores regarding
      * the request queue).
      * @param filePath The path to the image that was just decoded.
      * @param bitmap The resulting decoded bitmap.
      * @param decodeTime The length of time it took to decode the bitmap.
      */
     public void closeRequest(String filePath, Bitmap bitmap, long decodeTime) {
         DecoderServiceParams params = getRequests().get(filePath);
@@ skipping 30 matching lines @@
         Bundle bundle = new Bundle();
 
         // The restricted utility process can't open the file to read the
         // contents, so we need to obtain a file descriptor to pass over.
         StrictMode.ThreadPolicy oldPolicy = StrictMode.allowThreadDiskReads();
         try {
             try {
                 inputFile = new FileInputStream(file);
                 FileDescriptor fd = inputFile.getFD();
                 pfd = ParcelFileDescriptor.dup(fd);
-                bundle.putParcelable(DecoderService.KEY_FILE_DESCRIPTOR, pfd);
+                bundle.putParcelable(DecoderServiceImpl.KEY_FILE_DESCRIPTOR, pfd);
             } catch (IOException e) {
                 Log.e(TAG, "Unable to obtain FileDescriptor: " + e);
                 closeRequest(filePath, null, -1);
             }
         } finally {
             try {
                 if (inputFile != null) inputFile.close();
             } catch (IOException e) {
                 Log.e(TAG, "Unable to close inputFile: " + e);
             }
             StrictMode.setThreadPolicy(oldPolicy);
         }
 
         if (pfd == null) return;
 
         // Prepare and send the data over.
-        Message payload = Message.obtain(null, DecoderService.MSG_DECODE_IMAGE);
-        payload.replyTo = mMessenger;
-        bundle.putString(DecoderService.KEY_FILE_PATH, filePath);
-        bundle.putInt(DecoderService.KEY_SIZE, size);
+        Message payload = Message.obtain();
+        bundle.putString(DecoderServiceImpl.KEY_FILE_PATH, filePath);
+        bundle.putInt(DecoderServiceImpl.KEY_SIZE, size);
         payload.setData(bundle);
         try {
-            mService.send(payload);
+            mIRemoteService.decodeImage(bundle, this);
             pfd.close();
         } catch (RemoteException e) {
             Log.e(TAG, "Communications failed (Remote): " + e);
             closeRequest(filePath, null, -1);
         } catch (IOException e) {
             Log.e(TAG, "Communications failed (IO): " + e);
             closeRequest(filePath, null, -1);
         }
     }
 
     /**
      * Cancels a request to decode an image (if it hasn't already been dispatched).
      * @param filePath The path to the image to cancel decoding.
      */
     public void cancelDecodeImage(String filePath) {
         mRequests.remove(filePath);
     }
-
-    /**
-     * A class for handling communications from the service to us.
-     */
-    static class IncomingHandler extends Handler {
-        // The DecoderServiceHost object to communicate with.
-        private final WeakReference<DecoderServiceHost> mHost;
-
-        /**
-         * Constructor for IncomingHandler.
-         * @param host The DecoderServiceHost object to communicate with.
-         */
-        IncomingHandler(DecoderServiceHost host) {
-            mHost = new WeakReference<DecoderServiceHost>(host);
-        }
-
-        @Override
-        public void handleMessage(Message msg) {
-            DecoderServiceHost host = mHost.get();
-            if (host == null) {
-                super.handleMessage(msg);
-                return;
-            }
-
-            switch (msg.what) {
-                case DecoderService.MSG_IMAGE_DECODED_REPLY:
-                    Bundle payload = msg.getData();
-
-                    // Read the reply back from the service.
-                    String filePath = payload.getString(DecoderService.KEY_FILE_PATH);
-                    Boolean success = payload.getBoolean(DecoderService.KEY_SUCCESS);
-                    Bitmap bitmap = success
-                            ? (Bitmap) payload.getParcelable(DecoderService.KEY_IMAGE_BITMAP)
-                            : null;
-                    long decodeTime = payload.getLong(DecoderService.KEY_DECODE_TIME);
-                    host.closeRequest(filePath, bitmap, decodeTime);
-                    break;
-                default:
-                    super.handleMessage(msg);
-            }
-        }
-    }
 }