Merge pull request #10883 from nextcloud/fix/permission-dialog-crash

Fix creation and result logic for StoragePermissionDialogFragment

app/src/androidTest/java/com/owncloud/android/ui/dialog/DialogFragmentIT.java

@@ -484,6 +484,18 @@ public class DialogFragmentIT extends AbstractIT {
         showDialog(sut);
     }
 
+
+    @Test
+    @ScreenshotTest
+    public void testStoragePermissionDialog() {
+        if (Looper.myLooper() == null) {
+            Looper.prepare();
+        }
+
+        StoragePermissionDialogFragment sut = StoragePermissionDialogFragment.Companion.newInstance(false);
+        showDialog(sut);
+    }
+
     private FileDisplayActivity showDialog(DialogFragment dialog) {
         Intent intent = new Intent(targetContext, FileDisplayActivity.class);
 

app/src/main/java/com/owncloud/android/ui/dialog/StoragePermissionDialogFragment.kt

@@ -23,35 +23,43 @@ package com.owncloud.android.ui.dialog
 import android.app.Dialog
 import android.os.Build
 import android.os.Bundle
+import android.os.Parcelable
 import android.view.View
 import androidx.annotation.RequiresApi
 import androidx.appcompat.app.AlertDialog
+import androidx.core.os.bundleOf
 import androidx.fragment.app.DialogFragment
 import com.google.android.material.dialog.MaterialAlertDialogBuilder
 import com.nextcloud.client.di.Injectable
 import com.owncloud.android.R
 import com.owncloud.android.databinding.StoragePermissionDialogBinding
-import com.owncloud.android.ui.dialog.StoragePermissionDialogFragment.Listener
 import com.owncloud.android.utils.theme.ViewThemeUtils
+import kotlinx.parcelize.Parcelize
 import javax.inject.Inject
 
 /**
  * Dialog that shows permission options in SDK >= 30
  *
  * Allows choosing "full access" (MANAGE_ALL_FILES) or "read-only media" (READ_EXTERNAL_STORAGE)
- *
- * @param listener a [Listener] for button clicks. The dialog will auto-dismiss after the callback is called.
- * @param permissionRequired Whether the permission is absolutely required by the calling component.
- * This changes the texts to a more strict version.
  */
 @RequiresApi(Build.VERSION_CODES.R)
-class StoragePermissionDialogFragment(val listener: Listener, val permissionRequired: Boolean = false) :
+class StoragePermissionDialogFragment :
     DialogFragment(), Injectable {
+
     private lateinit var binding: StoragePermissionDialogBinding
 
+    private var permissionRequired = false
+
     @Inject
     lateinit var viewThemeUtils: ViewThemeUtils
 
+    override fun onCreate(savedInstanceState: Bundle?) {
+        super.onCreate(savedInstanceState)
+        arguments?.let {
+            permissionRequired = it.getBoolean(ARG_PERMISSION_REQUIRED, false)
+        }
+    }
+
     override fun onStart() {
         super.onStart()
         dialog?.let {
@@ -75,12 +83,12 @@ class StoragePermissionDialogFragment(val listener: Listener, val permissionRequ
         // Setup layout
         viewThemeUtils.material.colorMaterialButtonPrimaryFilled(binding.btnFullAccess)
         binding.btnFullAccess.setOnClickListener {
-            listener.onClickFullAccess()
+            setResult(Result.FULL_ACCESS)
             dismiss()
         }
         viewThemeUtils.platform.colorTextButtons(binding.btnReadOnly)
         binding.btnReadOnly.setOnClickListener {
-            listener.onClickMediaReadOnly()
+            setResult(Result.MEDIA_READ_ONLY)
             dismiss()
         }
 
@@ -94,7 +102,7 @@ class StoragePermissionDialogFragment(val listener: Listener, val permissionRequ
             .setTitle(titleResource)
             .setView(view)
             .setNegativeButton(R.string.common_cancel) { _, _ ->
-                listener.onCancel()
+                setResult(Result.CANCEL)
                 dismiss()
             }
 
@@ -103,9 +111,28 @@ class StoragePermissionDialogFragment(val listener: Listener, val permissionRequ
         return builder.create()
     }
 
-    interface Listener {
-        fun onCancel()
-        fun onClickFullAccess()
-        fun onClickMediaReadOnly()
+    private fun setResult(result: Result) {
+        parentFragmentManager.setFragmentResult(REQUEST_KEY, bundleOf(RESULT_KEY to result))
+    }
+
+    @Parcelize
+    enum class Result : Parcelable {
+        CANCEL, FULL_ACCESS, MEDIA_READ_ONLY
+    }
+
+    companion object {
+        private const val ARG_PERMISSION_REQUIRED = "ARG_PERMISSION_REQUIRED"
+        const val REQUEST_KEY = "REQUEST_KEY_STORAGE_PERMISSION"
+        const val RESULT_KEY = "RESULT"
+
+        /**
+         * @param permissionRequired Whether the permission is absolutely required by the calling component.
+         * This changes the texts to a more strict version.
+         */
+        fun newInstance(permissionRequired: Boolean): StoragePermissionDialogFragment {
+            return StoragePermissionDialogFragment().apply {
+                arguments = bundleOf(ARG_PERMISSION_REQUIRED to permissionRequired)
+            }
+        }
     }
 }

app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt

@@ -209,35 +209,39 @@ object PermissionUtil {
         viewThemeUtils: ViewThemeUtils
     ) {
         val preferences: AppPreferences = AppPreferencesImpl.fromContext(activity)
-
-        if (!preferences.isStoragePermissionRequested || permissionRequired) {
-            if (activity.supportFragmentManager.findFragmentByTag(PERMISSION_CHOICE_DIALOG_TAG) == null) {
-                val listener = object : StoragePermissionDialogFragment.Listener {
-                    override fun onCancel() {
-                        preferences.isStoragePermissionRequested = true
-                    }
-
-                    override fun onClickFullAccess() {
+        val shouldRequestPermission = !preferences.isStoragePermissionRequested || permissionRequired
+        if (shouldRequestPermission &&
+            activity.supportFragmentManager.findFragmentByTag(PERMISSION_CHOICE_DIALOG_TAG) == null
+        ) {
+            activity.supportFragmentManager.setFragmentResultListener(
+                StoragePermissionDialogFragment.REQUEST_KEY,
+                activity
+            ) { _, resultBundle ->
+                val result: StoragePermissionDialogFragment.Result? =
+                    resultBundle.getParcelable(StoragePermissionDialogFragment.RESULT_KEY)
+                if (result != null) {
                     preferences.isStoragePermissionRequested = true
+                    when (result) {
+                        StoragePermissionDialogFragment.Result.FULL_ACCESS -> {
                             val intent = getManageAllFilesIntent(activity)
                             activity.startActivityForResult(intent, REQUEST_CODE_MANAGE_ALL_FILES)
-                        preferences.isStoragePermissionRequested = true
                         }
-
-                    override fun onClickMediaReadOnly() {
-                        preferences.isStoragePermissionRequested = true
+                        StoragePermissionDialogFragment.Result.MEDIA_READ_ONLY -> {
                             requestStoragePermission(
                                 activity,
                                 Manifest.permission.READ_EXTERNAL_STORAGE,
                                 viewThemeUtils
                             )
                         }
+                        StoragePermissionDialogFragment.Result.CANCEL -> {}
                     }
-                val dialogFragment = StoragePermissionDialogFragment(listener, permissionRequired)
+                }
+            }
+
+            val dialogFragment = StoragePermissionDialogFragment.newInstance(permissionRequired)
             dialogFragment.show(activity.supportFragmentManager, PERMISSION_CHOICE_DIALOG_TAG)
         }
     }
-    }
 
     @RequiresApi(Build.VERSION_CODES.R)
     private fun getManageAllFilesIntent(context: Context) =