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

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

Index: chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderService.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderService.java b/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderService.java
index 9f307683bb5558327b1ad5405d2d2108b72c895b..77c06c198c45fdb7d417b3f2d6171dd4ab1889a7 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderService.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderService.java
@@ -8,10 +8,7 @@ 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 android.os.SystemClock;
@@ -25,19 +22,10 @@ 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";
     static final String KEY_DECODE_TIME = "decode_time";
@@ -45,98 +33,78 @@ public class DecoderService extends Service {
     // 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();
-
-                        long begin = SystemClock.elapsedRealtime();
-                        Bitmap bitmap = BitmapUtils.decodeBitmapFromFileDescriptor(fd, size);
-                        long decodeTime = SystemClock.elapsedRealtime() - begin;
-
-                        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);
-                        bundle.putLong(KEY_DECODE_TIME, decodeTime);
-                        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);
+    @Override
+    public void onCreate() {
+        super.onCreate();
+    }
+
+    @Override
+    public IBinder onBind(Intent intent) {
+        return mBinder;
+    }
+
+    private final IDecoderService.Stub mBinder = new IDecoderService.Stub() {
+        public void decodeImage(Bundle payload, IDecoderServiceCallback callback) {
+            Bundle bundle = null;
+            String filePath = "";
+            int size = 0;
+            try {
+                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();
+
+                long begin = SystemClock.elapsedRealtime();
+                Bitmap bitmap = BitmapUtils.decodeBitmapFromFileDescriptor(fd, size);
+                long decodeTime = SystemClock.elapsedRealtime() - begin;
+
+                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(callback, 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);
+                bundle.putLong(KEY_DECODE_TIME, decodeTime);
+                sendReply(callback, 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) sendReply(callback, bundle);
             }
         }
 
-        private void sendReply(Messenger client, Bundle bundle) {
-            Message reply = Message.obtain(null, MSG_IMAGE_DECODED_REPLY);
-            reply.setData(bundle);
+        private void sendReply(IDecoderServiceCallback callback, Bundle bundle) {
             try {
-                client.send(reply);
+                callback.onDecodeImageDone(bundle);
             } 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();
-    }
+    };
 }