Merge branch 'fix-android-foreground-and-tile-state'

4 files changed, 92 insertions, 44 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index e2ba3f2e8c..8d4d5c2d51 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -40,6 +40,9 @@ Line wrap the file at 100 chars.                                              Th
 #### Android
 - Fix unused dependencies loaded in the service/tile DI graph.
 - Fix missing IPC message unregistration causing multiple copies of some messages to be received.
+- Fix quick settings tile being unresponsive and causing crashes on some devices.
+- Fix quick settings tile not working when the device is locked. It will now prompt the user to
+  unlock the device before attempting to toggle the tunnel state.
 
 ### Security
 #### Android
diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/service/ForegroundNotificationManager.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/service/ForegroundNotificationManager.kt
index e98646d34f..0bf7e54606 100644
--- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/service/ForegroundNotificationManager.kt
+++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/service/ForegroundNotificationManager.kt
@@ -8,6 +8,7 @@ import kotlinx.coroutines.channels.Channel
 import kotlinx.coroutines.channels.actor
 import kotlinx.coroutines.channels.sendBlocking
 import kotlinx.coroutines.flow.collect
+import kotlinx.coroutines.flow.onStart
 import net.mullvad.mullvadvpn.model.DeviceState
 import net.mullvad.mullvadvpn.model.TunnelState
 import net.mullvad.mullvadvpn.service.endpoint.ConnectionProxy
@@ -55,9 +56,13 @@ class ForegroundNotificationManager(
 
         intermittentDaemon.registerListener(this) { daemon ->
             jobTracker.newBackgroundJob("notificationLoggedInJob") {
-                daemon?.deviceStateUpdates?.collect { deviceState ->
-                    loggedIn = deviceState is DeviceState.LoggedIn
-                }
+                daemon?.deviceStateUpdates
+                    ?.onStart {
+                        emit(daemon.getAndEmitDeviceState())
+                    }
+                    ?.collect { deviceState ->
+                        loggedIn = deviceState is DeviceState.LoggedIn
+                    }
             }
         }
 
@@ -87,7 +92,7 @@ class ForegroundNotificationManager(
         }
     }
 
