Merge branch 'toggletunnel-crash-in-tile-droid-1283'

4 files changed, 43 insertions, 29 deletions

diff --git a/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadVpnService.kt b/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadVpnService.kt
index 2b7833fb8d..2db5a00fdb 100644
--- a/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadVpnService.kt
+++ b/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadVpnService.kt
@@ -12,7 +12,7 @@ import arrow.atomic.AtomicInt
 import co.touchlab.kermit.Logger
 import java.io.File
 import kotlinx.coroutines.flow.filter
-import kotlinx.coroutines.flow.first
+import kotlinx.coroutines.flow.filterIsInstance
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.runBlocking
 import net.mullvad.mullvadvpn.lib.common.constant.KEY_CONNECT_ACTION
@@ -90,6 +90,15 @@ class MullvadVpnService : TalpidVpnService() {
 
         Logger.i("Start management service")
         managementService.start()
+
+        lifecycleScope.launch {
+            // If the service is started with a connect command and a non-blocking error occur (e.g.
+            // unable to start the tunnel) then the service is demoted from foreground.
+            managementService.tunnelState
+                .filterIsInstance<TunnelState.Error>()
+                .filter { !it.errorState.isBlocking }
+                .collect { foregroundNotificationHandler.stopForeground() }
+        }
     }
 
     override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int {
@@ -105,15 +114,24 @@ class MullvadVpnService : TalpidVpnService() {
             keyguardManager.isKeyguardLocked -> {
                 Logger.i("Keyguard is locked, ignoring command")
             }
+
             intent.isFromSystem() || intent?.action == KEY_CONNECT_ACTION -> {
-                // Only show on foreground if we have permission
-                if (prepare(this) == null) {
-                    foregroundNotificationHandler.startForeground()
-                }
+                foregroundNotificationHandler.startForeground()
                 lifecycleScope.launch { connectionProxy.connectWithoutPermissionCheck() }
             }
+
             intent?.action == KEY_DISCONNECT_ACTION -> {
+                // MullvadTileService might have launched this service with the expectancy of it
+                // being foreground, thus it must go into foreground to please the android system
+                // requirements.
+                foregroundNotificationHandler.startForeground()
                 lifecycleScope.launch { connectionProxy.disconnect() }
+
+                // If disconnect intent is received and no one is using this service, simply stop
+                // foreground and let system stop service when it deems it not to be necessary.
+                if (bindCount.get() == 0) {
+                    foregroundNotificationHandler.stopForeground()
+                }
             }
         }
 
@@ -180,25 +198,6 @@ class MullvadVpnService : TalpidVpnService() {
             foregroundNotificationHandler.stopForeground()
         }
 
-        if (count == 0) {
-            Logger.i("No one bound to the service, stopSelf()")
-            lifecycleScope.launch {
-                Logger.i("Waiting for disconnected state")
-                // TODO This needs reworking, we should not wait for the disconnected state, what we
-                // want is the notification of disconnected to go out before we start shutting down
-                connectionProxy.tunnelState
-                    .filter {
-                        it is TunnelState.Disconnected ||
-                            (it is TunnelState.Error && !it.errorState.isBlocking)
-                    }
-                    .first()
-
-                if (bindCount.get() == 0) {
-                    Logger.i("Stopping service")
-                    stopSelf()
-                }
-            }
-        }
         return true
     }
 
@@ -209,8 +208,10 @@ class MullvadVpnService : TalpidVpnService() {
 
         Logger.i("Shutdown MullvadDaemon")
         MullvadDaemon.shutdown()
+
         Logger.i("Enter Idle")
         managementService.enterIdle()
+
         Logger.i("Shutdown complete")
         super.onDestroy()
     }
diff --git a/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/notifications/NotificationManager.kt b/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/notifications/NotificationManager.kt
index 3d58e571fd..ed2d2f053a 100644
--- a/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/notifications/NotificationManager.kt
+++ b/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/notifications/NotificationManager.kt
@@ -5,7 +5,10 @@ import android.content.Context
 import android.content.pm.PackageManager
 import androidx.core.app.ActivityCompat
 import androidx.core.app.NotificationManagerCompat
+import kotlin.time.Duration.Companion.milliseconds
 import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.FlowPreview
