Fix foreground not starting in time

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()
     }