-    private fun showOnForeground() {
+    fun showOnForeground() {
         service.startForeground(
             TunnelStateNotification.NOTIFICATION_ID,
             tunnelStateNotification.build()
diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadTileService.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadTileService.kt
index 3fbc627b7e..8b64face8f 100644
--- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadTileService.kt
+++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadTileService.kt
@@ -5,14 +5,17 @@ import android.graphics.drawable.Icon
 import android.os.Build
 import android.service.quicksettings.Tile
 import android.service.quicksettings.TileService
-import kotlin.properties.Delegates.observable
-import kotlinx.coroutines.CoroutineScope
+import android.util.Log
 import kotlinx.coroutines.FlowPreview
+import kotlinx.coroutines.Job
 import kotlinx.coroutines.MainScope
-import kotlinx.coroutines.cancel
+import kotlinx.coroutines.delay
 import kotlinx.coroutines.flow.collect
 import kotlinx.coroutines.flow.debounce
+import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.launch
+import kotlinx.coroutines.runBlocking
+import kotlinx.coroutines.withTimeoutOrNull
 import net.mullvad.mullvadvpn.R
 import net.mullvad.mullvadvpn.ipc.ServiceConnection
 import net.mullvad.mullvadvpn.model.ServiceResult
@@ -20,47 +23,66 @@ import net.mullvad.mullvadvpn.model.TunnelState
 import net.mullvad.talpid.tunnel.ActionAfterDisconnect
 
 class MullvadTileService : TileService() {
-    private var secured by observable(false) { _, _, _ ->
-        updateTileState()
-    }
-
-    private lateinit var scope: CoroutineScope
+    private val scope = MainScope()
+    private var listenerJob: Job? = null
 
     private lateinit var securedIcon: Icon
     private lateinit var unsecuredIcon: Icon
 
     override fun onCreate() {
-        super.onCreate()
-
         securedIcon = Icon.createWithResource(this, R.drawable.small_logo_white)
         unsecuredIcon = Icon.createWithResource(this, R.drawable.small_logo_black)
     }
 
-    override fun onStartListening() {
-        super.onStartListening()
+    override fun onClick() {
+        // Workaround for the reported bug: https://issuetracker.google.com/issues/236862865
+        suspend fun isUnlockStatusPropagatedWithinTimeout(
+            unlockTimeoutMillis: Long,
+            unlockCheckDelayMillis: Long
+        ): Boolean {
+            return withTimeoutOrNull(unlockTimeoutMillis) {
+                while (isLocked) {
+                    delay(unlockCheckDelayMillis)
+                }
+                return@withTimeoutOrNull true
+            } ?: false
+        }
 
-        scope = MainScope()
+        unlockAndRun {
+            runBlocking {
+                val isUnlockStatusPropagated = isUnlockStatusPropagatedWithinTimeout(
+                    unlockTimeoutMillis = 1000L,
+                    unlockCheckDelayMillis = 100L
+                )
 
-        scope.launch { listenToTunnelState() }
+                if (isUnlockStatusPropagated) {
+                    toggleTunnel()
+                } else {
+                    Log.e("mullvad", "Unable to toggle tunnel state")
+                }
+            }
+        }
     }
 
-    override fun onClick() {
-        super.onClick()
+    override fun onStartListening() {
+        listenerJob = scope.launch { listenToTunnelState() }
+    }
 
-        val intent = Intent(this, MullvadVpnService::class.java)
+    override fun onStopListening() {
+        listenerJob?.cancel()
+    }
 
-        if (secured) {
-            intent.action = MullvadVpnService.KEY_DISCONNECT_ACTION
-            startService(intent)
-        } else {
-            intent.action = MullvadVpnService.KEY_CONNECT_ACTION
-            startForegroundService(intent)
+    private fun toggleTunnel() {
+        val intent = Intent(this, MullvadVpnService::class.java).apply {
+            action = if (qsTile.state == Tile.STATE_INACTIVE) {
+                MullvadVpnService.KEY_CONNECT_ACTION
+            } else {
+                MullvadVpnService.KEY_DISCONNECT_ACTION
+            }
         }
-    }
 
-    override fun onStopListening() {
-        scope.cancel()
-        super.onStopListening()
+        // Always start as foreground in case tile is out-of-sync.
+        startForegroundService(intent)
     }
 
     @OptIn(FlowPreview::class)
@@ -68,31 +90,44 @@ class MullvadTileService : TileService() {
         ServiceConnection(this@MullvadTileService, scope)
             .tunnelState
             .debounce(300L)
-            .collect { updateTunnelState(it.first, it.second) }
+            .map { (tunnelState, connectionState) -> mapToTileState(tunnelState, connectionState) }
+            .collect {
+                updateTileState(it)
+            }
     }
 
-    private fun updateTunnelState(
+    private fun mapToTileState(
         tunnelState: TunnelState,
         connectionState: ServiceResult.ConnectionState
-    ) {
-        secured = if (connectionState == ServiceResult.ConnectionState.CONNECTED) {
+    ): Int {
+        return if (connectionState == ServiceResult.ConnectionState.CONNECTED) {
             when (tunnelState) {
-                is TunnelState.Disconnected -> false
-                is TunnelState.Connecting -> true
-                is TunnelState.Connected -> true
+                is TunnelState.Disconnected -> Tile.STATE_INACTIVE
+                is TunnelState.Connecting -> Tile.STATE_ACTIVE
+                is TunnelState.Connected -> Tile.STATE_ACTIVE
                 is TunnelState.Disconnecting -> {
-                    tunnelState.actionAfterDisconnect == ActionAfterDisconnect.Reconnect
+                    if (tunnelState.actionAfterDisconnect == ActionAfterDisconnect.Reconnect) {
+                        Tile.STATE_ACTIVE
+                    } else {
+                        Tile.STATE_INACTIVE
+                    }
+                }
+                is TunnelState.Error -> {
+                    if (tunnelState.errorState.isBlocking) {
+                        Tile.STATE_ACTIVE
+                    } else {
+                        Tile.STATE_INACTIVE
+                    }
                 }
-                is TunnelState.Error -> tunnelState.errorState.isBlocking
             }
         } else {
-            false
+            Tile.STATE_INACTIVE
         }
     }
 
-    private fun updateTileState() {
+    private fun updateTileState(newState: Int) {
         qsTile?.apply {
-            if (secured) {
+            if (newState == Tile.STATE_ACTIVE) {
                 state = Tile.STATE_ACTIVE
                 icon = securedIcon
 
@@ -107,7 +142,6 @@ class MullvadTileService : TileService() {
                     subtitle = resources.getText(R.string.unsecured)
                 }
             }
-
             updateTile()
         }
     }
diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadVpnService.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadVpnService.kt
index 9ff1e9f6a5..0a6c29b44e 100644
--- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadVpnService.kt
+++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadVpnService.kt
@@ -114,6 +114,12 @@ class MullvadVpnService : TalpidVpnService() {
         val startResult = super.onStartCommand(intent, flags, startId)
         var quitCommand = false
 
+        // Always promote to foreground if connect/disconnect actions are provided to mitigate cases
+        // where the service would potentially otherwise be too slow running `startForeground`.
+        if (intent?.action == KEY_CONNECT_ACTION || intent?.action == KEY_DISCONNECT_ACTION) {
+            notificationManager.showOnForeground()
+        }
+
         notificationManager.updateNotification()
 
         if (!keyguardManager.isDeviceLocked) {