Fix bind not being invoked

Author: Rawa

Diff for: android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadVpnService.kt

@@ -12,11 +12,8 @@ import arrow.atomic.AtomicInt
 import co.touchlab.kermit.Logger
 import java.io.File
 import kotlinx.coroutines.Dispatchers
-import kotlinx.coroutines.flow.MutableStateFlow
-import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.filter
 import kotlinx.coroutines.flow.first
-import kotlinx.coroutines.flow.update
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.runBlocking
 import net.mullvad.mullvadvpn.lib.common.constant.BuildTypes
@@ -35,17 +32,14 @@ import net.mullvad.mullvadvpn.service.migration.MigrateSplitTunneling
 import net.mullvad.mullvadvpn.service.notifications.ForegroundNotificationManager
 import net.mullvad.mullvadvpn.service.notifications.NotificationChannelFactory
 import net.mullvad.mullvadvpn.service.notifications.NotificationManager
-import net.mullvad.mullvadvpn.service.notifications.ShouldBeOnForegroundProvider
 import net.mullvad.talpid.TalpidVpnService
 import org.koin.android.ext.android.getKoin
 import org.koin.core.context.loadKoinModules
 import org.koin.core.qualifier.named
 
 private const val RELAYS_FILE = "relays.json"
 
-class MullvadVpnService : TalpidVpnService(), ShouldBeOnForegroundProvider {
-    private val _shouldBeOnForeground = MutableStateFlow(false)
-    override val shouldBeOnForeground: StateFlow<Boolean> = _shouldBeOnForeground
+class MullvadVpnService : TalpidVpnService() {
 
     private lateinit var keyguardManager: KeyguardManager
 
@@ -74,7 +68,7 @@ class MullvadVpnService : TalpidVpnService(), ShouldBeOnForegroundProvider {
             managementService = get()
 
             foregroundNotificationHandler =
-                ForegroundNotificationManager(this@MullvadVpnService, get(), lifecycleScope)
+                ForegroundNotificationManager(this@MullvadVpnService, get())
             get<NotificationManager>()
 
             apiEndpointConfiguration = get()
@@ -86,8 +80,6 @@ class MullvadVpnService : TalpidVpnService(), ShouldBeOnForegroundProvider {
 
         keyguardManager = getSystemService<KeyguardManager>()!!
 
-        lifecycleScope.launch { foregroundNotificationHandler.start(this@MullvadVpnService) }
-
         // TODO We should avoid lifecycleScope.launch (current needed due to InetSocketAddress
         // with intent from API)
         lifecycleScope.launch(context = Dispatchers.IO) {
@@ -118,7 +110,7 @@ class MullvadVpnService : TalpidVpnService(), ShouldBeOnForegroundProvider {
             intent.isFromSystem() || intent?.action == KEY_CONNECT_ACTION -> {
                 // Only show on foreground if we have permission
                 if (prepare(this) == null) {
-                    _shouldBeOnForeground.update { true }
+                    foregroundNotificationHandler.startForeground()
                 }
                 lifecycleScope.launch { connectionProxy.connectWithoutPermissionCheck() }
             }
@@ -131,12 +123,12 @@ class MullvadVpnService : TalpidVpnService(), ShouldBeOnForegroundProvider {
     }
 
     override fun onBind(intent: Intent?): IBinder {
-        bindCount.incrementAndGet()
-        Logger.i("onBind: $intent")
+        val count = bindCount.incrementAndGet()
+        Logger.i("onBind: $intent, bindCount: $count")
 
         if (intent.isFromSystem()) {
-            Logger.i("onBind from system")
-            _shouldBeOnForeground.update { true }
+            Logger.i("onBind was from system")
+            foregroundNotificationHandler.startForeground()
         }
 
         // We always need to return a binder. If the system binds to our VPN service, VpnService
@@ -145,6 +137,17 @@ class MullvadVpnService : TalpidVpnService(), ShouldBeOnForegroundProvider {
         return super.onBind(intent) ?: emptyBinder()
     }
 
+    override fun onRebind(intent: Intent?) {
+        super.onRebind(intent)
+        val count = bindCount.incrementAndGet()
+        Logger.i("onRebind: $intent, bindCount: $count")
+
+        if (intent.isFromSystem()) {
+            Logger.i("onRebind from system")
+            foregroundNotificationHandler.startForeground()
+        }
+    }
+
     private fun startDaemon() {
         val apiEndpointConfiguration =
             if (Build.TYPE == BuildTypes.DEBUG) {
@@ -172,16 +175,18 @@ class MullvadVpnService : TalpidVpnService(), ShouldBeOnForegroundProvider {
         }
 
     override fun onRevoke() {
+        Logger.d("onRevoke")
         runBlocking { connectionProxy.disconnect() }
     }
 
     override fun onUnbind(intent: Intent): Boolean {
         val count = bindCount.decrementAndGet()
+        Logger.i("onUnbind: $intent, bindCount: $count")
 
         // Foreground?
         if (intent.isFromSystem()) {
             Logger.i("onUnbind from system")
-            _shouldBeOnForeground.update { false }
+            foregroundNotificationHandler.stopForeground()
         }
 
         if (count == 0) {
@@ -203,7 +208,7 @@ class MullvadVpnService : TalpidVpnService(), ShouldBeOnForegroundProvider {
                 }
             }
         }
-        return false
+        return true
     }
 
     override fun onDestroy() {

Diff for: android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/notifications/ForegroundNotificationManager.kt

@@ -5,8 +5,6 @@ import android.content.pm.ServiceInfo
 import android.net.VpnService
 import android.os.Build
 import co.touchlab.kermit.Logger
-import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.launch
 import net.mullvad.mullvadvpn.lib.model.Notification
 import net.mullvad.mullvadvpn.lib.model.NotificationChannel
 import net.mullvad.mullvadvpn.lib.model.NotificationTunnelState
@@ -18,20 +16,15 @@ import net.mullvad.mullvadvpn.service.notifications.tunnelstate.toNotification
 class ForegroundNotificationManager(
     private val vpnService: MullvadVpnService,
     private val tunnelStateNotificationProvider: TunnelStateNotificationProvider,
-    private val scope: CoroutineScope,
 ) {
-    suspend fun start(foregroundProvider: ShouldBeOnForegroundProvider) {
-        scope.launch {
-            foregroundProvider.shouldBeOnForeground.collect {
-                if (it) {
-                    Logger.i("Starting foreground")
-                    notifyForeground(getTunnelStateNotificationOrDefault())
-                } else {
-                    Logger.i("Stopping foreground")
-                    vpnService.stopForeground(Service.STOP_FOREGROUND_DETACH)
-                }
-            }
-        }
+    fun startForeground() {
+        Logger.d("startForeground")
+        notifyForeground(getTunnelStateNotificationOrDefault())
+    }
+
+    fun stopForeground() {
+        Logger.d("stopForeground")
+        vpnService.stopForeground(Service.STOP_FOREGROUND_DETACH)
     }
 
     private fun getTunnelStateNotificationOrDefault(): Notification.Tunnel {

Diff for: android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/notifications/ShouldBeOnForegroundProvider.kt

@@ -1,7 +1,7 @@
 package net.mullvad.mullvadvpn.service.notifications
 
-import kotlinx.coroutines.flow.StateFlow
+import kotlinx.coroutines.flow.Flow
 
 interface ShouldBeOnForegroundProvider {
-    val shouldBeOnForeground: StateFlow<Boolean>
+    val shouldBeOnForeground: Flow<Boolean>
 }

Diff for: android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/notifications/tunnelstate/TunnelStateNotificationProvider.kt

@@ -11,6 +11,7 @@ import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.flow.onStart
 import kotlinx.coroutines.flow.stateIn
 import net.mullvad.mullvadvpn.lib.model.ActionAfterDisconnect
+import net.mullvad.mullvadvpn.lib.model.DeviceState
 import net.mullvad.mullvadvpn.lib.model.ErrorStateCause
 import net.mullvad.mullvadvpn.lib.model.Notification
 import net.mullvad.mullvadvpn.lib.model.NotificationAction
@@ -40,6 +41,11 @@ class TunnelStateNotificationProvider(
                 deviceRepository.deviceState
             ) { tunnelState: TunnelState, actionAfterDisconnect: ActionAfterDisconnect?, deviceState
                 ->
+                if (
+                    deviceState is DeviceState.LoggedOut && tunnelState is TunnelState.Disconnected
+                ) {
+                    return@combine NotificationUpdate.Cancel(notificationId)
+                }
                 val notificationTunnelState =
                     tunnelState(
                         tunnelState,