Use handler with messenger instead of Binder for communication between service and activity

OpenPGP-Keychain/AndroidManifest.xml

@@ -500,11 +500,6 @@
             <intent-filter>
                 <action android:name="org.openintents.crypto.ICryptoService" />
             </intent-filter>
-            <intent-filter>
-
-                <!-- Can only be used from OpenPGP Keychain (internal): -->
-                <action android:name="org.sufficientlysecure.keychain.crypto_provider.IServiceActivityCallback" />
-            </intent-filter>
 
             <meta-data
                 android:name="api_version"

OpenPGP-Keychain/src/org/sufficientlysecure/keychain/remote_api/CryptoService.java

@@ -49,22 +49,22 @@ import android.database.Cursor;
 import android.net.Uri;
 import android.os.Binder;
 import android.os.Bundle;
+import android.os.Handler;
 import android.os.IBinder;
+import android.os.Message;
+import android.os.Messenger;
 import android.os.RemoteException;
 
 public class CryptoService extends Service {
     Context mContext;
 
-    // just one pool of 4 threads, pause on every user action needed
+    final ArrayBlockingQueue<Runnable> mPoolQueue = new ArrayBlockingQueue<Runnable>(100);
-    final ArrayBlockingQueue<Runnable> mPoolQueue = new ArrayBlockingQueue<Runnable>(20);
     // TODO: Are these parameters okay?
     PausableThreadPoolExecutor mThreadPool = new PausableThreadPoolExecutor(2, 4, 10,
             TimeUnit.SECONDS, mPoolQueue);
 
     final Object userInputLock = new Object();
 
-    public static final String ACTION_SERVICE_ACTIVITY = "org.sufficientlysecure.keychain.crypto_provider.IServiceActivityCallback";
-
     @Override
     public void onCreate() {
         super.onCreate();
@@ -80,20 +80,8 @@ public class CryptoService extends Service {
 
     @Override
     public IBinder onBind(Intent intent) {
-        // return different binder for connections from internal service activity
-        if (ACTION_SERVICE_ACTIVITY.equals(intent.getAction())) {
-
-            // this binder can only be used from OpenPGP Keychain
-            if (isCallerAllowed(true)) {
-                return mBinderServiceActivity;
-            } else {
-                Log.e(Constants.TAG, "This binder can only be used from " + Constants.PACKAGE_NAME);
-                return null;
-            }
-        } else {
         return mBinder;
     }
-    }
 
     private String getCachedPassphrase(long keyId) {
         String passphrase = PassphraseCacheService.getCachedPassphrase(mContext, keyId);
@@ -104,15 +92,56 @@ public class CryptoService extends Service {
             // start passphrase dialog
             Bundle extras = new Bundle();
             extras.putLong(CryptoServiceActivity.EXTRA_SECRET_KEY_ID, keyId);
-            pauseQueueAndStartServiceActivity(CryptoServiceActivity.ACTION_CACHE_PASSPHRASE, extras);
+
+            PassphraseActivityCallback callback = new PassphraseActivityCallback();
+            Messenger messenger = new Messenger(new Handler(getMainLooper(), callback));
+
+            pauseQueueAndStartServiceActivity(CryptoServiceActivity.ACTION_CACHE_PASSPHRASE,
+                    messenger, extras);
+
+            if (callback.isSuccess()) {
+                Log.d(Constants.TAG, "New passphrase entered!");
 
                 // get again after it was entered
                 passphrase = PassphraseCacheService.getCachedPassphrase(mContext, keyId);
+            } else {
+                Log.d(Constants.TAG, "Passphrase dialog canceled!");
+
+                // TODO: stop thread?!
+            }
+
         }
 
         return passphrase;
     }
 
+    public class PassphraseActivityCallback implements Handler.Callback {
+        public static final int SUCCESS = 1;
+        public static final int NO_SUCCESS = 0;
+
+        private boolean success;
+
+        public boolean isSuccess() {
+            return success;
+        }
+
+        @Override
+        public boolean handleMessage(Message msg) {
+            if (msg.arg1 == SUCCESS) {
+                success = true;
+            } else {
+                success = false;
+            }
+
+            // resume
+            synchronized (userInputLock) {
+                userInputLock.notifyAll();
+            }
+            mThreadPool.resume();
+            return true;
+        }
+    };
+
     /**
      * Search database for key ids based on emails.
      * 
@@ -143,21 +172,66 @@ public class CryptoService extends Service {
         // also encrypt to our self (so that we can decrypt it later!)
         keyIds.add(ownKeyId);
 
-        // convert o long[]
+        // convert to long[]
         long[] keyIdsArray = new long[keyIds.size()];
         for (int i = 0; i < keyIdsArray.length; i++) {
             keyIdsArray[i] = keyIds.get(i);
         }
 
         if (missingUserIds || manySameUserIds) {
+            SelectPubKeysActivityCallback callback = new SelectPubKeysActivityCallback();
+            Messenger messenger = new Messenger(new Handler(getMainLooper(), callback));
+
             Bundle extras = new Bundle();
             extras.putLongArray(CryptoServiceActivity.EXTRA_SELECTED_MASTER_KEY_IDS, keyIdsArray);
-            pauseQueueAndStartServiceActivity(CryptoServiceActivity.ACTION_SELECT_PUB_KEYS, extras);
+            pauseQueueAndStartServiceActivity(CryptoServiceActivity.ACTION_SELECT_PUB_KEYS,
+                    messenger, extras);
+
+            if (callback.isNewSelection()) {
+                Log.d(Constants.TAG, "New selection of pub keys!");
+                keyIdsArray = callback.getPubKeyIds();
+            } else {
+                Log.d(Constants.TAG, "Pub key selection canceled!");
+            }
         }
 
         return keyIdsArray;
     }
 
+    public class SelectPubKeysActivityCallback implements Handler.Callback {
+        public static final int OKAY = 1;
+        public static final int CANCEL = 0;
+        public static final String PUB_KEY_IDS = "pub_key_ids";
+
+        private boolean newSelection;
+        private long[] pubKeyIds;
+
+        public boolean isNewSelection() {
+            return newSelection;
+        }
+
+        public long[] getPubKeyIds() {
+            return pubKeyIds;
+        }
+
+        @Override
+        public boolean handleMessage(Message msg) {
+            if (msg.arg1 == OKAY) {
+                newSelection = true;
+                pubKeyIds = msg.getData().getLongArray(PUB_KEY_IDS);
+            } else {
+                newSelection = false;
+            }
+
+            // resume
+            synchronized (userInputLock) {
+                userInputLock.notifyAll();
+            }
+            mThreadPool.resume();
+            return true;
+        }
+    };
+
     private synchronized void encryptAndSignSafe(byte[] inputBytes, String[] encryptionUserIds,
             ICryptoCallback callback, AppSettings appSettings, boolean sign) throws RemoteException {
         try {
@@ -390,55 +464,6 @@ public class CryptoService extends Service {
             checkAndEnqueue(r);
         }
 
-        // @Override
-        // public void setup(boolean asciiArmor, boolean newKeyring, String newKeyringUserId)
-        // throws RemoteException {
-        //
-        //
-        // }
-
-    };
-
-    private final IServiceActivityCallback.Stub mBinderServiceActivity = new IServiceActivityCallback.Stub() {
-
-        @Override
-        public void onRegistered(boolean success, String packageName) throws RemoteException {
-            Log.d(Constants.TAG, "current therad id: " + Thread.currentThread().getId());
-
-            if (success) {
-                // resume threads
-                if (isPackageAllowed(packageName, false)) {
-                    mThreadPool.resume();
-                } else {
-                    // TODO: should not happen?
-                    mThreadPool.shutdownNow();
-                }
-            } else {
-                mThreadPool.resume();
-                // TODO
-                // mPoolQueue.clear();
-                // mPoolQueue.re
-                // mThreadPool.
-            }
-
-        }
-
-        @Override
-        public void onCachedPassphrase(boolean success) throws RemoteException {
-            Log.d(Constants.TAG, "current therad id: " + Thread.currentThread().getId());
-            mThreadPool.resume();
-
-            synchronized (userInputLock) {
-                userInputLock.notifyAll();
-            }
-        }
-
-        @Override
-        public void onSelectedPublicKeys(long[] keyIds) throws RemoteException {
-            mThreadPool.resume();
-
-        }
-
     };
 
     private void checkAndEnqueue(Runnable r) {
@@ -454,14 +479,71 @@ public class CryptoService extends Service {
             Bundle extras = new Bundle();
             // TODO: currently simply uses first entry
             extras.putString(CryptoServiceActivity.EXTRA_PACKAGE_NAME, callingPackages[0]);
-            pauseQueueAndStartServiceActivity(CryptoServiceActivity.ACTION_REGISTER, extras);
 
+            RegisterActivityCallback callback = new RegisterActivityCallback();
+            Messenger messenger = new Messenger(new Handler(getMainLooper(), callback));
+
+            pauseQueueAndStartServiceActivity(CryptoServiceActivity.ACTION_REGISTER, messenger,
+                    extras);
+
+            if (callback.isAllowed()) {
                 mThreadPool.execute(r);
+            } else {
+                Log.d(Constants.TAG, "User disallowed app!");
+            }
 
             Log.d(Constants.TAG, "Enqueued runnable…");
         }
     }
 
+    public class RegisterActivityCallback implements Handler.Callback {
+        public static final int ALLOW = 1;
+        public static final int DISALLOW = 0;
+        public static final String PACKAGE_NAME = "package_name";
+
+        private boolean allowed;
+        private String packageName;
+
+        public boolean isAllowed() {
+            return allowed;
+        }
+
+        public String getPackageName() {
+            return packageName;
+        }
+
+        @Override
+        public boolean handleMessage(Message msg) {
+            Log.d(Constants.TAG, "msg what: " + msg.what);
+
+            if (msg.arg1 == ALLOW) {
+                allowed = true;
+                packageName = msg.getData().getString(PACKAGE_NAME);
+
+                // resume threads
+                if (isPackageAllowed(packageName, false)) {
+                    synchronized (userInputLock) {
+                        userInputLock.notifyAll();
+                    }
+                    mThreadPool.resume();
+                } else {
+                    // Should not happen!
+                    Log.e(Constants.TAG, "Should not happen! Emergency shutdown!");
+                    mThreadPool.shutdownNow();
+                }
+            } else {
+                allowed = false;
+
+                synchronized (userInputLock) {
+                    userInputLock.notifyAll();
+                }
+                mThreadPool.resume();
+            }
+            return false;
+        }
+
+    }
+
     /**
      * Checks if process that binds to this service (i.e. the package name corresponding to the
      * process) is in the list of allowed package names.
@@ -531,25 +613,27 @@ public class CryptoService extends Service {
         return false;
     }
 
-    private void pauseQueueAndStartServiceActivity(String action, Bundle extras) {
+    private void pauseQueueAndStartServiceActivity(String action, Messenger messenger, Bundle extras) {
+        synchronized (userInputLock) {
             mThreadPool.pause();
 
             Log.d(Constants.TAG, "starting activity...");
             Intent intent = new Intent(getBaseContext(), CryptoServiceActivity.class);
             intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
             intent.setAction(action);
-        if (extras != null) {
+
+            extras.putParcelable(CryptoServiceActivity.EXTRA_MESSENGER, messenger);
             intent.putExtras(extras);
-        }
+
             getApplication().startActivity(intent);
 
             // lock current thread for user input
-        synchronized (userInputLock) {
             try {
                 userInputLock.wait();
             } catch (InterruptedException e) {
                 Log.e(Constants.TAG, "CryptoService", e);
             }
         }
+
     }
 }

OpenPGP-Keychain/src/org/sufficientlysecure/keychain/remote_api/CryptoServiceActivity.java

@@ -27,13 +27,9 @@ import org.sufficientlysecure.keychain.ui.SelectPublicKeyFragment;
 import org.sufficientlysecure.keychain.ui.dialog.PassphraseDialogFragment;
 import org.sufficientlysecure.keychain.util.Log;
 
-import android.content.ComponentName;
-import android.content.Context;
 import android.content.Intent;
-import android.content.ServiceConnection;
 import android.os.Bundle;
 import android.os.Handler;
-import android.os.IBinder;
 import android.os.Message;
 import android.os.Messenger;
 import android.os.RemoteException;
@@ -50,84 +46,26 @@ public class CryptoServiceActivity extends SherlockFragmentActivity {
     public static final String ACTION_SELECT_PUB_KEYS = Constants.INTENT_PREFIX
             + "API_ACTIVITY_SELECT_PUB_KEYS";
 
+    public static final String EXTRA_MESSENGER = "messenger";
+
     public static final String EXTRA_SECRET_KEY_ID = "secretKeyId";
     public static final String EXTRA_PACKAGE_NAME = "packageName";
     public static final String EXTRA_SELECTED_MASTER_KEY_IDS = "masterKeyIds";
 
-    private IServiceActivityCallback mServiceCallback;
+    private Messenger mMessenger;
-    private boolean mServiceBound;
 
     // register view
-    AppSettingsFragment mSettingsFragment;
+    private AppSettingsFragment mSettingsFragment;
     // select pub key view
-    SelectPublicKeyFragment mSelectFragment;
+    private SelectPublicKeyFragment mSelectFragment;
-
-    private ServiceConnection mServiceActivityConnection = new ServiceConnection() {
-        public void onServiceConnected(ComponentName name, IBinder service) {
-            mServiceCallback = IServiceActivityCallback.Stub.asInterface(service);
-            Log.d(Constants.TAG, "connected to ICryptoServiceActivity");
-            mServiceBound = true;
-        }
-
-        public void onServiceDisconnected(ComponentName name) {
-            mServiceCallback = null;
-            Log.d(Constants.TAG, "disconnected from ICryptoServiceActivity");
-            mServiceBound = false;
-        }
-    };
-
-    /**
-     * If not already bound, bind!
-     * 
-     * @return
-     */
-    public boolean bindToService() {
-        if (mServiceCallback == null && !mServiceBound) { // if not already connected
-            try {
-                Log.d(Constants.TAG, "not bound yet");
-
-                Intent serviceIntent = new Intent();
-                serviceIntent
-                        .setAction("org.sufficientlysecure.keychain.crypto_provider.IServiceActivityCallback");
-                bindService(serviceIntent, mServiceActivityConnection, Context.BIND_AUTO_CREATE);
-
-                return true;
-            } catch (Exception e) {
-                Log.d(Constants.TAG, "Exception", e);
-                return false;
-            }
-        } else { // already connected
-            Log.d(Constants.TAG, "already bound... ");
-            return true;
-        }
-    }
-
-    public void unbindFromService() {
-        unbindService(mServiceActivityConnection);
-    }
 
     @Override
     protected void onCreate(Bundle savedInstanceState) {
         super.onCreate(savedInstanceState);
 
-        Log.d(Constants.TAG, "onCreate…");
-
-        // bind to our own crypto service
-        bindToService();
-
         handleActions(getIntent(), savedInstanceState);
     }
 
-    @Override
-    protected void onDestroy() {
-        super.onDestroy();
-
-        // unbind from our crypto service
-        if (mServiceActivityConnection != null) {
-            unbindFromService();
-        }
-    }
-
     protected void handleActions(Intent intent, Bundle savedInstanceState) {
         String action = intent.getAction();
         Bundle extras = intent.getExtras();
@@ -136,6 +74,8 @@ public class CryptoServiceActivity extends SherlockFragmentActivity {
             extras = new Bundle();
         }
 
+        mMessenger = extras.getParcelable(EXTRA_MESSENGER);
+
         /**
          * com.android.crypto actions
          */
@@ -158,10 +98,16 @@ public class CryptoServiceActivity extends SherlockFragmentActivity {
                                 ProviderHelper.insertApiApp(CryptoServiceActivity.this,
                                         mSettingsFragment.getAppSettings());
 
+                                Message msg = Message.obtain();
+                                msg.arg1 = CryptoService.RegisterActivityCallback.ALLOW;
+                                Bundle data = new Bundle();
+                                data.putString(CryptoService.RegisterActivityCallback.PACKAGE_NAME,
+                                        packageName);
+                                msg.setData(data);
                                 try {
-                                    mServiceCallback.onRegistered(true, packageName);
+                                    mMessenger.send(msg);
                                 } catch (RemoteException e) {
-                                    Log.e(Constants.TAG, "ServiceActivity", e);
+                                    Log.e(Constants.TAG, "CryptoServiceActivity", e);
                                 }
                                 finish();
                             }
@@ -171,10 +117,12 @@ public class CryptoServiceActivity extends SherlockFragmentActivity {
                         public void onClick(View v) {
                             // Disallow
 
+                            Message msg = Message.obtain();
+                            msg.arg1 = CryptoService.RegisterActivityCallback.DISALLOW;
                             try {
-                                mServiceCallback.onRegistered(false, packageName);
+                                mMessenger.send(msg);
                             } catch (RemoteException e) {
-                                Log.e(Constants.TAG, "ServiceActivity", e);
+                                Log.e(Constants.TAG, "CryptoServiceActivity", e);
                             }
                             finish();
                         }
@@ -201,11 +149,17 @@ public class CryptoServiceActivity extends SherlockFragmentActivity {
                         public void onClick(View v) {
                             // ok
 
+                            Message msg = Message.obtain();
+                            msg.arg1 = CryptoService.SelectPubKeysActivityCallback.OKAY;
+                            Bundle data = new Bundle();
+                            data.putLongArray(
+                                    CryptoService.SelectPubKeysActivityCallback.PUB_KEY_IDS,
+                                    mSelectFragment.getSelectedMasterKeyIds());
+                            msg.setData(data);
                             try {
-                                mServiceCallback.onSelectedPublicKeys(mSelectFragment
+                                mMessenger.send(msg);
-                                        .getSelectedMasterKeyIds());
                             } catch (RemoteException e) {
-                                Log.e(Constants.TAG, "ServiceActivity", e);
+                                Log.e(Constants.TAG, "CryptoServiceActivity", e);
                             }
                             finish();
                         }
@@ -214,12 +168,13 @@ public class CryptoServiceActivity extends SherlockFragmentActivity {
                         public void onClick(View v) {
                             // cancel
 
-                            // TODO: currently does the same as OK...
+                            Message msg = Message.obtain();
+                            msg.arg1 = CryptoService.SelectPubKeysActivityCallback.CANCEL;
+                            ;
                             try {
-                                mServiceCallback.onSelectedPublicKeys(mSelectFragment
+                                mMessenger.send(msg);
-                                        .getSelectedMasterKeyIds());
                             } catch (RemoteException e) {
-                                Log.e(Constants.TAG, "ServiceActivity", e);
+                                Log.e(Constants.TAG, "CryptoServiceActivity", e);
                             }
                             finish();
                         }
@@ -227,6 +182,7 @@ public class CryptoServiceActivity extends SherlockFragmentActivity {
 
             setContentView(R.layout.api_app_select_pub_keys_activity);
 
+            /* Load select pub keys fragment */
             // Check that the activity is using the layout version with
             // the fragment_container FrameLayout
             if (findViewById(R.id.api_select_pub_keys_fragment_container) != null) {
@@ -263,16 +219,20 @@ public class CryptoServiceActivity extends SherlockFragmentActivity {
             @Override
             public void handleMessage(Message message) {
                 if (message.what == PassphraseDialogFragment.MESSAGE_OKAY) {
+                    Message msg = Message.obtain();
+                    msg.arg1 = CryptoService.PassphraseActivityCallback.SUCCESS;
                     try {
-                        mServiceCallback.onCachedPassphrase(true);
+                        mMessenger.send(msg);
                     } catch (RemoteException e) {
-                        Log.e(Constants.TAG, "ServiceActivity", e);
+                        Log.e(Constants.TAG, "CryptoServiceActivity", e);
                     }
                 } else {
+                    Message msg = Message.obtain();
+                    msg.arg1 = CryptoService.PassphraseActivityCallback.NO_SUCCESS;
                     try {
-                        mServiceCallback.onCachedPassphrase(false);
+                        mMessenger.send(msg);
                     } catch (RemoteException e) {
-                        Log.e(Constants.TAG, "ServiceActivity", e);
+                        Log.e(Constants.TAG, "CryptoServiceActivity", e);
                     }
                 }
                 finish();

OpenPGP-Keychain/src/org/sufficientlysecure/keychain/remote_api/IServiceActivityCallback.aidl

@@ -1,29 +0,0 @@
-/*
- * Copyright (C) 2013 Dominik Schürmann <dominik@dominikschuermann.de>
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- */
- 
-package org.sufficientlysecure.keychain.remote_api;
-
-
-interface IServiceActivityCallback {
-    
-    oneway void onRegistered(in boolean success, in String packageName);
-    
-    oneway void onCachedPassphrase(in boolean success);
-    
-    oneway void onSelectedPublicKeys(in long[] keyIds);
-
-}