Merge branch 'fix-android-multi-process-lifecycle'

9 files changed, 96 insertions, 69 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index fea6f8d5c7..ca4da7be88 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -27,6 +27,15 @@ Line wrap the file at 100 chars.                                              Th
 - Keep unspecified constraints unchanged in the CLI when providing specific tunnel constraints
   instead of setting them to default values.
 
+#### Android
+- Avoid running in foreground when not connected.
+- Avoid removing notification when service is stopped.
+- Change so that swiping the notification no longer kills the service since that isn't a common way of handling the 
+  lifecycle in Android. Instead rely on the following mechanisms to kill the service:
+  * Swiping to remove from the Recents/Overview screen.
+  * Android Background Execution Limits.
+  * The System Settings way of killing apps ("Force Stop").
+
 ### Fixed
 - Always kill `sslocal` if the tunnel monitor fails to start when using bridges.
 
@@ -35,6 +44,8 @@ Line wrap the file at 100 chars.                                              Th
 - Fix daemon not starting if all excluded app paths reside on non-existent/unmounted volumes.
 - Remove tray icon of current running app version when upgrading.
 
+#### Android
+- Fix Quick Settings tile showing wrong state in certain scenarios.
 
 ## [2021.6] - 2021-11-17
 ### Fixed
diff --git a/android/src/main/kotlin/net/mullvad/mullvadvpn/ipc/ServiceConnection.kt b/android/src/main/kotlin/net/mullvad/mullvadvpn/ipc/ServiceConnection.kt
index 751bcc9bc3..66ac88c91d 100644
--- a/android/src/main/kotlin/net/mullvad/mullvadvpn/ipc/ServiceConnection.kt
+++ b/android/src/main/kotlin/net/mullvad/mullvadvpn/ipc/ServiceConnection.kt
@@ -7,21 +7,30 @@ import android.os.Looper
 import android.os.Messenger
 import kotlin.reflect.KClass
 import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.FlowPreview
+import kotlinx.coroutines.channels.Channel
+import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.SharingStarted
 import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.collect
+import kotlinx.coroutines.flow.combine
+import kotlinx.coroutines.flow.consumeAsFlow
 import kotlinx.coroutines.flow.filterNotNull
 import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.flow.onCompletion
+import kotlinx.coroutines.flow.onEach
+import kotlinx.coroutines.flow.onStart
 import kotlinx.coroutines.flow.stateIn
 import kotlinx.coroutines.launch
+import net.mullvad.mullvadvpn.model.ServiceResult
 import net.mullvad.mullvadvpn.model.TunnelState
 import net.mullvad.mullvadvpn.service.MullvadVpnService
 import net.mullvad.mullvadvpn.util.DispatchingFlow
 import net.mullvad.mullvadvpn.util.bindServiceFlow
 import net.mullvad.mullvadvpn.util.dispatchTo
 
