decryptlist: some cleanup and streamlining of control flow

1 files changed, 84 insertions, 81 deletions

diff --git a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/DecryptListFragment.java b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/DecryptListFragment.java
index dbee564b1..737a5b3b6 100644
--- a/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/DecryptListFragment.java
+++ b/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/ui/DecryptListFragment.java
@@ -22,7 +22,6 @@ import java.io.File;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.HashMap;
-import java.util.Iterator;
 import java.util.List;
 
 import android.Manifest;
@@ -103,7 +102,7 @@ public class DecryptListFragment
     public static final String ARG_CAN_DELETE = "can_delete";
 
     private static final int REQUEST_CODE_OUTPUT = 0x00007007;
-    private static final int MY_PERMISSIONS_REQUEST_READ_EXTERNAL_STORAGE = 12;
+    private static final int REQUEST_PERMISSION_READ_EXTERNAL_STORAGE = 12;
 
     private ArrayList<Uri> mInputUris;
     private HashMap<Uri, InputDataResult> mInputDataResults;
@@ -215,12 +214,7 @@ public class DecryptListFragment
             mAdapter.add(uri);
 
             if (mCancelledInputUris.contains(uri)) {
-                mAdapter.setCancelled(uri, new OnClickListener() {
-                    @Override
-                    public void onClick(View v) {
-                        retryUri(uri);
-                    }
-                });
+                mAdapter.setCancelled(uri, true);
                 continue;
             }
 
@@ -362,12 +356,7 @@ public class DecryptListFragment
         mCurrentInputUri = null;
 
         mCancelledInputUris.add(uri);
-        mAdapter.setCancelled(uri, new OnClickListener() {
-            @Override
-            public void onClick(View v) {
-                retryUri(uri);
-            }
-        });
+        mAdapter.setCancelled(uri, true);
 
         cryptoOperation();
 
@@ -458,7 +447,7 @@ public class DecryptListFragment
         // un-cancel this one
         mCancelledInputUris.remove(uri);
         mPendingInputUris.add(uri);
-        mAdapter.setCancelled(uri, null);
+        mAdapter.setCancelled(uri, false);
 
         // check if there are any pending input uris
         cryptoOperation();
