[stable-24.0] fix: send local state when ICE connects instead of at participant-add time

app/build.gradle.kts

@@ -382,6 +382,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") {

app/src/main/java/com/nextcloud/talk/activities/CallActivity.kt

@@ -2132,9 +2132,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")
@@ -2439,7 +2440,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()
     }

app/src/main/java/com/nextcloud/talk/call/LocalStateBroadcaster.java

@@ -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>
@@ -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);
 

app/src/main/java/com/nextcloud/talk/call/LocalStateBroadcasterNoMcu.kt

@@ -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(
@@ -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) {
@@ -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)

app/src/test/java/com/nextcloud/talk/activities/CallActivityAddParticipantTest.kt

@@ -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
+    }
+}