Skip to content
3 changes: 3 additions & 0 deletions app/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,9 @@ dependencies {
testImplementation("com.squareup.okhttp3:mockwebserver:$okhttpVersion")
testImplementation("com.google.dagger:hilt-android-testing:2.59.2")
testImplementation("org.robolectric:robolectric:4.16.1")
// conscrypt-android provides Android JNI libs only; the openjdk-uber variant bundles
// JVM host natives so Robolectric can initialise the security provider without crashing.
testImplementation("org.conscrypt:conscrypt-openjdk-uber:2.5.2")
}

tasks.register<Copy>("installGitHooks") {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2117,9 +2117,10 @@ class CallActivity : CallBaseActivity() {
) {
Log.d(TAG, "handleCallParticipantsChanged")

// The signaling session is the same as the Nextcloud session only when the MCU is not used.
// The signaling session is the same as the Nextcloud session only when internal signaling is used.
// With external signaling (HPB), participant session IDs are HPB session IDs regardless of MCU.
var currentSessionId = callSession
if (hasMCU) {
if (webSocketClient != null) {
currentSessionId = webSocketClient!!.sessionId
}
Log.d(TAG, " currentSessionId is $currentSessionId")
Expand Down Expand Up @@ -2424,7 +2425,9 @@ class CallActivity : CallBaseActivity() {
signalingMessageReceiver!!
)

localStateBroadcaster!!.handleCallParticipantAdded(callViewModel.getParticipant(sessionId)?.uiState?.value)
// Pass the live StateFlow so LocalStateBroadcasterNoMcu can observe ICE state changes
// and send the local state exactly when the data channel becomes ready.
localStateBroadcaster!!.handleCallParticipantAdded(callViewModel.getParticipant(sessionId)!!.uiState)

initPipMode()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@

import java.util.Objects;

import kotlinx.coroutines.flow.StateFlow;

/**
* Helper class to send the local participant state to the other participants in the call.
* <p>
Expand Down Expand Up @@ -82,6 +84,17 @@ public void destroy() {
this.localCallParticipantModel.removeObserver(localCallParticipantModelObserver);
}

/**
* Passes the live StateFlow for the given participant so the broadcaster can react to each
* connection-state change. The default implementation takes a snapshot of the current value
* and delegates to {@link #handleCallParticipantAdded(ParticipantUiState)}; subclasses that
* need to observe future emissions (e.g. {@code LocalStateBroadcasterNoMcu}) should override
* this method instead.
*/
public void handleCallParticipantAdded(StateFlow<ParticipantUiState> uiStateFlow) {
handleCallParticipantAdded(uiStateFlow.getValue());
}

public abstract void handleCallParticipantAdded(ParticipantUiState uiState);
public abstract void handleCallParticipantRemoved(String sessionId);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,26 +14,25 @@ package com.nextcloud.talk.call
* state when a remote participant is added.
*
*
* The state is sent when a connection with another participant is first established (which implicitly broadcasts the
* initial state when the local participant joins the call, as a connection is established with all the remote
* participants). Note that, as long as that participant stays in the call, the initial state is not sent again, even
* after a temporary disconnection; data channels use a reliable transport by default, so even if the state changes
* while the connection is temporarily interrupted the normal state update messages should be received by the other
* participant once the connection is restored.
* The state is sent the first time the ICE connection reaches a "connected" state for a given participant
* (isConnected transitions from false/unknown to true). The observer collects the participant's
* uiState StateFlow so that if the connection is briefly lost and then restored (e.g. an ICE restart),
* the state is re-sent on the next false → true transition.
*
*
* Nevertheless, in case of a failed connection and an ICE restart it is unclear whether the data channel messages
* would be received or not (as the data channel transport may be the one that failed and needs to be restarted).
* However, the state (except the speaking state) is also sent through signaling messages, which need to be
* explicitly fetched from the internal signaling server, so even in case of a failed connection they will be
* eventually received once the remote participant connects again.
* Note that, as long as a participant stays in the call, the state is sent each time the ICE connection
* goes from disconnected to connected; data channels use a reliable transport by default, so even if the
* state changes while the connection is temporarily interrupted the normal state update messages should be
* received by the other participant once the connection is restored.
*/
import com.nextcloud.talk.activities.ParticipantUiState
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.Job
import kotlinx.coroutines.SupervisorJob
import org.webrtc.PeerConnection.IceConnectionState
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.launch
import java.util.concurrent.ConcurrentHashMap

class LocalStateBroadcasterNoMcu(
Expand All @@ -42,39 +41,25 @@ class LocalStateBroadcasterNoMcu(
private val scope: CoroutineScope = CoroutineScope(Dispatchers.Main.immediate + SupervisorJob())
) : LocalStateBroadcaster(localCallParticipantModel, messageSender) {

// Map sessionId -> observer wrapper (Flow collector job)
private val iceConnectionStateObservers = ConcurrentHashMap<String, IceConnectionStateObserver>()

private inner class IceConnectionStateObserver(val uiState: ParticipantUiState) {
private var job: Job? = null

init {
handleStateChange(uiState)
}

private fun handleStateChange(uiState: ParticipantUiState) {
// Determine ICE connection state
val iceState = if (uiState.isConnected) IceConnectionState.CONNECTED else IceConnectionState.NEW

if (iceState == IceConnectionState.CONNECTED) {
remove()
sendState(uiState.sessionKey)
}
}

fun remove() {
job?.cancel()
iceConnectionStateObservers.remove(uiState.sessionKey)
}
/**
* Primary entry point called by CallActivity. Starts (or restarts) collecting the live
* StateFlow so that the local state is sent every time the ICE connection transitions from
* disconnected to connected.
*/
override fun handleCallParticipantAdded(uiStateFlow: StateFlow<ParticipantUiState>) {
val sessionId = uiStateFlow.value.sessionKey ?: return
iceConnectionStateObservers[sessionId]?.remove()
iceConnectionStateObservers[sessionId] = IceConnectionStateObserver(sessionId, uiStateFlow)
}

/**
* Fallback for callers that only have a snapshot (e.g. tests that pre-date the StateFlow API).
* Wraps the snapshot in a single-value StateFlow and delegates to the primary overload.
*/
override fun handleCallParticipantAdded(uiState: ParticipantUiState) {
uiState.sessionKey?.let {
iceConnectionStateObservers[it]?.remove()

iceConnectionStateObservers[it] =
IceConnectionStateObserver(uiState)
}
handleCallParticipantAdded(MutableStateFlow(uiState) as StateFlow<ParticipantUiState>)
}

override fun handleCallParticipantRemoved(sessionId: String) {
Expand All @@ -83,13 +68,35 @@ class LocalStateBroadcasterNoMcu(

override fun destroy() {
super.destroy()
// Cancel all collectors safely
val observersCopy = iceConnectionStateObservers.values.toList()
for (observer in observersCopy) {
observer.remove()
}
}

private inner class IceConnectionStateObserver(
private val sessionId: String,
uiStateFlow: StateFlow<ParticipantUiState>
) {
private val job: Job = scope.launch {
var previousIsConnected: Boolean? = null
uiStateFlow.collect { uiState ->
val currentIsConnected = uiState.isConnected
// Send state on every false → true (or null → true) transition so that the
// remote participant receives the local state as soon as the data channel is ready.
if (currentIsConnected && previousIsConnected != true) {
sendState(uiState.sessionKey)
}
previousIsConnected = currentIsConnected
}
}

fun remove() {
job.cancel()
iceConnectionStateObservers.remove(sessionId)
}
}

private fun sendState(sessionKey: String?) {
messageSender.send(getDataChannelMessageForAudioState(), sessionKey)
messageSender.send(getDataChannelMessageForSpeakingState(), sessionKey)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
/*
* Nextcloud Talk - Android Client
*
* SPDX-FileCopyrightText: 2026 Alain Lauzon
* SPDX-License-Identifier: GPL-3.0-or-later
*/
package com.nextcloud.talk.activities

import android.app.Application
import com.nextcloud.talk.call.LocalStateBroadcaster
import com.nextcloud.talk.signaling.SignalingMessageReceiver
import kotlinx.coroutines.flow.StateFlow
import org.junit.Assert.assertSame
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.kotlin.argumentCaptor
import org.mockito.kotlin.mock
import org.mockito.kotlin.verify
import org.robolectric.Robolectric
import org.robolectric.RobolectricTestRunner
import org.robolectric.annotation.Config
import java.lang.reflect.Field

/**
* Robolectric test verifying that [CallActivity.addCallParticipant] passes the **live StateFlow**
* from [ParticipantHandler.uiState] to [LocalStateBroadcaster.handleCallParticipantAdded].
*
* Before the fix, the method passed only the snapshot value [StateFlow.value], meaning
* [LocalStateBroadcasterNoMcu] could not observe future ICE state changes and videoOn/audioOn
* were never sent once the data channel was ready.
*
* This test:
* - creates [CallActivity] via Robolectric WITHOUT calling [Activity.onCreate] (avoiding native
* WebRTC and Dagger initialisation)
* - injects the required fields via reflection
* - calls [addCallParticipant] via reflection
* - verifies that the argument passed to [LocalStateBroadcaster.handleCallParticipantAdded] is
* the exact same [StateFlow] object as [CallViewModel.getParticipant]!!.uiState
*
* If the buggy snapshot pattern is used (`uiState.value` instead of `uiState`), the verification
* fails because the [StateFlow] overload is never called.
*/
@RunWith(RobolectricTestRunner::class)
@Config(
// Use the plain Application to avoid NextcloudTalkApplication.onCreate() which initialises
// Dagger, WebRTC, and WorkManager — none of which are needed for this focused test.
application = Application::class,
sdk = [33]
)
class CallActivityAddParticipantTest {

private val mockLocalStateBroadcaster: LocalStateBroadcaster = mock()
private val mockSignalingMessageReceiver: SignalingMessageReceiver = mock()

private lateinit var callViewModel: CallViewModel
private lateinit var activity: CallActivity

@Before
fun setUp() {
callViewModel = CallViewModel()

// Build the Activity without triggering onCreate — this avoids Dagger injection,
// EglBase.create() (native), and all network calls.
activity = Robolectric.buildActivity(CallActivity::class.java).get()

// Inject the minimum set of fields required by addCallParticipant.
setField("callViewModel", callViewModel)
setField("localStateBroadcaster", mockLocalStateBroadcaster)
setField("signalingMessageReceiver", mockSignalingMessageReceiver)
setField("baseUrl", "https://test.example.com")
setField("roomToken", "testRoom")
// hasExternalSignalingServer = true skips the OfferAnswerNickProvider branch
setField("hasExternalSignalingServer", true)
}

// -----------------------------------------------------------------------------------------
// Tests
// -----------------------------------------------------------------------------------------

/**
* The live [StateFlow] from [ParticipantHandler.uiState] must be passed to
* [LocalStateBroadcaster.handleCallParticipantAdded], not just the current snapshot.
*
* Fails with the buggy code because the snapshot call goes to the
* [handleCallParticipantAdded(ParticipantUiState)] overload, not the StateFlow one.
*/
@Test
fun `addCallParticipant passes the live StateFlow to handleCallParticipantAdded`() {
val sessionId = "robolectric-session-1"

invokeAddCallParticipant(sessionId)

val captor = argumentCaptor<StateFlow<ParticipantUiState>>()
verify(mockLocalStateBroadcaster).handleCallParticipantAdded(captor.capture())

val capturedFlow = captor.firstValue

// The captured argument must be the SAME StateFlow object that ParticipantHandler
// exposes — not a copy, not a one-shot MutableStateFlow wrapping the snapshot.
assertSame(
"Expected the live ParticipantHandler.uiState StateFlow, got a different object",
callViewModel.getParticipant(sessionId)!!.uiState,
capturedFlow
)
}

// -----------------------------------------------------------------------------------------
// Reflection helpers
// -----------------------------------------------------------------------------------------

private fun invokeAddCallParticipant(sessionId: String) {
val method = CallActivity::class.java.getDeclaredMethod("addCallParticipant", String::class.java)
method.isAccessible = true
method.invoke(activity, sessionId)
}

private fun setField(fieldName: String, value: Any?) {
val field = findField(CallActivity::class.java, fieldName)
?: error("Field '$fieldName' not found in CallActivity hierarchy")
field.isAccessible = true
field.set(activity, value)
}

private fun findField(clazz: Class<*>, fieldName: String): Field? {
var current: Class<*>? = clazz
while (current != null) {
try {
return current.getDeclaredField(fieldName)
} catch (e: NoSuchFieldException) {
current = current.superclass
}
}
return null
}
}
Loading