fix: Crash on starting foreground service for calling (WPB-11112) (#3464

)
wireapp · Sep 25, 2024 · 7c0cc88 · 7c0cc88
1 parent 456653a
commit 7c0cc88
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 15 deletions.
diff --git a/app/src/main/kotlin/com/wire/android/services/CallService.kt b/app/src/main/kotlin/com/wire/android/services/CallService.kt
@@ -45,6 +45,7 @@ import kotlinx.coroutines.SupervisorJob
 import kotlinx.coroutines.cancel
 import kotlinx.coroutines.flow.collectLatest
 import kotlinx.coroutines.flow.combine
+import kotlinx.coroutines.flow.debounce
 import kotlinx.coroutines.flow.distinctUntilChanged
 import kotlinx.coroutines.flow.flatMapLatest
 import kotlinx.coroutines.flow.flowOf
@@ -121,6 +122,8 @@ class CallService : Service() {
                     .distinctUntilChanged()
                     .flatMapRight { callData ->
                         callNotificationManager.reloadIfNeeded(callData)
+                    }.debounce {
+                        if (it is Either.Left) ServicesManager.DEBOUNCE_TIME else 0L
                     }
                     .collectLatest {
                         it.fold(

diff --git a/app/src/main/kotlin/com/wire/android/services/ServicesManager.kt b/app/src/main/kotlin/com/wire/android/services/ServicesManager.kt
@@ -52,7 +52,7 @@ class ServicesManager @Inject constructor(
     init {
         scope.launch {
             callServiceEvents
-                .debounce { if (!it) 0L else DEBOUNCE_TIME } // debounce to avoid starting and stopping service too fast
+                .debounce { if (it) 0L else DEBOUNCE_TIME } // debounce to avoid starting and stopping service too fast
                 .distinctUntilChanged()
                 .collectLatest { shouldBeStarted ->
                     if (!shouldBeStarted) {
@@ -127,6 +127,6 @@ class ServicesManager @Inject constructor(
 
     companion object {
         @VisibleForTesting
-        internal const val DEBOUNCE_TIME = 200L
+        const val DEBOUNCE_TIME = 500L
     }
 }
diff --git a/app/src/test/kotlin/com/wire/android/services/ServicesManagerTest.kt b/app/src/test/kotlin/com/wire/android/services/ServicesManagerTest.kt
@@ -39,19 +39,35 @@ class ServicesManagerTest {
     val dispatcherProvider = TestDispatcherProvider()
 
     @Test
-    fun `given ongoing call service running, when stop comes instantly after start, then do not even start the service`() =
+    fun `given ongoing call service running, when stop comes instantly after start, then start the service and stop it after a while`() =
         runTest(dispatcherProvider.main()) {
             // given
             val (arrangement, servicesManager) = Arrangement()
                 .withServiceState(CallService.ServiceState.FOREGROUND)
                 .arrange()
             // when
             servicesManager.startCallService()
-            advanceTimeBy((ServicesManager.DEBOUNCE_TIME - 50).milliseconds)
             servicesManager.stopCallService()
             // then
-            verify(exactly = 0) { arrangement.context.startService(arrangement.ongoingCallServiceIntent) }
-            verify(exactly = 1) { arrangement.context.stopService(arrangement.ongoingCallServiceIntent) }
+            verify(exactly = 1) { arrangement.context.startService(arrangement.callServiceIntent) }
+            advanceUntilIdle()
+            verify(exactly = 1) { arrangement.context.stopService(arrangement.callServiceIntent) }
+        }
+
+    @Test
+    fun `given call service running, when start comes instantly after stop, then do not stop the service`() =
+        runTest(dispatcherProvider.main()) {
+            // given
+            val (arrangement, servicesManager) = Arrangement()
+                .withServiceState(CallService.ServiceState.FOREGROUND)
+                .arrange()
+            servicesManager.startCallService()
+            // when
+            servicesManager.stopCallService()
+            servicesManager.startCallService()
+            // then
+            verify(exactly = 1) { arrangement.context.startService(arrangement.callServiceIntent) }
+            verify(exactly = 0) { arrangement.context.stopService(arrangement.callServiceIntent) }
         }
 
     @Test
@@ -67,8 +83,9 @@ class ServicesManagerTest {
             advanceTimeBy((ServicesManager.DEBOUNCE_TIME + 50).milliseconds)
             servicesManager.stopCallService()
             // then
-            verify(exactly = 1) { arrangement.context.startService(arrangement.ongoingCallServiceIntent) }
-            verify(exactly = 1) { arrangement.context.stopService(arrangement.ongoingCallServiceIntent) }
+            verify(exactly = 1) { arrangement.context.startService(arrangement.callServiceIntent) }
+            advanceUntilIdle()
+            verify(exactly = 1) { arrangement.context.stopService(arrangement.callServiceIntent) }
         }
 
     @Test
@@ -79,13 +96,13 @@ class ServicesManagerTest {
                 .withServiceState(CallService.ServiceState.FOREGROUND)
                 .arrange()
             servicesManager.startCallService()
-            advanceUntilIdle()
             arrangement.clearRecordedCallsForContext() // clear calls recorded when initializing the state
             // when
             servicesManager.stopCallService()
+            advanceUntilIdle()
             // then
             verify(exactly = 0) { arrangement.context.startService(arrangement.ongoingCallServiceIntentWithStopArgument) }
-            verify(exactly = 1) { arrangement.context.stopService(arrangement.ongoingCallServiceIntent) }
+            verify(exactly = 1) { arrangement.context.stopService(arrangement.callServiceIntent) }
         }
 
     @Test
@@ -96,13 +113,13 @@ class ServicesManagerTest {
                 .withServiceState(CallService.ServiceState.STARTED)
                 .arrange()
             servicesManager.startCallService()
-            advanceUntilIdle()
             arrangement.clearRecordedCallsForContext() // clear calls recorded when initializing the state
             // when
             servicesManager.stopCallService()
+            advanceUntilIdle()
             // then
             verify(exactly = 1) { arrangement.context.startService(arrangement.ongoingCallServiceIntentWithStopArgument) }
-            verify(exactly = 0) { arrangement.context.stopService(arrangement.ongoingCallServiceIntent) }
+            verify(exactly = 0) { arrangement.context.stopService(arrangement.callServiceIntent) }
         }
 
     @Test
@@ -119,7 +136,7 @@ class ServicesManagerTest {
             servicesManager.startCallService()
             // then
             verify(exactly = 0) { arrangement.context.startService(arrangement.ongoingCallServiceIntentWithStopArgument) }
-            verify(exactly = 0) { arrangement.context.stopService(arrangement.ongoingCallServiceIntent) }
+            verify(exactly = 0) { arrangement.context.stopService(arrangement.callServiceIntent) }
         }
 
     private inner class Arrangement {
@@ -130,15 +147,15 @@ class ServicesManagerTest {
         private val servicesManager: ServicesManager by lazy { ServicesManager(context, dispatcherProvider) }
 
         @MockK
-        lateinit var ongoingCallServiceIntent: Intent
+        lateinit var callServiceIntent: Intent
 
         @MockK
         lateinit var ongoingCallServiceIntentWithStopArgument: Intent
 
         init {
             MockKAnnotations.init(this, relaxUnitFun = true)
             mockkObject(CallService.Companion)
-            every { CallService.Companion.newIntent(context) } returns ongoingCallServiceIntent
+            every { CallService.Companion.newIntent(context) } returns callServiceIntent
             every { CallService.Companion.newIntentToStop(context) } returns ongoingCallServiceIntentWithStopArgument
         }