@@ -582,6 +571,11 @@ public class DecryptListFragment
     @Override
     public InputDataParcel createOperationInput() {
 
+        Activity activity = getActivity();
+        if (activity == null) {
+            return null;
+        }
+
         if (mCurrentInputUri == null) {
             if (mPendingInputUris.isEmpty()) {
                 // nothing left to do
@@ -593,95 +587,95 @@ public class DecryptListFragment
 
         Log.d(Constants.TAG, "mCurrentInputUri=" + mCurrentInputUri);
 
-        if (readPermissionGranted(mCurrentInputUri)) {
-            PgpDecryptVerifyInputParcel decryptInput = new PgpDecryptVerifyInputParcel()
-                    .setAllowSymmetricDecryption(true);
-            return new InputDataParcel(mCurrentInputUri, decryptInput);
-        } else {
+        if ( ! checkAndRequestReadPermission(activity, mCurrentInputUri)) {
             return null;
         }
+
+        PgpDecryptVerifyInputParcel decryptInput = new PgpDecryptVerifyInputParcel()
+                .setAllowSymmetricDecryption(true);
+        return new InputDataParcel(mCurrentInputUri, decryptInput);
+
     }
 
     /**
-     * Request READ_EXTERNAL_STORAGE permission on Android >= 6.0 to read content from "file" Uris
+     * Request READ_EXTERNAL_STORAGE permission on Android >= 6.0 to read content from "file" Uris.
+     *
+     * This method returns true on Android < 6, or if permission is already granted. It
+     * requests the permission and returns false otherwise, taking over responsibility
+     * for mCurrentInputUri.
      *
-     * see
-     * https://commonsware.com/blog/2015/10/07/runtime-permissions-files-action-send.html
+     * see https://commonsware.com/blog/2015/10/07/runtime-permissions-files-action-send.html
      */
-    private boolean readPermissionGranted(final Uri uri) {
+    private boolean checkAndRequestReadPermission(Activity activity, final Uri uri) {
+        if ( ! "file".equals(uri.getScheme())) {
+            return true;
+        }
+
         if (Build.VERSION.SDK_INT < VERSION_CODES.M) {
             return true;
         }
-        if (! "file".equals(uri.getScheme())) {
+
+        // Additional check due to https://commonsware.com/blog/2015/11/09/you-cannot-hold-nonexistent-permissions.html
+        if (Build.VERSION.SDK_INT < Build.VERSION_CODES.JELLY_BEAN) {
             return true;
         }
 
-        // Build check due to https://commonsware.com/blog/2015/11/09/you-cannot-hold-nonexistent-permissions.html
-        if (Build.VERSION.SDK_INT < Build.VERSION_CODES.JELLY_BEAN ||
-                ContextCompat.checkSelfPermission(getActivity(), Manifest.permission.READ_EXTERNAL_STORAGE)
+        if (ContextCompat.checkSelfPermission(activity, Manifest.permission.READ_EXTERNAL_STORAGE)
                         == PackageManager.PERMISSION_GRANTED) {
             return true;
-        } else {
-            requestPermissions(
-                    new String[]{Manifest.permission.READ_EXTERNAL_STORAGE},
-                    MY_PERMISSIONS_REQUEST_READ_EXTERNAL_STORAGE);
-
-            mCurrentInputUri = null;
-            mCancelledInputUris.add(uri);
-            mAdapter.setCancelled(uri, new OnClickListener() {
-                @Override
-                public void onClick(View v) {
-                    retryUri(uri);
-                }
-            });
-            return false;
         }
+
+        requestPermissions(
+                new String[] { Manifest.permission.READ_EXTERNAL_STORAGE },
+                REQUEST_PERMISSION_READ_EXTERNAL_STORAGE);
+
+        return false;
+
     }
 
     @Override
-    public void onRequestPermissionsResult(int requestCode, @NonNull String[] permissions,
-                                           @NonNull int[] grantResults) {
-        switch (requestCode) {
-            case MY_PERMISSIONS_REQUEST_READ_EXTERNAL_STORAGE: {
-                if (grantResults.length > 0
-                        && grantResults[0] == PackageManager.PERMISSION_GRANTED) {
-
-                    // permission granted -> retry all cancelled file uris!
-                    for (Iterator<Uri> iterator = mCancelledInputUris.iterator(); iterator.hasNext(); ) {
-                        Uri uri = iterator.next();
-
-                        if ("file".equals(uri.getScheme())) {
-                            iterator.remove();
-                            mPendingInputUris.add(uri);
-                            mAdapter.setCancelled(uri, null);
-                        }
-                    }
+    public void onRequestPermissionsResult(int requestCode,
+            @NonNull String[] permissions,
+            @NonNull int[] grantResults) {
 
-                    // check if there are any pending input uris
-                    cryptoOperation();
-                } else {
+        if (requestCode != REQUEST_PERMISSION_READ_EXTERNAL_STORAGE) {
+            super.onRequestPermissionsResult(requestCode, permissions, grantResults);
+            return;
+        }
 
-                    // permission denied -> cancel all pending file uris
-                    mCurrentInputUri = null;
-                    for (final Uri uri : mPendingInputUris) {
-                        if ("file".equals(uri.getScheme())) {
-                            if (! mCancelledInputUris.contains(uri)) {
-                                mCancelledInputUris.add(uri);
-                            }
-                            mAdapter.setCancelled(uri, new OnClickListener() {
-                                @Override
-                                public void onClick(View v) {
-                                    retryUri(uri);
-                                }
-                            });
-                        }
-                    }
+        boolean permissionWasGranted = grantResults.length > 0
+                && grantResults[0] == PackageManager.PERMISSION_GRANTED;
+
+        if (permissionWasGranted) {
+
+            // permission granted -> retry all cancelled file uris
+            for (Uri uri : mCancelledInputUris) {
+                if ( ! "file".equals(uri.getScheme())) {
+                    continue;
                 }
+                mCancelledInputUris.remove(uri);
+                mPendingInputUris.add(uri);
+                mAdapter.setCancelled(uri, false);
             }
-            default: {
-                super.onRequestPermissionsResult(requestCode, permissions, grantResults);
+
+        } else {
+
+            // permission denied -> cancel current, and all pending file uris
+            mCurrentInputUri = null;
+            for (final Uri uri : mPendingInputUris) {
+                if ( ! "file".equals(uri.getScheme())) {
+                    continue;
+                }
+                mPendingInputUris.remove(uri);
+                mCancelledInputUris.add(uri);
+                mAdapter.setCancelled(uri, true);
             }
+
         }
+
+        // hand control flow back
+        cryptoOperation();
+
     }
 
     @Override
@@ -1034,10 +1028,19 @@ public class DecryptListFragment
             notifyItemChanged(pos);
         }
 
-        public void setCancelled(Uri uri, OnClickListener retryListener) {
+        public void setCancelled(final Uri uri, boolean isCancelled) {
             ViewModel newModel = new ViewModel(uri);
             int pos = mDataset.indexOf(newModel);
-            mDataset.get(pos).setCancelled(retryListener);
+            if (isCancelled) {
+                mDataset.get(pos).setCancelled(new OnClickListener() {
+                    @Override
+                    public void onClick(View v) {
+                        retryUri(uri);
+                    }
+                });
+            } else {
+                mDataset.get(pos).setCancelled(null);
+            }
             notifyItemChanged(pos);
         }