+@FlowPreview
 class ServiceConnection(context: Context, scope: CoroutineScope) {
     private val activeListeners = MutableStateFlow<Pair<Messenger, Int>?>(null)
     private val handler = HandlerFlow(Looper.getMainLooper(), Event::fromMessage)
@@ -30,9 +39,12 @@ class ServiceConnection(context: Context, scope: CoroutineScope) {
 
     private lateinit var listenerRegistrations: StateFlow<Pair<Messenger, Int>?>
 
-    lateinit var tunnelState: StateFlow<TunnelState>
+    lateinit var tunnelState: Flow<Pair<TunnelState, ServiceResult.ConnectionState>>
         private set
 
+    private val serviceConnectionStateChannel =
+        Channel<ServiceResult.ConnectionState>(Channel.RENDEZVOUS)
+
     init {
         val dispatcher = handler
             .filterNotNull()
@@ -41,11 +53,18 @@ class ServiceConnection(context: Context, scope: CoroutineScope) {
                     Pair(connection, listenerId)
                 }
 
-                tunnelState = subscribeToState(
+                val tunnelStateEvents = subscribeToState(
                     Event.TunnelStateChange::class,
                     scope,
                     TunnelState.Disconnected
                 ) { tunnelState }
+
+                tunnelState = tunnelStateEvents
+                    .combine(
+                        serviceConnectionStateChannel.consumeAsFlow()
+                    ) { tunnelState, serviceConnectionState ->
+                        tunnelState to serviceConnectionState
+                    }
             }
 
         scope.launch { connect(context) }
@@ -57,10 +76,14 @@ class ServiceConnection(context: Context, scope: CoroutineScope) {
     private suspend fun connect(context: Context) {
         val intent = Intent(context, MullvadVpnService::class.java)
 
-        context.bindServiceFlow(intent).collect { binder ->
-            activeListeners.value = null
-            binder?.let(::registerListener)
-        }
+        context
+            .bindServiceFlow(intent)
+            .onStart { emit(ServiceResult.NOT_CONNECTED) }
+            .onEach { result -> serviceConnectionStateChannel.send(result.connectionState) }
+            .collect { result ->
+                activeListeners.value = null
+                result.binder?.let(::registerListener)
+            }
     }
 
     private fun registerListener(binder: IBinder) {
diff --git a/android/src/main/kotlin/net/mullvad/mullvadvpn/model/ServiceResult.kt b/android/src/main/kotlin/net/mullvad/mullvadvpn/model/ServiceResult.kt
new file mode 100644
index 0000000000..b1d9f5be4c
--- /dev/null
+++ b/android/src/main/kotlin/net/mullvad/mullvadvpn/model/ServiceResult.kt
@@ -0,0 +1,25 @@
+package net.mullvad.mullvadvpn.model
+
+import android.os.IBinder
+
+data class ServiceResult(
+    val binder: IBinder?
+) {
+    enum class ConnectionState {
+        CONNECTED,
+        DISCONNECTED;
+    }
+
+    val connectionState: ConnectionState
+        get() {
+            return if (binder == null) {
+                ConnectionState.DISCONNECTED
+            } else {
+                ConnectionState.CONNECTED
+            }
+        }
+
+    companion object {
+        val NOT_CONNECTED = ServiceResult(null)
+    }
+}
diff --git a/android/src/main/kotlin/net/mullvad/mullvadvpn/service/ForegroundNotificationManager.kt b/android/src/main/kotlin/net/mullvad/mullvadvpn/service/ForegroundNotificationManager.kt
index c3e37d6115..92c4fcd078 100644
--- a/android/src/main/kotlin/net/mullvad/mullvadvpn/service/ForegroundNotificationManager.kt
+++ b/android/src/main/kotlin/net/mullvad/mullvadvpn/service/ForegroundNotificationManager.kt
@@ -92,19 +92,6 @@ class ForegroundNotificationManager(
         service.unregisterReceiver(deviceLockListener)
 
         updater.close()
-
-        tunnelStateNotification.visible = false
-    }
-
-    fun acknowledgeStartForegroundService() {
-        // When sending start commands to the service, it is necessary to request the service to be
-        // on the foreground. With such request, when the service is started it must be placed on
-        // the foreground with a call to startForeground before a timeout expires, otherwise Android
-        // kills the app.
-        showOnForeground()
-
-        // Restore the notification to its correct state.
-        updateNotification()
     }
 
     private fun runUpdater() = GlobalScope.actor<UpdaterMessage>(
@@ -132,7 +119,7 @@ class ForegroundNotificationManager(
         onForeground = true
     }
 
-    private fun updateNotification() {
+    fun updateNotification() {
         if (shouldBeOnForeground != onForeground) {
             if (shouldBeOnForeground) {
                 showOnForeground()
diff --git a/android/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadTileService.kt b/android/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadTileService.kt
index 8045dfbe06..4c15559912 100644
--- a/android/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadTileService.kt
+++ b/android/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadTileService.kt
@@ -14,14 +14,13 @@ import kotlinx.coroutines.flow.debounce
 import kotlinx.coroutines.launch
 import net.mullvad.mullvadvpn.R
 import net.mullvad.mullvadvpn.ipc.ServiceConnection
+import net.mullvad.mullvadvpn.model.ServiceResult
 import net.mullvad.mullvadvpn.model.TunnelState
 import net.mullvad.talpid.tunnel.ActionAfterDisconnect
 
 class MullvadTileService : TileService() {
-    private var secured by observable(false) { _, wasSecured, isSecured ->
-        if (wasSecured != isSecured) {
-            updateTileState()
-        }
+    private var secured by observable(false) { _, _, _ ->
+        updateTileState()
     }
 
     private lateinit var scope: CoroutineScope
@@ -51,11 +50,11 @@ class MullvadTileService : TileService() {
 
         if (secured) {
             intent.action = MullvadVpnService.KEY_DISCONNECT_ACTION
+            startService(intent)
         } else {
             intent.action = MullvadVpnService.KEY_CONNECT_ACTION
+            startForegroundService(intent)
         }
-
-        startForegroundService(intent)
     }
 
     override fun onStopListening() {
@@ -68,18 +67,25 @@ class MullvadTileService : TileService() {
         ServiceConnection(this@MullvadTileService, scope)
             .tunnelState
             .debounce(300L)
-            .collect(::updateTunnelState)
+            .collect { updateTunnelState(it.first, it.second) }
     }
 
-    private fun updateTunnelState(tunnelState: TunnelState) {
-        secured = when (tunnelState) {
-            is TunnelState.Disconnected -> false
-            is TunnelState.Connecting -> true
-            is TunnelState.Connected -> true
-            is TunnelState.Disconnecting -> {
-                tunnelState.actionAfterDisconnect == ActionAfterDisconnect.Reconnect
+    private fun updateTunnelState(
+        tunnelState: TunnelState,
+        connectionState: ServiceResult.ConnectionState
+    ) {
+        secured = if (connectionState == ServiceResult.ConnectionState.CONNECTED) {
+            when (tunnelState) {
+                is TunnelState.Disconnected -> false
+                is TunnelState.Connecting -> true
+                is TunnelState.Connected -> true
+                is TunnelState.Disconnecting -> {
+                    tunnelState.actionAfterDisconnect == ActionAfterDisconnect.Reconnect
+                }
+                is TunnelState.Error -> tunnelState.errorState.isBlocking
             }
-            is TunnelState.Error -> tunnelState.errorState.isBlocking
+        } else {
+            false
         }
     }
 
diff --git a/android/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadVpnService.kt b/android/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadVpnService.kt
index 177db1d712..71067c44d7 100644
--- a/android/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadVpnService.kt
+++ b/android/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadVpnService.kt
@@ -61,14 +61,6 @@ class MullvadVpnService : TalpidVpnService() {
         }
     }
 
-    private var isBound: Boolean by observable(false) { _, _, isBound ->
-        notificationManager.lockedToForeground = isUiVisible or isBound
-    }
-
-    private var isUiVisible: Boolean by observable(false) { _, _, isUiVisible ->
-        notificationManager.lockedToForeground = isUiVisible or isBound
-    }
-
     override fun onCreate() {
         super.onCreate()
         Log.d(TAG, "Initializing service")
@@ -91,7 +83,6 @@ class MullvadVpnService : TalpidVpnService() {
 
         notificationManager =
             ForegroundNotificationManager(this, connectionProxy, keyguardManager).apply {
-                acknowledgeStartForegroundService()
                 accountNumberEvents = endpoint.settingsListener.accountNumberNotifier
             }
 
@@ -121,7 +112,7 @@ class MullvadVpnService : TalpidVpnService() {
         val startResult = super.onStartCommand(intent, flags, startId)
         var quitCommand = false
 
-        notificationManager.acknowledgeStartForegroundService()
+        notificationManager.updateNotification()
 
         if (!keyguardManager.isDeviceLocked) {
             val action = intent?.action
@@ -145,15 +136,11 @@ class MullvadVpnService : TalpidVpnService() {
 
     override fun onBind(intent: Intent): IBinder {
         Log.d(TAG, "New connection to service")
-        isBound = true
-
         return super.onBind(intent) ?: endpoint.messenger.binder
     }
 
     override fun onRebind(intent: Intent) {
         Log.d(TAG, "Connection to service restored")
-        isBound = true
-
         if (state == State.Stopping) {
             restart()
         }
@@ -165,7 +152,6 @@ class MullvadVpnService : TalpidVpnService() {
 
     override fun onUnbind(intent: Intent): Boolean {
         Log.d(TAG, "Closed all connections to service")
-        isBound = false
 
         if (state != State.Running) {
             stop()
diff --git a/android/src/main/kotlin/net/mullvad/mullvadvpn/service/notifications/TunnelStateNotification.kt b/android/src/main/kotlin/net/mullvad/mullvadvpn/service/notifications/TunnelStateNotification.kt
index d59f4dec28..10f929ab97 100644
--- a/android/src/main/kotlin/net/mullvad/mullvadvpn/service/notifications/TunnelStateNotification.kt
+++ b/android/src/main/kotlin/net/mullvad/mullvadvpn/service/notifications/TunnelStateNotification.kt
@@ -9,7 +9,6 @@ import androidx.core.app.NotificationCompat
 import kotlin.properties.Delegates.observable
 import net.mullvad.mullvadvpn.R
 import net.mullvad.mullvadvpn.model.TunnelState
-import net.mullvad.mullvadvpn.service.MullvadVpnService
 import net.mullvad.mullvadvpn.ui.MainActivity
 import net.mullvad.talpid.tunnel.ActionAfterDisconnect
 
@@ -90,15 +89,13 @@ class TunnelStateNotification(val context: Context) {
         val pendingIntent =
             PendingIntent.getActivity(context, 1, intent, PendingIntent.FLAG_UPDATE_CURRENT)
 
-        val deleteIntent = buildDeleteIntent()
-
         val actions = if (showAction) {
             listOf(buildAction())
         } else {
             emptyList()
         }
 
-        return channel.buildNotification(pendingIntent, notificationText, actions, deleteIntent)
+        return channel.buildNotification(pendingIntent, notificationText, actions)
     }
 
     private fun buildAction(): NotificationCompat.Action {
@@ -112,10 +109,4 @@ class TunnelStateNotification(val context: Context) {
 
         return NotificationCompat.Action(action.icon, label, pendingIntent)
     }
-
-    private fun buildDeleteIntent(): PendingIntent {
-        val intent = Intent(MullvadVpnService.KEY_QUIT_ACTION).setPackage("net.mullvad.mullvadvpn")
-        val flags = PendingIntent.FLAG_UPDATE_CURRENT
-        return PendingIntent.getForegroundService(context, 1, intent, flags)
-    }
 }
diff --git a/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/MainActivity.kt b/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/MainActivity.kt
index 69db9c1243..dfc541d8d5 100644
--- a/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/MainActivity.kt
+++ b/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/MainActivity.kt
@@ -26,7 +26,6 @@ open class MainActivity : FragmentActivity() {
     val problemReport = MullvadProblemReport()
     val serviceNotifier = EventNotifier<ServiceConnection?>(null)
 
-    private var isUiVisible = false
     private var visibleSecureScreens = HashSet<Fragment>()
 
     private val deviceIsTv by lazy {
@@ -87,11 +86,9 @@ open class MainActivity : FragmentActivity() {
         android.util.Log.d("mullvad", "Starting main activity")
         super.onStart()
 
-        isUiVisible = true
-
         val intent = Intent(this, MullvadVpnService::class.java)
 
-        startForegroundService(intent)
+        startService(intent)
         bindService(intent, serviceConnectionManager, 0)
     }
 
@@ -109,7 +106,6 @@ open class MainActivity : FragmentActivity() {
 
     override fun onStop() {
         android.util.Log.d("mullvad", "Stoping main activity")
-        isUiVisible = false
         unbindService(serviceConnectionManager)
 
         super.onStop()
diff --git a/android/src/main/kotlin/net/mullvad/mullvadvpn/util/FlowUtils.kt b/android/src/main/kotlin/net/mullvad/mullvadvpn/util/FlowUtils.kt
index 71ce51a005..588f2ecfd1 100644
--- a/android/src/main/kotlin/net/mullvad/mullvadvpn/util/FlowUtils.kt
+++ b/android/src/main/kotlin/net/mullvad/mullvadvpn/util/FlowUtils.kt
@@ -13,6 +13,7 @@ import kotlinx.coroutines.channels.awaitClose
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.callbackFlow
 import kotlinx.coroutines.flow.take
+import net.mullvad.mullvadvpn.model.ServiceResult
 
 fun <T> SendChannel<T>.safeOffer(element: T): Boolean {
     return runCatching { offer(element) }.getOrDefault(false)
@@ -32,21 +33,22 @@ fun Animation.transitionFinished(): Flow<Unit> = callbackFlow<Unit> {
     }
 }.take(1)
 
-fun Context.bindServiceFlow(intent: Intent, flags: Int = 0): Flow<IBinder?> = callbackFlow {
+fun Context.bindServiceFlow(intent: Intent, flags: Int = 0): Flow<ServiceResult> = callbackFlow {
     val connectionCallback = object : ServiceConnection {
         override fun onServiceConnected(className: ComponentName, binder: IBinder) {
-            safeOffer(binder)
+            safeOffer(ServiceResult(binder))
         }
 
         override fun onServiceDisconnected(className: ComponentName) {
-            safeOffer(null)
+            safeOffer(ServiceResult.NOT_CONNECTED)
+            bindService(intent, this, flags)
         }
     }
 
     bindService(intent, connectionCallback, flags)
 
     awaitClose {
-        safeOffer(null)
+        safeOffer(ServiceResult.NOT_CONNECTED)
 
         Dispatchers.Default.dispatch(EmptyCoroutineContext) {
             unbindService(connectionCallback)