Improve Foreground Service orchestration when transitioning from outgoing call to active call#1627
Merged
aleksandar-apostolov merged 11 commits intodevelopfrom Apr 1, 2026
Conversation
… if permissions are missing when making outgoing call
Contributor
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
Contributor
SDK Size Comparison 📏
|
rahul-lohra
changed the title
rahul-lohra
changed the title
rahul-lohra
changed the title
rahul-lohra
changed the title
WalkthroughThe changes implement a permission-based UI flow for foreground services in video calls. A new permission handling system checks required permissions before call initialization, shows conditional UI for permission requests, and updates service startup logic to verify service running state rather than automatically starting foreground services. Capabilities are now set based on available foreground permissions. Changes
Sequence DiagramsequenceDiagram
participant User
participant StreamCallActivity
participant PermissionUI as Permission UI Flow
participant PermissionManager
participant ServiceChecker
participant Dispatcher as Notification Dispatcher
User->>StreamCallActivity: Start Call
StreamCallActivity->>PermissionManager: Check required foreground permissions
alt Permissions Missing
PermissionManager-->>StreamCallActivity: Missing permissions
StreamCallActivity->>PermissionUI: Show permission request UI
PermissionUI->>User: Request foreground permissions
User->>PermissionUI: Grant/Deny permissions
PermissionUI->>StreamCallActivity: Permissions result
StreamCallActivity->>StreamCallActivity: Update capabilities based on granted permissions
else All Permissions Available
PermissionManager-->>StreamCallActivity: All permissions available
StreamCallActivity->>StreamCallActivity: Set full capabilities
end
StreamCallActivity->>ServiceChecker: Verify call service is running
alt Service Running
ServiceChecker-->>StreamCallActivity: Service confirmed running
else Service Not Running
ServiceChecker-->>StreamCallActivity: Service not running (log error)
end
StreamCallActivity->>Dispatcher: Dispatch call notification
Dispatcher->>Dispatcher: Handle notification with foreground permissions
Dispatcher-->>User: Display call UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/CallState.kt (1)
1784-1787: Parameter name should be plural to match type.The parameter
ownCapabilityis singular but expects aList<OwnCapability>. UseownCapabilitiesfor consistency with the internal_ownCapabilitiesfield.✨ Proposed fix
`@InternalStreamVideoApi` - fun setOwnCapabilities(ownCapability: List<OwnCapability>) { - this._ownCapabilities.value = ownCapability + fun setOwnCapabilities(ownCapabilities: List<OwnCapability>) { + this._ownCapabilities.value = ownCapabilities }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/CallState.kt` around lines 1784 - 1787, The parameter name in setOwnCapabilities should be plural to match its type and the internal field; rename the parameter ownCapability to ownCapabilities in the function signature of setOwnCapabilities and update all usages inside the function (this._ownCapabilities.value) and any call sites referencing that parameter to use the new name; ensure imports/overloads are unchanged and rebuild to catch any unresolved references.stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/internal/service/StreamForegroundPermissionUtil.kt (2)
24-24: Remove unnecessary import.The import
kotlin.jvm.javais not needed. The.javaextension property for getting a JavaClassfrom a KotlinKClassis available without explicit import.✨ Proposed fix
import io.getstream.video.android.core.notifications.internal.service.permissions.AudioCallPermissionManager import io.getstream.video.android.core.notifications.internal.service.permissions.ForegroundServicePermissionManager -import kotlin.jvm.java🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/internal/service/StreamForegroundPermissionUtil.kt` at line 24, The file StreamForegroundPermissionUtil.kt contains an unnecessary import `kotlin.jvm.java`; remove that import line so the code relies on the built-in `.java` extension on KClass (no other code changes required), e.g., delete the `import kotlin.jvm.java` entry in StreamForegroundPermissionUtil.kt to clean up unused imports.
29-40: Consider caching permission managers and documenting the sentinel value.
ForegroundServicePermissionManager()andAudioCallPermissionManager()are instantiated on every call. Consider caching them sincerequiredForegroundTypesis a constant set.The return value
setOf(-1)is a sentinel value that should be documented for callers.♻️ Proposed improvement
`@InternalStreamVideoApi` public object StreamForegroundPermissionUtil { + private val callServicePermissionManager = ForegroundServicePermissionManager() + private val audioCallPermissionManager = AudioCallPermissionManager() + + /** + * Returns the required foreground service types for the given call type. + * `@param` callType The call type string. + * `@return` Set of ServiceInfo.FOREGROUND_SERVICE_TYPE_* constants, or setOf(-1) if + * StreamVideo client is not initialized or an unknown service class is configured. + */ public fun getForegroundPermissionsForCallType(callType: String): Set<Int> { val client = StreamVideo.instanceOrNull() as? StreamVideoClient if (client != null) { val config = client.callServiceConfigRegistry.get(callType) return when (config.serviceClass) { - CallService::class.java -> ForegroundServicePermissionManager().requiredForegroundTypes - AudioCallService::class.java -> AudioCallPermissionManager().requiredForegroundTypes + CallService::class.java -> callServicePermissionManager.requiredForegroundTypes + AudioCallService::class.java -> audioCallPermissionManager.requiredForegroundTypes else -> setOf(-1) } } return setOf(-1) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/internal/service/StreamForegroundPermissionUtil.kt` around lines 29 - 40, getForegroundPermissionsForCallType currently instantiates ForegroundServicePermissionManager and AudioCallPermissionManager on every call and returns a magic sentinel setOf(-1) undocumented; fix by adding cached instances (e.g., private val foregroundPermissionManager and audioCallPermissionManager) and reuse their requiredForegroundTypes in getForegroundPermissionsForCallType, and replace the inline sentinel with a named constant (e.g., UNKNOWN_FOREGROUND_PERMISSION = setOf(-1)) documented via KDoc on the function or constant to explain the sentinel meaning for callers.stream-video-android-ui-compose/src/main/kotlin/io/getstream/video/android/compose/ui/StreamCallActivityComposeDelegate.kt (1)
127-135: Consider guarding against redundant recomposition.When
AllPermissionsGrantedfires,onAllPermissionGranted()setsrenderPermissionUitofalse. However, sinceAllPermissionsGrantedfires on every recomposition while permissions are granted (see context snippet 3), there's a brief window where the callback may execute multiple times before the state change propagates.While this is harmless (setting
falsetofalseis idempotent), you could add a guard for clarity.✨ Optional guard
- if (renderPermissionUi) { + val renderPermissionUi by activity.renderPermissionUi.collectAsStateWithLifecycle() + if (renderPermissionUi) { NoPermissionUi(activity, call) { - activity.updateRenderPermissionUi(false) + if (activity.renderPermissionUi.value) { + activity.updateRenderPermissionUi(false) + } } } else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-video-android-ui-compose/src/main/kotlin/io/getstream/video/android/compose/ui/StreamCallActivityComposeDelegate.kt` around lines 127 - 135, The NoPermissionUi callback can trigger redundant updates because AllPermissionsGranted may fire multiple times before state updates; modify the callback in StreamCallActivityComposeDelegate to check the current renderPermissionUi state (from activity.renderPermissionUi.collectAsStateWithLifecycle()/renderPermissionUi) or query activity before calling activity.updateRenderPermissionUi(false) so you only call updateRenderPermissionUi(false) when renderPermissionUi is true, e.g., add a guard around the lambda passed to NoPermissionUi that references renderPermissionUi (or calls a new activity.isRenderPermissionUiVisible()) to avoid redundant state writes from onAllPermissionGranted.stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/internal/service/observers/CallServiceNotificationUpdateObserver.kt (1)
144-151: Verify notification dispatcher fallback when service is not running.The change delegates active call notification to
StreamVideo.instanceOrNull()?.getStreamNotificationDispatcher()?.notify(), which only posts a notification without starting the foreground service (as confirmed byDefaultNotificationDispatcher.notify()at lines 27-40). This is correct for the outgoing→active transition where the service should already be running.However, if
StreamVideo.instanceOrNull()returnsnullor the dispatcher is unavailable, this silently fails without logging or fallback.♻️ Proposed improvement with fallback logging
private fun showActiveCallNotification( context: Context, callId: StreamCallId, notification: Notification, ) { logger.d { "[showActiveCallNotification] Showing active call notification" } val notificationId = call.state.notificationIdFlow.value ?: callId.getNotificationId(NotificationType.Ongoing) - StreamVideo.instanceOrNull() - ?.getStreamNotificationDispatcher() - ?.notify( - callId, - notificationId, - notification, - ) + val dispatcher = StreamVideo.instanceOrNull()?.getStreamNotificationDispatcher() + if (dispatcher != null) { + dispatcher.notify(callId, notificationId, notification) + } else { + logger.w { "[showActiveCallNotification] StreamVideo dispatcher unavailable, notification not posted" } + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/internal/service/observers/CallServiceNotificationUpdateObserver.kt` around lines 144 - 151, CallServiceNotificationUpdateObserver currently forwards active-call notifications via StreamVideo.instanceOrNull()?.getStreamNotificationDispatcher()?.notify(...), which silently does nothing when StreamVideo or the dispatcher is null; update the observer to handle that fallback by checking for null and either logging a clear warning/error (including callId/notificationId) and/or starting the foreground call service directly when the dispatcher is unavailable; reference StreamVideo.instanceOrNull(), getStreamNotificationDispatcher(), notify(), and DefaultNotificationDispatcher.notify() when making the change so the fix ensures the active call is surfaced or at least diagnostic logs are emitted when the dispatcher/service isn't present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/ClientState.kt`:
- Around line 188-198: The check uses reflection incorrectly:
callServiceConfig.serviceClass is already a Class<out Service>, so replace the
redundant serviceClass::class.java usage in the
ServiceIntentBuilder().isServiceRunning call with the serviceClass value
directly; update the invocation in the ClientState code that uses
callServiceConfig and ServiceIntentBuilder.isServiceRunning (passing
this.client.context and serviceClass) to avoid taking the KClass of the Class
object.
In
`@stream-video-android-ui-core/src/main/kotlin/io/getstream/video/android/ui/common/StreamCallActivity.kt`:
- Around line 875-880: The call to uiDelegate.setContent(...) and any UI state
reads/updates (_renderPermissionUi.value, renderPermissionUi.first {...}) are
being executed on Dispatchers.IO; move those UI interactions onto the Main
dispatcher (e.g., with withContext(Dispatchers.Main) or
lifecycleScope.launch(Dispatchers.Main)) before calling create(...). Also
prevent infinite recursion by adding a retry guard: introduce a permission retry
counter (e.g., permissionRetryCount field and MAX_PERMISSION_RETRIES constant)
and increment it each time create(...) is re-invoked from the permission flow
(referencing create and renderPermissionUi); when the max is exceeded, perform a
graceful fallback such as finishing the activity with an error instead of
recursing.
- Around line 884-904: The current setCallCapabilitiesIfNotPresent overwrites
existing capabilities with an empty list because
call.state.setOwnCapabilities(outgoingCallCapabilities) is executed even for
non-outgoing intents or when no permissions match; update the logic in
setCallCapabilitiesIfNotPresent so that you only compute and set capabilities
when intent.action == NotificationHandler.ACTION_OUTGOING_CALL and only call
call.state.setOwnCapabilities(...) when outgoingCallCapabilities is non-empty
(or at least inside that outgoing-call branch), using
StreamForegroundPermissionUtil.getForegroundPermissionsForCallType to populate
outgoingCallCapabilities and keeping the early return that checks
call.state.ownCapabilities.value.
---
Nitpick comments:
In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/CallState.kt`:
- Around line 1784-1787: The parameter name in setOwnCapabilities should be
plural to match its type and the internal field; rename the parameter
ownCapability to ownCapabilities in the function signature of setOwnCapabilities
and update all usages inside the function (this._ownCapabilities.value) and any
call sites referencing that parameter to use the new name; ensure
imports/overloads are unchanged and rebuild to catch any unresolved references.
In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/internal/service/observers/CallServiceNotificationUpdateObserver.kt`:
- Around line 144-151: CallServiceNotificationUpdateObserver currently forwards
active-call notifications via
StreamVideo.instanceOrNull()?.getStreamNotificationDispatcher()?.notify(...),
which silently does nothing when StreamVideo or the dispatcher is null; update
the observer to handle that fallback by checking for null and either logging a
clear warning/error (including callId/notificationId) and/or starting the
foreground call service directly when the dispatcher is unavailable; reference
StreamVideo.instanceOrNull(), getStreamNotificationDispatcher(), notify(), and
DefaultNotificationDispatcher.notify() when making the change so the fix ensures
the active call is surfaced or at least diagnostic logs are emitted when the
dispatcher/service isn't present.
In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/internal/service/StreamForegroundPermissionUtil.kt`:
- Line 24: The file StreamForegroundPermissionUtil.kt contains an unnecessary
import `kotlin.jvm.java`; remove that import line so the code relies on the
built-in `.java` extension on KClass (no other code changes required), e.g.,
delete the `import kotlin.jvm.java` entry in StreamForegroundPermissionUtil.kt
to clean up unused imports.
- Around line 29-40: getForegroundPermissionsForCallType currently instantiates
ForegroundServicePermissionManager and AudioCallPermissionManager on every call
and returns a magic sentinel setOf(-1) undocumented; fix by adding cached
instances (e.g., private val foregroundPermissionManager and
audioCallPermissionManager) and reuse their requiredForegroundTypes in
getForegroundPermissionsForCallType, and replace the inline sentinel with a
named constant (e.g., UNKNOWN_FOREGROUND_PERMISSION = setOf(-1)) documented via
KDoc on the function or constant to explain the sentinel meaning for callers.
In
`@stream-video-android-ui-compose/src/main/kotlin/io/getstream/video/android/compose/ui/StreamCallActivityComposeDelegate.kt`:
- Around line 127-135: The NoPermissionUi callback can trigger redundant updates
because AllPermissionsGranted may fire multiple times before state updates;
modify the callback in StreamCallActivityComposeDelegate to check the current
renderPermissionUi state (from
activity.renderPermissionUi.collectAsStateWithLifecycle()/renderPermissionUi) or
query activity before calling activity.updateRenderPermissionUi(false) so you
only call updateRenderPermissionUi(false) when renderPermissionUi is true, e.g.,
add a guard around the lambda passed to NoPermissionUi that references
renderPermissionUi (or calls a new activity.isRenderPermissionUiVisible()) to
avoid redundant state writes from onAllPermissionGranted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1f94450e-0056-4b12-b8f3-755fd3801401
📒 Files selected for processing (9)
demo-app/src/main/kotlin/io/getstream/video/android/util/StreamVideoInitHelper.ktstream-video-android-core/src/main/kotlin/io/getstream/video/android/core/CallState.ktstream-video-android-core/src/main/kotlin/io/getstream/video/android/core/ClientState.ktstream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/internal/service/StreamForegroundPermissionUtil.ktstream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/internal/service/observers/CallServiceNotificationUpdateObserver.ktstream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/internal/service/permissions/ForegroundServicePermissionManager.ktstream-video-android-ui-compose/src/main/kotlin/io/getstream/video/android/compose/ui/StreamCallActivityComposeDelegate.ktstream-video-android-ui-core/api/stream-video-android-ui-core.apistream-video-android-ui-core/src/main/kotlin/io/getstream/video/android/ui/common/StreamCallActivity.kt
stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/ClientState.kt
Show resolved
Hide resolved
...o-android-ui-core/src/main/kotlin/io/getstream/video/android/ui/common/StreamCallActivity.kt
Show resolved
Hide resolved
...o-android-ui-core/src/main/kotlin/io/getstream/video/android/ui/common/StreamCallActivity.kt
Show resolved
Hide resolved
rahul-lohra
commented
Mar 9, 2026
stream-video-android-ui-core/api/stream-video-android-ui-core.api
Outdated
Show resolved
Hide resolved
...o-android-ui-core/src/main/kotlin/io/getstream/video/android/ui/common/StreamCallActivity.kt
Show resolved
Hide resolved
...e/src/main/kotlin/io/getstream/video/android/compose/ui/StreamCallActivityComposeDelegate.kt
Show resolved
Hide resolved
...etstream/video/android/core/notifications/internal/service/StreamForegroundPermissionUtil.kt
Show resolved
Hide resolved
Contributor
|
This pull request has been automatically marked as stale because it has been inactive for 14 days. It will be closed in 7 days if no further activity occurs. |
aleksandar-apostolov
approved these changes
Apr 1, 2026
Contributor
aleksandar-apostolov
left a comment
aleksandar-apostolov
left a comment
There was a problem hiding this comment.
All feedback addressed. LGTM.
|
aleksandar-apostolov
deleted the
bugfix/rahullohra/outgoing-call-fg-service-exception
branch
Collaborator
|
🚀 Available in v1.21.0 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Goal
Implementation
Root Cause - Launching FG Service even when the application is in background and ringing state transitions from outgoing to active
Proposed Solution
Because of this we need to correctly handle the following
2. Correctly update the notification transition from outgoing to active
🎨 UI Changes
Just a no permission ui in outgoing-call flow
Add relevant videos
Testing
Scenario: Outgoing call → background → call accepted
1. Make an outgoing call to another user.
2. While the call is still in the outgoing state, press the Home button to send the app to the background.
3. Accept the call on the other device.
Expected behavior
• The caller should transition smoothly from outgoing → active state.
• PiP should not be rendered, since it was never shown before the app went to the background.
• The Call Service should continue running without interruption.
• The notification should update from outgoing → active.
End condition
• If either the caller or callee declines/ends the call, the other participant should also leave the call and the session should terminate correctly.
Summary by CodeRabbit
New Features
Bug Fixes