+import kotlinx.coroutines.flow.debounce
 import kotlinx.coroutines.flow.merge
 import kotlinx.coroutines.launch
 import net.mullvad.mullvadvpn.lib.model.Notification
@@ -13,6 +16,7 @@ import net.mullvad.mullvadvpn.lib.model.NotificationUpdate
 import net.mullvad.mullvadvpn.service.notifications.accountexpiry.toNotification
 import net.mullvad.mullvadvpn.service.notifications.tunnelstate.toNotification
 
+@OptIn(FlowPreview::class)
 class NotificationManager(
     private val notificationManagerCompat: NotificationManagerCompat,
     notificationProviders: List<NotificationProvider<Notification>>,
@@ -23,7 +27,7 @@ class NotificationManager(
     init {
         scope.launch {
             notificationProviders
-                .map { it.notifications }
+                .map { it.notifications.debounce(NOTIFICATION_DEBOUNCE) }
                 .merge()
                 .collect { notificationUpdate ->
                     when (notificationUpdate) {
@@ -31,6 +35,7 @@ class NotificationManager(
                             notificationManagerCompat.cancel(
                                 notificationUpdate.notificationId.value
                             )
+
                         is NotificationUpdate.Notify -> {
                             val notification = notificationUpdate.value
                             val androidNotification = notification.toAndroidNotification(context)
@@ -56,4 +61,11 @@ class NotificationManager(
             is Notification.Tunnel -> toNotification(context)
             is Notification.AccountExpiry -> toNotification(context)
         }
+
+    companion object {
+        // According to testing we are only allowed to send 5 notifications per second at most,
+        // otherwise the system will start dropping them. To ensure we don't drop the latest
+        // notification debounce if we spam too much.
+        val NOTIFICATION_DEBOUNCE = 200.milliseconds
+    }
 }
diff --git a/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/notifications/tunnelstate/TunnelStateNotificationProvider.kt b/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/notifications/tunnelstate/TunnelStateNotificationProvider.kt
index 3ff3b07e66..2068f1adff 100644
--- a/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/notifications/tunnelstate/TunnelStateNotificationProvider.kt
+++ b/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/notifications/tunnelstate/TunnelStateNotificationProvider.kt
@@ -39,8 +39,7 @@ class TunnelStateNotificationProvider(
                 connectionProxy.tunnelState,
                 connectionProxy.tunnelState.actionAfterDisconnect().distinctUntilChanged(),
                 deviceRepository.deviceState,
-            ) { tunnelState: TunnelState, actionAfterDisconnect: ActionAfterDisconnect?, deviceState
-                ->
+            ) { tunnelState, actionAfterDisconnect, deviceState ->
                 if (
                     deviceState is DeviceState.LoggedOut && tunnelState is TunnelState.Disconnected
                 ) {
@@ -113,7 +112,8 @@ class TunnelStateNotificationProvider(
     ): NotificationTunnelState.Error {
         val cause = errorState.cause
         return when {
-            cause is ErrorStateCause.IsOffline -> NotificationTunnelState.Error.DeviceOffline
+            cause is ErrorStateCause.IsOffline && errorState.isBlocking ->
+                NotificationTunnelState.Error.DeviceOffline
             cause is ErrorStateCause.InvalidDnsServers -> NotificationTunnelState.Error.Blocking
             cause is ErrorStateCause.VpnPermissionDenied ->
                 alwaysOnVpnPermissionName?.let { NotificationTunnelState.Error.AlwaysOnVpn }
diff --git a/android/tile/src/main/kotlin/net/mullvad/mullvadvpn/tile/MullvadTileService.kt b/android/tile/src/main/kotlin/net/mullvad/mullvadvpn/tile/MullvadTileService.kt
index 8e1d9e10ed..5aab152b72 100644
--- a/android/tile/src/main/kotlin/net/mullvad/mullvadvpn/tile/MullvadTileService.kt
+++ b/android/tile/src/main/kotlin/net/mullvad/mullvadvpn/tile/MullvadTileService.kt
@@ -118,7 +118,8 @@ class MullvadTileService : TileService() {
                     }
             }
 
-        // Always start as foreground in case tile is out-of-sync.
+        // Always start as foreground, e.g if app is dead we won't be allowed to start if not
+        // in foreground.
         startForegroundService(intent)
     }