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 <Jason@zx2c4.com>
This commit is contained in:
Jason A. Donenfeld 2020-09-26 13:34:20 +02:00
parent 53ca421a85
commit 938399d881
6 changed files with 80 additions and 69 deletions

View File

@ -31,7 +31,6 @@ import kotlinx.coroutines.launch
* attached to a `BaseActivity`. * attached to a `BaseActivity`.
*/ */
abstract class BaseFragment : Fragment(), OnSelectedTunnelChangedListener { abstract class BaseFragment : Fragment(), OnSelectedTunnelChangedListener {
private var baseActivity: BaseActivity? = null
private var pendingTunnel: ObservableTunnel? = null private var pendingTunnel: ObservableTunnel? = null
private var pendingTunnelUp: Boolean? = null private var pendingTunnelUp: Boolean? = null
private val permissionActivityResultLauncher = registerForActivityResult(ActivityResultContracts.StartActivityForResult()) { private val permissionActivityResultLauncher = registerForActivityResult(ActivityResultContracts.StartActivityForResult()) {
@ -44,24 +43,18 @@ abstract class BaseFragment : Fragment(), OnSelectedTunnelChangedListener {
} }
protected var selectedTunnel: ObservableTunnel? protected var selectedTunnel: ObservableTunnel?
get() = baseActivity?.selectedTunnel get() = (activity as? BaseActivity)?.selectedTunnel
protected set(tunnel) { protected set(tunnel) {
baseActivity?.selectedTunnel = tunnel (activity as? BaseActivity)?.selectedTunnel = tunnel
} }
override fun onAttach(context: Context) { override fun onAttach(context: Context) {
super.onAttach(context) super.onAttach(context)
if (context is BaseActivity) { (activity as? BaseActivity)?.addOnSelectedTunnelChangedListener(this)
baseActivity = context
baseActivity?.addOnSelectedTunnelChangedListener(this)
} else {
baseActivity = null
}
} }
override fun onDetach() { override fun onDetach() {
baseActivity?.removeOnSelectedTunnelChangedListener(this) (activity as? BaseActivity)?.removeOnSelectedTunnelChangedListener(this)
baseActivity = null
super.onDetach() super.onDetach()
} }
@ -71,9 +64,10 @@ abstract class BaseFragment : Fragment(), OnSelectedTunnelChangedListener {
is TunnelListItemBinding -> binding.item is TunnelListItemBinding -> binding.item
else -> return else -> return
} ?: return } ?: return
lifecycleScope.launch { val activity = activity ?: return
activity.lifecycleScope.launch {
if (Application.getBackend() is GoBackend) { if (Application.getBackend() is GoBackend) {
val intent = GoBackend.VpnService.prepare(view.context) val intent = GoBackend.VpnService.prepare(activity)
if (intent != null) { if (intent != null) {
pendingTunnel = tunnel pendingTunnel = tunnel
pendingTunnelUp = checked pendingTunnelUp = checked
@ -86,20 +80,21 @@ abstract class BaseFragment : Fragment(), OnSelectedTunnelChangedListener {
} }
private fun setTunnelStateWithPermissionsResult(tunnel: ObservableTunnel, checked: Boolean) { private fun setTunnelStateWithPermissionsResult(tunnel: ObservableTunnel, checked: Boolean) {
lifecycleScope.launch { val activity = activity ?: return
activity.lifecycleScope.launch {
try { try {
tunnel.setStateAsync(Tunnel.State.of(checked)) tunnel.setStateAsync(Tunnel.State.of(checked))
} catch (e: Throwable) { } catch (e: Throwable) {
val error = ErrorMessages[e] val error = ErrorMessages[e]
val messageResId = if (checked) R.string.error_up else R.string.error_down 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 val view = view
if (view != null) if (view != null)
Snackbar.make(view, message, Snackbar.LENGTH_LONG) Snackbar.make(view, message, Snackbar.LENGTH_LONG)
.setAnchorView(view.findViewById(R.id.create_fab)) .setAnchorView(view.findViewById(R.id.create_fab))
.show() .show()
else else
Toast.makeText(activity ?: Application.get(), message, Toast.LENGTH_LONG).show() Toast.makeText(activity, message, Toast.LENGTH_LONG).show()
Log.e(TAG, message, e) Log.e(TAG, message, e)
} }
} }

View File

@ -29,15 +29,15 @@ class ConfigNamingDialogFragment : DialogFragment() {
private var imm: InputMethodManager? = null private var imm: InputMethodManager? = null
private fun createTunnelAndDismiss() { private fun createTunnelAndDismiss() {
binding?.let { val binding = binding ?: return
val name = it.tunnelNameText.text.toString() val activity = activity ?: return
lifecycleScope.launch { val name = binding.tunnelNameText.text.toString()
try { activity.lifecycleScope.launch {
Application.getTunnelManager().create(name, config) try {
dismiss() Application.getTunnelManager().create(name, config)
} catch (e: Throwable) { dismiss()
it.tunnelNameTextLayout.error = e.message } catch (e: Throwable) {
} binding.tunnelNameTextLayout.error = e.message
} }
} }
} }

View File

@ -39,24 +39,27 @@ class TunnelEditorFragment : BaseFragment() {
private var haveShownKeys = false private var haveShownKeys = false
private var binding: TunnelEditorFragmentBinding? = null private var binding: TunnelEditorFragmentBinding? = null
private var tunnel: ObservableTunnel? = null private var tunnel: ObservableTunnel? = null
private fun onConfigLoaded(config: Config) { private fun onConfigLoaded(config: Config) {
binding?.config = ConfigProxy(config) binding?.config = ConfigProxy(config)
} }
private fun onConfigSaved(savedTunnel: Tunnel, throwable: Throwable?) { private fun onConfigSaved(savedTunnel: Tunnel, throwable: Throwable?) {
val message: String val ctx = activity ?: Application.get()
if (throwable == null) { 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) Log.d(TAG, message)
Toast.makeText(activity ?: Application.get(), message, Toast.LENGTH_SHORT).show() Toast.makeText(ctx, message, Toast.LENGTH_SHORT).show()
onFinished() onFinished()
} else { } else {
val error = ErrorMessages[throwable] 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) Log.e(TAG, message, throwable)
binding?.let { val binding = binding
Snackbar.make(it.mainContainer, message, Snackbar.LENGTH_LONG).show() 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() Snackbar.make(binding!!.mainContainer, error, Snackbar.LENGTH_LONG).show()
return false return false
} }
lifecycleScope.launch { val activity = requireActivity()
activity.lifecycleScope.launch {
when { when {
tunnel == null -> { tunnel == null -> {
Log.d(TAG, "Attempting to create new tunnel " + binding!!.name) Log.d(TAG, "Attempting to create new tunnel " + binding!!.name)
@ -209,46 +213,48 @@ class TunnelEditorFragment : BaseFragment() {
} }
private fun onTunnelCreated(newTunnel: ObservableTunnel?, throwable: Throwable?) { private fun onTunnelCreated(newTunnel: ObservableTunnel?, throwable: Throwable?) {
val message: String val ctx = activity ?: Application.get()
if (throwable == null) { if (throwable == null) {
tunnel = newTunnel 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) Log.d(TAG, message)
Toast.makeText(activity ?: Application.get(), message, Toast.LENGTH_SHORT).show() Toast.makeText(ctx, message, Toast.LENGTH_SHORT).show()
onFinished() onFinished()
} else { } else {
val error = ErrorMessages[throwable] 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) Log.e(TAG, message, throwable)
binding?.let { val binding = binding
Snackbar.make(it.mainContainer, message, Snackbar.LENGTH_LONG).show() 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, private suspend fun onTunnelRenamed(renamedTunnel: ObservableTunnel, newConfig: Config,
throwable: Throwable?) { throwable: Throwable?) {
val message: String val ctx = activity ?: Application.get()
if (throwable == null) { 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) Log.d(TAG, message)
// Now save the rest of configuration changes. // Now save the rest of configuration changes.
Log.d(TAG, "Attempting to save config of renamed tunnel " + tunnel!!.name) Log.d(TAG, "Attempting to save config of renamed tunnel " + tunnel!!.name)
lifecycleScope.launch { try {
try { renamedTunnel.setConfigAsync(newConfig)
renamedTunnel.setConfigAsync(newConfig) onConfigSaved(renamedTunnel, null)
onConfigSaved(renamedTunnel, null) } catch (e: Throwable) {
} catch (e: Throwable) { onConfigSaved(renamedTunnel, e)
onConfigSaved(renamedTunnel, e)
}
} }
} else { } else {
val error = ErrorMessages[throwable] 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) Log.e(TAG, message, throwable)
binding?.let { val binding = binding
Snackbar.make(it.mainContainer, message, Snackbar.LENGTH_LONG).show() if (binding != null)
} Snackbar.make(binding.mainContainer, message, Snackbar.LENGTH_LONG).show()
else
Toast.makeText(ctx, message, Toast.LENGTH_SHORT).show()
} }
} }

View File

@ -15,6 +15,7 @@ import android.view.View
import android.view.ViewGroup import android.view.ViewGroup
import android.view.animation.Animation import android.view.animation.Animation
import android.view.animation.AnimationUtils import android.view.animation.AnimationUtils
import android.widget.Toast
import androidx.activity.result.contract.ActivityResultContracts import androidx.activity.result.contract.ActivityResultContracts
import androidx.appcompat.app.AppCompatActivity import androidx.appcompat.app.AppCompatActivity
import androidx.appcompat.view.ActionMode import androidx.appcompat.view.ActionMode
@ -46,9 +47,10 @@ class TunnelListFragment : BaseFragment() {
private var actionMode: ActionMode? = null private var actionMode: ActionMode? = null
private var binding: TunnelListFragmentBinding? = null private var binding: TunnelListFragmentBinding? = null
private val tunnelFileImportResultLauncher = registerForActivityResult(ActivityResultContracts.GetContent()) { data -> private val tunnelFileImportResultLauncher = registerForActivityResult(ActivityResultContracts.GetContent()) { data ->
lifecycleScope.launch { if (data == null) return@registerForActivityResult
if (data == null) return@launch val activity = activity ?: return@registerForActivityResult
val contentResolver = activity?.contentResolver ?: return@launch val contentResolver = activity.contentResolver ?: return@registerForActivityResult
activity.lifecycleScope.launch {
TunnelImporter.importTunnel(contentResolver, data) { showSnackbar(it) } TunnelImporter.importTunnel(contentResolver, data) { showSnackbar(it) }
} }
} }
@ -56,7 +58,9 @@ class TunnelListFragment : BaseFragment() {
private val qrImportResultLauncher = registerForActivityResult(ActivityResultContracts.StartActivityForResult()) { result -> private val qrImportResultLauncher = registerForActivityResult(ActivityResultContracts.StartActivityForResult()) { result ->
val qrCode = IntentIntegrator.parseActivityResult(result.resultCode, result.data)?.contents val qrCode = IntentIntegrator.parseActivityResult(result.resultCode, result.data)?.contents
?: return@registerForActivityResult ?: 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?) { override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
@ -121,11 +125,12 @@ class TunnelListFragment : BaseFragment() {
private fun onTunnelDeletionFinished(count: Int, throwable: Throwable?) { private fun onTunnelDeletionFinished(count: Int, throwable: Throwable?) {
val message: String val message: String
val ctx = activity ?: Application.get()
if (throwable == null) { if (throwable == null) {
message = resources.getQuantityString(R.plurals.delete_success, count, count) message = ctx.resources.getQuantityString(R.plurals.delete_success, count, count)
} else { } else {
val error = ErrorMessages[throwable] 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) Log.e(TAG, message, throwable)
} }
showSnackbar(message) showSnackbar(message)
@ -159,11 +164,13 @@ class TunnelListFragment : BaseFragment() {
} }
private fun showSnackbar(message: CharSequence) { private fun showSnackbar(message: CharSequence) {
binding?.let { val binding = binding
Snackbar.make(it.mainContainer, message, Snackbar.LENGTH_LONG) if (binding != null)
.setAnchorView(it.createFab) Snackbar.make(binding.mainContainer, message, Snackbar.LENGTH_LONG)
.setAnchorView(binding.createFab)
.show() .show()
} else
Toast.makeText(activity ?: Application.get(), message, Toast.LENGTH_SHORT).show()
} }
private fun viewForTunnel(tunnel: ObservableTunnel, tunnels: List<*>): MultiselectableRelativeLayout? { private fun viewForTunnel(tunnel: ObservableTunnel, tunnels: List<*>): MultiselectableRelativeLayout? {
@ -181,13 +188,14 @@ class TunnelListFragment : BaseFragment() {
override fun onActionItemClicked(mode: ActionMode, item: MenuItem): Boolean { override fun onActionItemClicked(mode: ActionMode, item: MenuItem): Boolean {
return when (item.itemId) { return when (item.itemId) {
R.id.menu_action_delete -> { R.id.menu_action_delete -> {
val activity = activity ?: return true
val copyCheckedItems = HashSet(checkedItems) val copyCheckedItems = HashSet(checkedItems)
binding?.createFab?.apply { binding?.createFab?.apply {
visibility = View.VISIBLE visibility = View.VISIBLE
scaleX = 1f scaleX = 1f
scaleY = 1f scaleY = 1f
} }
lifecycleScope.launch { activity.lifecycleScope.launch {
try { try {
val tunnels = Application.getTunnelManager().getTunnels() val tunnels = Application.getTunnelManager().getTunnels()
val tunnelsToDelete = ArrayList<ObservableTunnel>() val tunnelsToDelete = ArrayList<ObservableTunnel>()

View File

@ -8,6 +8,7 @@ import android.content.Context
import android.content.Intent import android.content.Intent
import android.util.AttributeSet import android.util.AttributeSet
import android.util.Log import android.util.Log
import androidx.lifecycle.lifecycleScope
import androidx.preference.Preference import androidx.preference.Preference
import com.wireguard.android.Application import com.wireguard.android.Application
import com.wireguard.android.R 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.Tunnel
import com.wireguard.android.backend.WgQuickBackend import com.wireguard.android.backend.WgQuickBackend
import com.wireguard.android.util.UserKnobs import com.wireguard.android.util.UserKnobs
import com.wireguard.android.util.activity
import com.wireguard.android.util.lifecycleScope import com.wireguard.android.util.lifecycleScope
import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.SupervisorJob 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 getTitle() = if (state == State.UNKNOWN) "" else context.getString(state.titleResourceId)
override fun onClick() { override fun onClick() {
lifecycleScope.launch { activity.lifecycleScope.launch {
if (state == State.DISABLED) { if (state == State.DISABLED) {
setState(State.ENABLING) setState(State.ENABLING)
UserKnobs.setDisableKernelModule(false) UserKnobs.setDisableKernelModule(false)

View File

@ -6,7 +6,7 @@ package com.wireguard.android.util
import android.content.res.Resources import android.content.res.Resources
import android.os.RemoteException 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.R
import com.wireguard.android.backend.BackendException import com.wireguard.android.backend.BackendException
import com.wireguard.android.util.RootShell.RootShellException import com.wireguard.android.util.RootShell.RootShellException
@ -63,7 +63,7 @@ object ErrorMessages {
) )
operator fun get(throwable: Throwable?): String { 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) if (throwable == null) return resources.getString(R.string.unknown_error)
val rootCause = rootCause(throwable) val rootCause = rootCause(throwable)
return when { return when {