From 938399d881aa6b365be131ffb3a517d64be427bb Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Sat, 26 Sep 2020 13:34:20 +0200 Subject: [PATCH] ui: queue up tunnel mutating on activity scope instead of fragment scope Fragment scopes get cancelled when the fragment goes away, but we don't actually want to cancel an in-flight transition in that case. Also, before when the fragment would cancel, there'd be an exception, and the exception handler would call Fragment::getString, which in turn called requireContext, which caused an exception. Work around this by using the `activity ?: Application.get()` idiom to always have a context for strings and toasts. Signed-off-by: Jason A. Donenfeld --- .../android/fragment/BaseFragment.kt | 27 ++++---- .../fragment/ConfigNamingDialogFragment.kt | 18 ++--- .../android/fragment/TunnelEditorFragment.kt | 66 ++++++++++--------- .../android/fragment/TunnelListFragment.kt | 30 +++++---- .../KernelModuleDisablerPreference.kt | 4 +- .../wireguard/android/util/ErrorMessages.kt | 4 +- 6 files changed, 80 insertions(+), 69 deletions(-) diff --git a/ui/src/main/java/com/wireguard/android/fragment/BaseFragment.kt b/ui/src/main/java/com/wireguard/android/fragment/BaseFragment.kt index 10239e6e..90943f0a 100644 --- a/ui/src/main/java/com/wireguard/android/fragment/BaseFragment.kt +++ b/ui/src/main/java/com/wireguard/android/fragment/BaseFragment.kt @@ -31,7 +31,6 @@ import kotlinx.coroutines.launch * attached to a `BaseActivity`. */ abstract class BaseFragment : Fragment(), OnSelectedTunnelChangedListener { - private var baseActivity: BaseActivity? = null private var pendingTunnel: ObservableTunnel? = null private var pendingTunnelUp: Boolean? = null private val permissionActivityResultLauncher = registerForActivityResult(ActivityResultContracts.StartActivityForResult()) { @@ -44,24 +43,18 @@ abstract class BaseFragment : Fragment(), OnSelectedTunnelChangedListener { } protected var selectedTunnel: ObservableTunnel? - get() = baseActivity?.selectedTunnel + get() = (activity as? BaseActivity)?.selectedTunnel protected set(tunnel) { - baseActivity?.selectedTunnel = tunnel + (activity as? BaseActivity)?.selectedTunnel = tunnel } override fun onAttach(context: Context) { super.onAttach(context) - if (context is BaseActivity) { - baseActivity = context - baseActivity?.addOnSelectedTunnelChangedListener(this) - } else { - baseActivity = null - } + (activity as? BaseActivity)?.addOnSelectedTunnelChangedListener(this) } override fun onDetach() { - baseActivity?.removeOnSelectedTunnelChangedListener(this) - baseActivity = null + (activity as? BaseActivity)?.removeOnSelectedTunnelChangedListener(this) super.onDetach() } @@ -71,9 +64,10 @@ abstract class BaseFragment : Fragment(), OnSelectedTunnelChangedListener { is TunnelListItemBinding -> binding.item else -> return } ?: return - lifecycleScope.launch { + val activity = activity ?: return + activity.lifecycleScope.launch { if (Application.getBackend() is GoBackend) { - val intent = GoBackend.VpnService.prepare(view.context) + val intent = GoBackend.VpnService.prepare(activity) if (intent != null) { pendingTunnel = tunnel pendingTunnelUp = checked @@ -86,20 +80,21 @@ abstract class BaseFragment : Fragment(), OnSelectedTunnelChangedListener { } private fun setTunnelStateWithPermissionsResult(tunnel: ObservableTunnel, checked: Boolean) { - lifecycleScope.launch { + val activity = activity ?: return + activity.lifecycleScope.launch { try { tunnel.setStateAsync(Tunnel.State.of(checked)) } catch (e: Throwable) { val error = ErrorMessages[e] val messageResId = if (checked) R.string.error_up else R.string.error_down - val message = getString(messageResId, error) + val message = activity.getString(messageResId, error) val view = view if (view != null) Snackbar.make(view, message, Snackbar.LENGTH_LONG) .setAnchorView(view.findViewById(R.id.create_fab)) .show() else - Toast.makeText(activity ?: Application.get(), message, Toast.LENGTH_LONG).show() + Toast.makeText(activity, message, Toast.LENGTH_LONG).show() Log.e(TAG, message, e) } } diff --git a/ui/src/main/java/com/wireguard/android/fragment/ConfigNamingDialogFragment.kt b/ui/src/main/java/com/wireguard/android/fragment/ConfigNamingDialogFragment.kt index cd2431c9..0919f5ea 100644 --- a/ui/src/main/java/com/wireguard/android/fragment/ConfigNamingDialogFragment.kt +++ b/ui/src/main/java/com/wireguard/android/fragment/ConfigNamingDialogFragment.kt @@ -29,15 +29,15 @@ class ConfigNamingDialogFragment : DialogFragment() { private var imm: InputMethodManager? = null private fun createTunnelAndDismiss() { - binding?.let { - val name = it.tunnelNameText.text.toString() - lifecycleScope.launch { - try { - Application.getTunnelManager().create(name, config) - dismiss() - } catch (e: Throwable) { - it.tunnelNameTextLayout.error = e.message - } + val binding = binding ?: return + val activity = activity ?: return + val name = binding.tunnelNameText.text.toString() + activity.lifecycleScope.launch { + try { + Application.getTunnelManager().create(name, config) + dismiss() + } catch (e: Throwable) { + binding.tunnelNameTextLayout.error = e.message } } } diff --git a/ui/src/main/java/com/wireguard/android/fragment/TunnelEditorFragment.kt b/ui/src/main/java/com/wireguard/android/fragment/TunnelEditorFragment.kt index e035210a..f002c085 100644 --- a/ui/src/main/java/com/wireguard/android/fragment/TunnelEditorFragment.kt +++ b/ui/src/main/java/com/wireguard/android/fragment/TunnelEditorFragment.kt @@ -39,24 +39,27 @@ class TunnelEditorFragment : BaseFragment() { private var haveShownKeys = false private var binding: TunnelEditorFragmentBinding? = null private var tunnel: ObservableTunnel? = null + private fun onConfigLoaded(config: Config) { binding?.config = ConfigProxy(config) } private fun onConfigSaved(savedTunnel: Tunnel, throwable: Throwable?) { - val message: String + val ctx = activity ?: Application.get() if (throwable == null) { - message = getString(R.string.config_save_success, savedTunnel.name) + val message = ctx.getString(R.string.config_save_success, savedTunnel.name) Log.d(TAG, message) - Toast.makeText(activity ?: Application.get(), message, Toast.LENGTH_SHORT).show() + Toast.makeText(ctx, message, Toast.LENGTH_SHORT).show() onFinished() } else { val error = ErrorMessages[throwable] - message = getString(R.string.config_save_error, savedTunnel.name, error) + val message = ctx.getString(R.string.config_save_error, savedTunnel.name, error) Log.e(TAG, message, throwable) - binding?.let { - Snackbar.make(it.mainContainer, message, Snackbar.LENGTH_LONG).show() - } + val binding = binding + if (binding != null) + Snackbar.make(binding.mainContainer, message, Snackbar.LENGTH_LONG).show() + else + Toast.makeText(ctx, message, Toast.LENGTH_SHORT).show() } } @@ -115,7 +118,8 @@ class TunnelEditorFragment : BaseFragment() { Snackbar.make(binding!!.mainContainer, error, Snackbar.LENGTH_LONG).show() return false } - lifecycleScope.launch { + val activity = requireActivity() + activity.lifecycleScope.launch { when { tunnel == null -> { Log.d(TAG, "Attempting to create new tunnel " + binding!!.name) @@ -209,46 +213,48 @@ class TunnelEditorFragment : BaseFragment() { } private fun onTunnelCreated(newTunnel: ObservableTunnel?, throwable: Throwable?) { - val message: String + val ctx = activity ?: Application.get() if (throwable == null) { tunnel = newTunnel - message = getString(R.string.tunnel_create_success, tunnel!!.name) + val message = ctx.getString(R.string.tunnel_create_success, tunnel!!.name) Log.d(TAG, message) - Toast.makeText(activity ?: Application.get(), message, Toast.LENGTH_SHORT).show() + Toast.makeText(ctx, message, Toast.LENGTH_SHORT).show() onFinished() } else { val error = ErrorMessages[throwable] - message = getString(R.string.tunnel_create_error, error) + val message = ctx.getString(R.string.tunnel_create_error, error) Log.e(TAG, message, throwable) - binding?.let { - Snackbar.make(it.mainContainer, message, Snackbar.LENGTH_LONG).show() - } + val binding = binding + if (binding != null) + Snackbar.make(binding.mainContainer, message, Snackbar.LENGTH_LONG).show() + else + Toast.makeText(ctx, message, Toast.LENGTH_SHORT).show() } } - private fun onTunnelRenamed(renamedTunnel: ObservableTunnel, newConfig: Config, - throwable: Throwable?) { - val message: String + private suspend fun onTunnelRenamed(renamedTunnel: ObservableTunnel, newConfig: Config, + throwable: Throwable?) { + val ctx = activity ?: Application.get() if (throwable == null) { - message = getString(R.string.tunnel_rename_success, renamedTunnel.name) + val message = ctx.getString(R.string.tunnel_rename_success, renamedTunnel.name) Log.d(TAG, message) // Now save the rest of configuration changes. Log.d(TAG, "Attempting to save config of renamed tunnel " + tunnel!!.name) - lifecycleScope.launch { - try { - renamedTunnel.setConfigAsync(newConfig) - onConfigSaved(renamedTunnel, null) - } catch (e: Throwable) { - onConfigSaved(renamedTunnel, e) - } + try { + renamedTunnel.setConfigAsync(newConfig) + onConfigSaved(renamedTunnel, null) + } catch (e: Throwable) { + onConfigSaved(renamedTunnel, e) } } else { val error = ErrorMessages[throwable] - message = getString(R.string.tunnel_rename_error, error) + val message = ctx.getString(R.string.tunnel_rename_error, error) Log.e(TAG, message, throwable) - binding?.let { - Snackbar.make(it.mainContainer, message, Snackbar.LENGTH_LONG).show() - } + val binding = binding + if (binding != null) + Snackbar.make(binding.mainContainer, message, Snackbar.LENGTH_LONG).show() + else + Toast.makeText(ctx, message, Toast.LENGTH_SHORT).show() } } diff --git a/ui/src/main/java/com/wireguard/android/fragment/TunnelListFragment.kt b/ui/src/main/java/com/wireguard/android/fragment/TunnelListFragment.kt index 9c9d0ecd..3fbc3451 100644 --- a/ui/src/main/java/com/wireguard/android/fragment/TunnelListFragment.kt +++ b/ui/src/main/java/com/wireguard/android/fragment/TunnelListFragment.kt @@ -15,6 +15,7 @@ import android.view.View import android.view.ViewGroup import android.view.animation.Animation import android.view.animation.AnimationUtils +import android.widget.Toast import androidx.activity.result.contract.ActivityResultContracts import androidx.appcompat.app.AppCompatActivity import androidx.appcompat.view.ActionMode @@ -46,9 +47,10 @@ class TunnelListFragment : BaseFragment() { private var actionMode: ActionMode? = null private var binding: TunnelListFragmentBinding? = null private val tunnelFileImportResultLauncher = registerForActivityResult(ActivityResultContracts.GetContent()) { data -> - lifecycleScope.launch { - if (data == null) return@launch - val contentResolver = activity?.contentResolver ?: return@launch + if (data == null) return@registerForActivityResult + val activity = activity ?: return@registerForActivityResult + val contentResolver = activity.contentResolver ?: return@registerForActivityResult + activity.lifecycleScope.launch { TunnelImporter.importTunnel(contentResolver, data) { showSnackbar(it) } } } @@ -56,7 +58,9 @@ class TunnelListFragment : BaseFragment() { private val qrImportResultLauncher = registerForActivityResult(ActivityResultContracts.StartActivityForResult()) { result -> val qrCode = IntentIntegrator.parseActivityResult(result.resultCode, result.data)?.contents ?: return@registerForActivityResult - lifecycleScope.launch { TunnelImporter.importTunnel(parentFragmentManager, qrCode) { showSnackbar(it) } } + val activity = activity ?: return@registerForActivityResult + val fragManager = parentFragmentManager + activity.lifecycleScope.launch { TunnelImporter.importTunnel(fragManager, qrCode) { showSnackbar(it) } } } override fun onViewCreated(view: View, savedInstanceState: Bundle?) { @@ -121,11 +125,12 @@ class TunnelListFragment : BaseFragment() { private fun onTunnelDeletionFinished(count: Int, throwable: Throwable?) { val message: String + val ctx = activity ?: Application.get() if (throwable == null) { - message = resources.getQuantityString(R.plurals.delete_success, count, count) + message = ctx.resources.getQuantityString(R.plurals.delete_success, count, count) } else { val error = ErrorMessages[throwable] - message = resources.getQuantityString(R.plurals.delete_error, count, count, error) + message = ctx.resources.getQuantityString(R.plurals.delete_error, count, count, error) Log.e(TAG, message, throwable) } showSnackbar(message) @@ -159,11 +164,13 @@ class TunnelListFragment : BaseFragment() { } private fun showSnackbar(message: CharSequence) { - binding?.let { - Snackbar.make(it.mainContainer, message, Snackbar.LENGTH_LONG) - .setAnchorView(it.createFab) + val binding = binding + if (binding != null) + Snackbar.make(binding.mainContainer, message, Snackbar.LENGTH_LONG) + .setAnchorView(binding.createFab) .show() - } + else + Toast.makeText(activity ?: Application.get(), message, Toast.LENGTH_SHORT).show() } private fun viewForTunnel(tunnel: ObservableTunnel, tunnels: List<*>): MultiselectableRelativeLayout? { @@ -181,13 +188,14 @@ class TunnelListFragment : BaseFragment() { override fun onActionItemClicked(mode: ActionMode, item: MenuItem): Boolean { return when (item.itemId) { R.id.menu_action_delete -> { + val activity = activity ?: return true val copyCheckedItems = HashSet(checkedItems) binding?.createFab?.apply { visibility = View.VISIBLE scaleX = 1f scaleY = 1f } - lifecycleScope.launch { + activity.lifecycleScope.launch { try { val tunnels = Application.getTunnelManager().getTunnels() val tunnelsToDelete = ArrayList() diff --git a/ui/src/main/java/com/wireguard/android/preference/KernelModuleDisablerPreference.kt b/ui/src/main/java/com/wireguard/android/preference/KernelModuleDisablerPreference.kt index 5d21a541..2b1e8e4e 100644 --- a/ui/src/main/java/com/wireguard/android/preference/KernelModuleDisablerPreference.kt +++ b/ui/src/main/java/com/wireguard/android/preference/KernelModuleDisablerPreference.kt @@ -8,6 +8,7 @@ import android.content.Context import android.content.Intent import android.util.AttributeSet import android.util.Log +import androidx.lifecycle.lifecycleScope import androidx.preference.Preference import com.wireguard.android.Application import com.wireguard.android.R @@ -15,6 +16,7 @@ import com.wireguard.android.activity.SettingsActivity import com.wireguard.android.backend.Tunnel import com.wireguard.android.backend.WgQuickBackend import com.wireguard.android.util.UserKnobs +import com.wireguard.android.util.activity import com.wireguard.android.util.lifecycleScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.SupervisorJob @@ -39,7 +41,7 @@ class KernelModuleDisablerPreference(context: Context, attrs: AttributeSet?) : P override fun getTitle() = if (state == State.UNKNOWN) "" else context.getString(state.titleResourceId) override fun onClick() { - lifecycleScope.launch { + activity.lifecycleScope.launch { if (state == State.DISABLED) { setState(State.ENABLING) UserKnobs.setDisableKernelModule(false) diff --git a/ui/src/main/java/com/wireguard/android/util/ErrorMessages.kt b/ui/src/main/java/com/wireguard/android/util/ErrorMessages.kt index d8ac94d9..1ee0dafc 100644 --- a/ui/src/main/java/com/wireguard/android/util/ErrorMessages.kt +++ b/ui/src/main/java/com/wireguard/android/util/ErrorMessages.kt @@ -6,7 +6,7 @@ package com.wireguard.android.util import android.content.res.Resources import android.os.RemoteException -import com.wireguard.android.Application.Companion.get +import com.wireguard.android.Application import com.wireguard.android.R import com.wireguard.android.backend.BackendException import com.wireguard.android.util.RootShell.RootShellException @@ -63,7 +63,7 @@ object ErrorMessages { ) operator fun get(throwable: Throwable?): String { - val resources = get().resources + val resources = Application.get().resources if (throwable == null) return resources.getString(R.string.unknown_error) val rootCause = rootCause(throwable) return when {