diff --git a/.gitignore b/.gitignore index 8a43e8d13c1..0ea67b20939 100644 --- a/.gitignore +++ b/.gitignore @@ -21,6 +21,7 @@ target/ # Local configuration files (sdk path, etc) local.properties tests/local.properties +.vscode/ # Mac .DS_Store files .DS_Store diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index 64f59300ad6..e8874b83416 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -184,7 +184,6 @@ + -> + // Log permission results + Log.d(TAG, "Permission request completed with results: $permissionMap") + val rationaleList: MutableList = ArrayList() val audioPermission = permissionMap[Manifest.permission.RECORD_AUDIO] if (audioPermission != null) { if (java.lang.Boolean.TRUE == audioPermission) { Log.d(TAG, "Microphone permission was granted") } else { + Log.d(TAG, "Microphone permission is not yet granted. Request will be made for permission.") rationaleList.add(resources.getString(R.string.nc_microphone_permission_hint)) } } @@ -333,6 +341,7 @@ class CallActivity : CallBaseActivity() { if (java.lang.Boolean.TRUE == cameraPermission) { Log.d(TAG, "Camera permission was granted") } else { + Log.d(TAG, "Camera permission was denied") rationaleList.add(resources.getString(R.string.nc_camera_permission_hint)) } } @@ -342,6 +351,7 @@ class CallActivity : CallBaseActivity() { if (java.lang.Boolean.TRUE == bluetoothPermission) { enableBluetoothManager() } else { + Log.d(TAG, "Bluetooth permission was denied") // Only ask for bluetooth when already asking to grant microphone or camera access. Asking // for bluetooth solely is not important enough here and would most likely annoy the user. if (rationaleList.isNotEmpty()) { @@ -350,11 +360,36 @@ class CallActivity : CallBaseActivity() { } } } + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { + val notificationPermission = permissionMap[Manifest.permission.POST_NOTIFICATIONS] + if (notificationPermission != null) { + if (java.lang.Boolean.TRUE == notificationPermission) { + Log.d(TAG, "Notification permission was granted") + } else { + Log.w(TAG, "Notification permission was denied - this may cause call hang") + rationaleList.add(resources.getString(R.string.nc_notification_permission_hint)) + } + } + } if (rationaleList.isNotEmpty()) { showRationaleDialogForSettings(rationaleList) } + // Check if we should proceed with call despite notification permission + val notificationPermissionGranted = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { + permissionMap[Manifest.permission.POST_NOTIFICATIONS] == true + } else { + true // Older Android versions have permission by default + } + + Log.d( + TAG, + "Notification permission granted: $notificationPermissionGranted, " + + "isConnectionEstablished: $isConnectionEstablished" + ) + if (!isConnectionEstablished) { + Log.d(TAG, "Proceeding with prepareCall() despite notification permission status") prepareCall() } } @@ -384,6 +419,21 @@ class CallActivity : CallBaseActivity() { super.onCreate(savedInstanceState) sharedApplication!!.componentApplication.inject(this) + // Register broadcast receiver for ending call from notification + val endCallFilter = IntentFilter(END_CALL_FROM_NOTIFICATION) + + // Use the proper utility function with ReceiverFlag for Android 14+ compatibility + // This receiver is for internal app use only (notification actions), so it should NOT be exported + registerPermissionHandlerBroadcastReceiver( + endCallFromNotificationReceiver, + endCallFilter, + permissionUtil!!.privateBroadcastPermission, + null, + ReceiverFlag.NotExported + ) + + Log.d(TAG, "Broadcast receiver registered successfully") + callViewModel = ViewModelProvider(this, viewModelFactory)[CallViewModel::class.java] rootEglBase = EglBase.create() @@ -684,6 +734,8 @@ class CallActivity : CallBaseActivity() { override fun onStop() { super.onStop() + Log.d(TAG, "CallActivity.onStop: isInPipMode=$isInPipMode currentCallStatus=$currentCallStatus" + + " isFinishing=$isFinishing isChangingConfigurations=$isChangingConfigurations") active = false if (isMicInputAudioThreadRunning) { @@ -788,9 +840,11 @@ class CallActivity : CallBaseActivity() { true } binding!!.hangupButton.setOnClickListener { + isIntentionallyLeavingCall = true hangup(shutDownView = true, endCallForAll = true) } binding!!.endCallPopupMenu.setOnClickListener { + isIntentionallyLeavingCall = true hangup(shutDownView = true, endCallForAll = true) binding!!.endCallPopupMenu.visibility = View.GONE } @@ -802,9 +856,11 @@ class CallActivity : CallBaseActivity() { } } binding!!.hangupButton.setOnClickListener { + isIntentionallyLeavingCall = true hangup(shutDownView = true, endCallForAll = false) } binding!!.endCallPopupMenu.setOnClickListener { + isIntentionallyLeavingCall = true hangup(shutDownView = true, endCallForAll = false) binding!!.endCallPopupMenu.visibility = View.GONE } @@ -1029,6 +1085,18 @@ class CallActivity : CallBaseActivity() { } } + // Check notification permission for Android 13+ (API 33+) + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { + if (permissionUtil!!.isPostNotificationsPermissionGranted()) { + Log.d(TAG, "Notification permission already granted") + } else if (shouldShowRequestPermissionRationale(Manifest.permission.POST_NOTIFICATIONS)) { + permissionsToRequest.add(Manifest.permission.POST_NOTIFICATIONS) + rationaleList.add(resources.getString(R.string.nc_notification_permission_hint)) + } else { + permissionsToRequest.add(Manifest.permission.POST_NOTIFICATIONS) + } + } + if (permissionsToRequest.isNotEmpty()) { if (rationaleList.isNotEmpty()) { showRationaleDialog(permissionsToRequest, rationaleList) @@ -1037,25 +1105,55 @@ class CallActivity : CallBaseActivity() { } } else if (!isConnectionEstablished) { prepareCall() + } else { + // All permissions granted but connection not established + Log.d(TAG, "All permissions granted but connection not established, proceeding with prepareCall()") + prepareCall() } } private fun prepareCall() { - basicInitialization() - initViews() - // updateSelfVideoViewPosition(true) - checkRecordingConsentAndInitiateCall() + Log.d(TAG, "prepareCall() started") if (permissionUtil!!.isMicrophonePermissionGranted()) { - CallForegroundService.start(applicationContext, conversationName, intent.extras) + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { + if (permissionUtil!!.isPostNotificationsPermissionGranted()) { + Log.d(TAG, "Starting foreground service with notification permission") + CallForegroundService.start(applicationContext, conversationName, intent.extras) + } else { + Log.w( + TAG, + "Notification permission not granted - call will work " + + "but without persistent notification" + ) + Snackbar.make( + binding!!.root, + resources.getString(R.string.nc_notification_permission_hint), + SEC_10 + ).show() + } + } else { + Log.d(TAG, "Starting foreground service (Android 12-)") + CallForegroundService.start(applicationContext, conversationName, intent.extras) + } + if (!microphoneOn) { onMicrophoneClick() } + } else { + Log.w(TAG, "Microphone permission not granted - skipping foreground service start") } + Log.d(TAG, "Ensuring call proceeds even without notification permission") + + basicInitialization() + initViews() + checkRecordingConsentAndInitiateCall() + if (isVoiceOnlyCall) { binding!!.selfVideoViewWrapper.visibility = View.GONE } else if (permissionUtil!!.isCameraPermissionGranted()) { + Log.d(TAG, "Camera permission granted, showing video") binding!!.selfVideoViewWrapper.visibility = View.VISIBLE // don't enable the camera if call was answered via notification if (!isIncomingCallFromNotification) { @@ -1064,6 +1162,8 @@ class CallActivity : CallBaseActivity() { if (cameraEnumerator!!.deviceNames.isEmpty()) { binding!!.cameraButton.visibility = View.GONE } + } else { + Log.w(TAG, "Camera permission not granted, hiding video") } } @@ -1080,13 +1180,33 @@ class CallActivity : CallBaseActivity() { for (rationale in rationaleList) { rationalesWithLineBreaks.append(rationale).append("\n\n") } + + // Log when permission rationale dialog is shown + Log.d(TAG, "Showing permission rationale dialog for permissions: $permissionsToRequest") + val hasNotificationPerm = permissionsToRequest + .contains(Manifest.permission.POST_NOTIFICATIONS) + Log.d(TAG, "Rationale includes notification permission: $hasNotificationPerm") + val dialogBuilder = MaterialAlertDialogBuilder(this) .setTitle(R.string.nc_permissions_rationale_dialog_title) .setMessage(rationalesWithLineBreaks) .setPositiveButton(R.string.nc_permissions_ask) { _, _ -> + Log.d(TAG, "User clicked 'Ask' for permissions") requestPermissionLauncher.launch(permissionsToRequest.toTypedArray()) } - .setNegativeButton(R.string.nc_common_dismiss, null) + .setNegativeButton(R.string.nc_common_dismiss) { _, _ -> + // Log when user dismisses permission request + Log.w(TAG, "User dismissed permission request for: $permissionsToRequest") + if (permissionsToRequest.contains(Manifest.permission.POST_NOTIFICATIONS)) { + Log.w(TAG, "Notification permission specifically dismissed - proceeding with call anyway") + } + + // Proceed with call even when notification permission is dismissed + if (!isConnectionEstablished) { + Log.d(TAG, "Proceeding with prepareCall() after dismissing notification permission") + prepareCall() + } + } viewThemeUtils.dialog.colorMaterialAlertDialogBackground(this, dialogBuilder) dialogBuilder.show() } @@ -1362,22 +1482,59 @@ class CallActivity : CallBaseActivity() { } public override fun onDestroy() { + Log.d(TAG, "onDestroy called") + Log.d(TAG, "onDestroy: isIntentionallyLeavingCall=$isIntentionallyLeavingCall") + Log.d(TAG, "onDestroy: currentCallStatus=$currentCallStatus") + + val isSystemInitiatedDestroy = !isIntentionallyLeavingCall && currentCallStatus !== CallStatus.LEAVING + if (signalingMessageReceiver != null) { - signalingMessageReceiver!!.removeListener(localParticipantMessageListener) - signalingMessageReceiver!!.removeListener(offerMessageListener) + if (!isSystemInitiatedDestroy) { + signalingMessageReceiver!!.removeListener(localParticipantMessageListener) + signalingMessageReceiver!!.removeListener(offerMessageListener) + } else { + Log.d(TAG, "System-initiated destroy, keeping signaling listeners for foreground service") + } } if (localStream != null) { - localStream!!.dispose() - localStream = null - Log.d(TAG, "Disposed localStream") + if (!isSystemInitiatedDestroy) { + localStream!!.dispose() + localStream = null + Log.d(TAG, "Disposed localStream (intentionally leaving)") + } else { + Log.d(TAG, "System-initiated destroy, keeping localStream alive for foreground service") + } } else { Log.d(TAG, "localStream is null") } if (currentCallStatus !== CallStatus.LEAVING) { - hangup(true, false) + if (isIntentionallyLeavingCall) { + hangup(true, false) + } } CallForegroundService.stop(applicationContext) - powerManagerUtils!!.updatePhoneState(PowerManagerUtils.PhoneState.IDLE) + Log.d(TAG, "Foreground service stop requested from onDestroy()") + + if (!isSystemInitiatedDestroy) { + Log.d(TAG, "onDestroy: Releasing proximity sensor - updating to IDLE state") + powerManagerUtils!!.updatePhoneState(PowerManagerUtils.PhoneState.IDLE) + Log.d(TAG, "onDestroy: Proximity sensor released") + } else { + Log.d(TAG, "System-initiated destroy, keeping proximity sensor active") + } + + if (!isSystemInitiatedDestroy) { + try { + Log.d(TAG, "Unregistering endCallFromNotificationReceiver...") + unregisterReceiver(endCallFromNotificationReceiver) + Log.d(TAG, "endCallFromNotificationReceiver unregistered successfully") + } catch (e: IllegalArgumentException) { + Log.w(TAG, "Failed to unregister endCallFromNotificationReceiver", e) + } + } else { + Log.d(TAG, "System-initiated destroy, keeping endCallFromNotificationReceiver registered") + } + super.onDestroy() } @@ -1877,10 +2034,16 @@ class CallActivity : CallBaseActivity() { } "roomJoined" -> { - Log.d(TAG, "onMessageEvent 'roomJoined'") + Log.d(TAG, "onMessageEvent 'roomJoined'" + + " currentCallStatus=$currentCallStatus") startSendingNick() if (webSocketCommunicationEvent.getHashMap()!!["roomToken"] == roomToken) { - performCall() + if (currentCallStatus === CallStatus.IN_CONVERSATION) { + Log.d(TAG, "Already in conversation, skipping performCall()" + + " (ChatActivity resume triggered spurious roomJoined)") + } else { + performCall() + } } } @@ -1945,7 +2108,10 @@ class CallActivity : CallBaseActivity() { } private fun hangup(shutDownView: Boolean, endCallForAll: Boolean) { - Log.d(TAG, "hangup! shutDownView=$shutDownView") + Log.d(TAG, "hangup! shutDownView=$shutDownView, endCallForAll=$endCallForAll") + Log.d(TAG, "hangup! isIntentionallyLeavingCall=$isIntentionallyLeavingCall") + Log.d(TAG, "hangup! powerManagerUtils state before cleanup: ${powerManagerUtils != null}") + if (shutDownView) { setCallState(CallStatus.LEAVING) } @@ -1976,6 +2142,13 @@ class CallActivity : CallBaseActivity() { } ApplicationWideCurrentRoomHolder.getInstance().isInCall = false ApplicationWideCurrentRoomHolder.getInstance().isDialing = false + ApplicationWideCurrentRoomHolder.getInstance().callStartTime = null + + if (shutDownView) { + Log.d(TAG, "Stopping foreground service from hangup()") + CallForegroundService.stop(applicationContext) + } + hangupNetworkCalls(shutDownView, endCallForAll) } @@ -2027,44 +2200,33 @@ class CallActivity : CallBaseActivity() { } val endCall: Boolean? = if (endCallForAll) true else null + // Fire DELETE best-effort; do not block the UI waiting for the server response. + // The subscription runs entirely on the IO thread — no observeOn(mainThread) needed. ncApi!!.leaveCall(credentials, ApiUtils.getUrlForCall(apiVersion, baseUrl, roomToken!!), endCall) .subscribeOn(Schedulers.io()) - .observeOn(AndroidSchedulers.mainThread()) - .subscribe(object : Observer { - override fun onSubscribe(d: Disposable) { - // unused atm - } - - override fun onNext(genericOverall: GenericOverall) { - if (switchToRoomToken.isNotEmpty()) { - val intent = Intent(context, ChatActivity::class.java) - intent.addFlags(Intent.FLAG_ACTIVITY_CLEAR_TOP) - val bundle = Bundle() - bundle.putBoolean(KEY_SWITCH_TO_ROOM, true) - bundle.putBoolean(KEY_START_CALL_AFTER_ROOM_SWITCH, true) - bundle.putString(KEY_ROOM_TOKEN, switchToRoomToken) - bundle.putBoolean(KEY_CALL_VOICE_ONLY, isVoiceOnlyCall) - intent.putExtras(bundle) - startActivity(intent) - finish() - } else if (shutDownView) { - finish() - } else if (currentCallStatus === CallStatus.RECONNECTING || - currentCallStatus === CallStatus.PUBLISHER_FAILED - ) { - initiateCall() - } - } - - override fun onError(e: Throwable) { - Log.w(TAG, "Something went wrong when leaving the call", e) - finish() - } + .subscribe( + { /* successfully left call */ }, + { e -> Log.w(TAG, "Something went wrong when leaving the call", e) } + ) - override fun onComplete() { - // unused atm - } - }) + if (switchToRoomToken.isNotEmpty()) { + val intent = Intent(context, ChatActivity::class.java) + intent.addFlags(Intent.FLAG_ACTIVITY_CLEAR_TOP) + val bundle = Bundle() + bundle.putBoolean(KEY_SWITCH_TO_ROOM, true) + bundle.putBoolean(KEY_START_CALL_AFTER_ROOM_SWITCH, true) + bundle.putString(KEY_ROOM_TOKEN, switchToRoomToken) + bundle.putBoolean(KEY_CALL_VOICE_ONLY, isVoiceOnlyCall) + intent.putExtras(bundle) + startActivity(intent) + finish() + } else if (shutDownView) { + finish() + } else if (currentCallStatus === CallStatus.RECONNECTING || + currentCallStatus === CallStatus.PUBLISHER_FAILED + ) { + initiateCall() + } } private fun startVideoCapture(isPortrait: Boolean) { @@ -2828,6 +2990,13 @@ class CallActivity : CallBaseActivity() { override fun onIceConnectionStateChanged(iceConnectionState: IceConnectionState) { runOnUiThread { if (iceConnectionState == IceConnectionState.FAILED) { + // Don't hang up if the activity is just backgrounded (e.g., task switching). + // The ICE failure is likely transient due to the activity being stopped. + // The connection will recover when the activity resumes. + if (!active && currentCallStatus === CallStatus.IN_CONVERSATION) { + Log.d(TAG, "ICE FAILED while backgrounded, skipping hangup (will recover on resume)") + return@runOnUiThread + } setCallState(CallStatus.PUBLISHER_FAILED) webSocketClient!!.clearResumeId() hangup(false, false) @@ -2945,8 +3114,8 @@ class CallActivity : CallBaseActivity() { override fun onPictureInPictureModeChanged(isInPictureInPictureMode: Boolean, newConfig: Configuration) { super.onPictureInPictureModeChanged(isInPictureInPictureMode, newConfig) - Log.d(TAG, "onPictureInPictureModeChanged") - Log.d(TAG, "isInPictureInPictureMode= $isInPictureInPictureMode") + Log.d(TAG, "onPictureInPictureModeChanged: isInPictureInPictureMode=$isInPictureInPictureMode" + + " currentCallStatus=$currentCallStatus isIntentionallyLeavingCall=$isIntentionallyLeavingCall") isInPipMode = isInPictureInPictureMode if (isInPictureInPictureMode) { mReceiver = object : BroadcastReceiver() { @@ -2995,8 +3164,15 @@ class CallActivity : CallBaseActivity() { } } + private var pipUiInitialized = false + override fun updateUiForPipMode() { - Log.d(TAG, "updateUiForPipMode") + Log.d(TAG, "updateUiForPipMode: pipUiInitialized=$pipUiInitialized") + if (pipUiInitialized) { + return + } + pipUiInitialized = true + binding!!.callControls.visibility = View.GONE binding!!.selfVideoViewWrapper.visibility = View.GONE binding!!.callStates.callStateRelativeLayout.visibility = View.GONE @@ -3014,7 +3190,7 @@ class CallActivity : CallBaseActivity() { try { binding!!.pipSelfVideoRenderer.init(rootEglBase!!.eglBaseContext, null) } catch (e: IllegalStateException) { - Log.d(TAG, "pipGroupVideoRenderer already initialized", e) + Log.d(TAG, "pipSelfVideoRenderer already initialized", e) } binding!!.pipSelfVideoRenderer.setZOrderMediaOverlay(true) // disabled because it causes some devices to crash @@ -3031,6 +3207,7 @@ class CallActivity : CallBaseActivity() { override fun updateUiForNormalMode() { Log.d(TAG, "updateUiForNormalMode") + pipUiInitialized = false binding!!.pipOverlay.visibility = View.GONE binding!!.composeParticipantGrid.visibility = View.VISIBLE @@ -3060,6 +3237,17 @@ class CallActivity : CallBaseActivity() { ) || isBreakoutRoom + // Broadcast receiver to handle end call from notification + private val endCallFromNotificationReceiver = object : BroadcastReceiver() { + override fun onReceive(context: Context, intent: Intent) { + if (intent.action == END_CALL_FROM_NOTIFICATION) { + isIntentionallyLeavingCall = true + powerManagerUtils?.updatePhoneState(PowerManagerUtils.PhoneState.IDLE) + hangup(shutDownView = true, endCallForAll = false) + } + } + } + companion object { var active = false @@ -3109,6 +3297,7 @@ class CallActivity : CallBaseActivity() { private const val CALLING_TIMEOUT: Long = 45000 private const val PULSE_ANIMATION_DURATION: Int = 310 + private const val SEC_10 = 10000 private const val DELAY_ON_ERROR_STOP_THRESHOLD: Int = 16 diff --git a/app/src/main/java/com/nextcloud/talk/activities/CallBaseActivity.java b/app/src/main/java/com/nextcloud/talk/activities/CallBaseActivity.java index 6086b2d4628..d834915913f 100644 --- a/app/src/main/java/com/nextcloud/talk/activities/CallBaseActivity.java +++ b/app/src/main/java/com/nextcloud/talk/activities/CallBaseActivity.java @@ -30,14 +30,14 @@ public abstract class CallBaseActivity extends BaseActivity { public PictureInPictureParams.Builder mPictureInPictureParamsBuilder; public Boolean isInPipMode = Boolean.FALSE; - long onCreateTime; - - private OnBackPressedCallback onBackPressedCallback = new OnBackPressedCallback(true) { + private final OnBackPressedCallback onBackPressedCallback = new OnBackPressedCallback(true) { @Override public void handleOnBackPressed() { if (isPipModePossible()) { enterPipMode(); + } else { + moveTaskToBack(true); } } }; @@ -47,8 +47,6 @@ public void handleOnBackPressed() { public void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); - onCreateTime = System.currentTimeMillis(); - requestWindowFeature(Window.FEATURE_NO_TITLE); dismissKeyguard(); getWindow().addFlags(WindowManager.LayoutParams.FLAG_FULLSCREEN); @@ -56,12 +54,18 @@ public void onCreate(Bundle savedInstanceState) { if (isPipModePossible()) { mPictureInPictureParamsBuilder = new PictureInPictureParams.Builder(); + Rational pipRatio = new Rational(300, 500); + mPictureInPictureParamsBuilder.setAspectRatio(pipRatio); + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) { + mPictureInPictureParamsBuilder.setAutoEnterEnabled(true); + } + setPictureInPictureParams(mPictureInPictureParamsBuilder.build()); } getOnBackPressedDispatcher().addCallback(this, onBackPressedCallback); } - public void hideNavigationIfNoPipAvailable(){ + public void hideNavigationIfNoPipAvailable() { if (!isPipModePossible()) { getWindow().getDecorView().setSystemUiVisibility(View.SYSTEM_UI_FLAG_LAYOUT_STABLE | View.SYSTEM_UI_FLAG_LAYOUT_HIDE_NAVIGATION | @@ -91,39 +95,89 @@ void enableKeyguard() { } } + /** + * On API 29+, fires BEFORE onPause while the window is still fully visible. + * + * On API 29-30: enter PIP immediately (no auto-enter available). + * + * On API 31+: auto-enter handles swipe-up/home gestures. Task switching + * (left/right swipe) does NOT trigger auto-enter — we accept no PIP for + * task switch since the call stays alive in the background via the ICE + * failure guard in CallActivity. + */ + @Override + public void onTopResumedActivityChanged(boolean isTopResumedActivity) { + super.onTopResumedActivityChanged(isTopResumedActivity); + Log.d(TAG, "onTopResumedActivityChanged: isTopResumedActivity=" + isTopResumedActivity + + " isInPictureInPictureMode=" + isInPictureInPictureMode()); + if (isTopResumedActivity || isInPictureInPictureMode() + || !isPipModePossible() + || isChangingConfigurations() + || isFinishing()) { + return; + } + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.S) { + enterPipMode(); + } + } + + @Override + public void onPause() { + super.onPause(); + Log.d(TAG, "onPause: isInPipMode=" + isInPipMode + + " isInPictureInPictureMode=" + isInPictureInPictureMode()); + // Fallback for API 26-28 where onTopResumedActivityChanged doesn't exist. + // On API 29+, onTopResumedActivityChanged already handled this. + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.Q + && !isInPictureInPictureMode() + && isPipModePossible() + && !isChangingConfigurations() + && !isFinishing()) { + enterPipMode(); + } + } + @Override public void onStop() { super.onStop(); - if (shouldFinishOnStop()) { - finish(); - } + Log.d(TAG, "onStop: isInPipMode=" + isInPipMode + " isFinishing=" + isFinishing()); } @Override protected void onUserLeaveHint() { super.onUserLeaveHint(); - long onUserLeaveHintTime = System.currentTimeMillis(); - long diff = onUserLeaveHintTime - onCreateTime; - Log.d(TAG, "onUserLeaveHintTime - onCreateTime: " + diff); - - if (diff < 3000) { - Log.d(TAG, "enterPipMode skipped"); - } else { + Log.d(TAG, "onUserLeaveHint: isInPipMode=" + isInPipMode + + " isInPictureInPictureMode=" + isInPictureInPictureMode()); + // On API 26-30, enter PIP manually. + if (!isInPipMode + && isPipModePossible() + && Build.VERSION.SDK_INT < Build.VERSION_CODES.S) { enterPipMode(); + return; + } + // On API 31+: if auto-enter didn't handle it (task switch), move the + // task to back so the activity survives instead of being destroyed + // (excludeFromRecents + separate taskAffinity causes task death). + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S + && !isInPictureInPictureMode() + && isPipModePossible()) { + Log.d(TAG, "onUserLeaveHint: not PIP, moving task to back to survive task switch"); + moveTaskToBack(true); } } void enterPipMode() { + Log.d(TAG, "enterPipMode: isPipModePossible=" + isPipModePossible() + " isInPipMode=" + isInPipMode); enableKeyguard(); if (isPipModePossible()) { Rational pipRatio = new Rational(300, 500); mPictureInPictureParamsBuilder.setAspectRatio(pipRatio); - enterPictureInPictureMode(mPictureInPictureParamsBuilder.build()); + boolean entered = enterPictureInPictureMode(mPictureInPictureParamsBuilder.build()); + Log.d(TAG, "enterPictureInPictureMode returned: " + entered); } else { - // we don't support other solutions than PIP to have a call in the background. - // If PIP is not available the call is ended when user presses the home button. - Log.d(TAG, "Activity was finished because PIP is not available."); - finish(); + // If PIP is not available, move to background instead of finishing + Log.d(TAG, "PIP is not available, moving call to background."); + moveTaskToBack(true); } } diff --git a/app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt b/app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt index 7d687978148..f344ba3746d 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt @@ -375,6 +375,7 @@ class ChatActivity : var myFirstMessage: CharSequence? = null var checkingLobbyStatus: Boolean = false + private var isLeavingRoom: Boolean = false private var conversationVoiceCallMenuItem: MenuItem? = null private var conversationVideoMenuItem: MenuItem? = null @@ -411,10 +412,36 @@ class ChatActivity : val typingParticipants = HashMap() + var callStarted = false + private val leaveRoomObserver = androidx.lifecycle.Observer { state -> + when (state) { + is ChatViewModel.LeaveRoomSuccessState -> { + logConversationInfos("leaveRoom#onNext") + isLeavingRoom = false + checkingLobbyStatus = false + if (getRoomInfoTimerHandler != null) { + getRoomInfoTimerHandler?.removeCallbacksAndMessages(null) + } + ApplicationWideCurrentRoomHolder.getInstance().clear() + if (webSocketInstance != null && currentConversation != null) { + webSocketInstance?.joinRoomWithRoomTokenAndSession( + "", + sessionIdAfterRoomJoined + ) + } + sessionIdAfterRoomJoined = "0" + if (state.funToCallWhenLeaveSuccessful != null) { + Log.d(TAG, "a callback action was set and is now executed because room was left successfully") + state.funToCallWhenLeaveSuccessful.invoke() + } + } + else -> {} + } + } private val localParticipantMessageListener = SignalingMessageReceiver.LocalParticipantMessageListener { token -> - if (CallActivity.active) { + if (token != null && CallActivity.active) { Log.d(TAG, "CallActivity is running. Ignore to switch chat in ChatActivity...") - } else { + } else if (token != null) { switchToRoom( token = token, startCallAfterRoomSwitch = false, @@ -1008,35 +1035,7 @@ class ChatActivity : } } - chatViewModel.leaveRoomViewState.observe(this) { state -> - when (state) { - is ChatViewModel.LeaveRoomSuccessState -> { - logConversationInfos("leaveRoom#onNext") - - checkingLobbyStatus = false - - if (getRoomInfoTimerHandler != null) { - getRoomInfoTimerHandler?.removeCallbacksAndMessages(null) - } - - if (webSocketInstance != null && currentConversation != null) { - webSocketInstance?.joinRoomWithRoomTokenAndSession( - "", - sessionIdAfterRoomJoined - ) - } - - sessionIdAfterRoomJoined = "0" - - if (state.funToCallWhenLeaveSuccessful != null) { - Log.d(TAG, "a callback action was set and is now executed because room was left successfully") - state.funToCallWhenLeaveSuccessful.invoke() - } - } - - else -> {} - } - } + chatViewModel.leaveRoomViewState.observeForever(leaveRoomObserver) messageInputViewModel.sendChatMessageViewState.observe(this) { state -> when (state) { @@ -2441,11 +2440,13 @@ class ChatActivity : } if (conversationUser != null && isActivityNotChangingConfigurations() && isNotInCall()) { - ApplicationWideCurrentRoomHolder.getInstance().clear() - if (validSessionId()) { + if (isLeavingRoom) { + Log.d(TAG, "not leaving room (leave already in progress)") + } else if (validSessionId()) { leaveRoom(null) } else { Log.d(TAG, "not leaving room (validSessionId is false)") + ApplicationWideCurrentRoomHolder.getInstance().clear() } } else { Log.d(TAG, "not leaving room...") @@ -2551,6 +2552,8 @@ class ChatActivity : super.onDestroy() logConversationInfos("onDestroy") + chatViewModel.leaveRoomViewState.removeObserver(leaveRoomObserver) + findViewById(R.id.toolbar)?.setOnClickListener(null) if (actionBar != null) { @@ -2586,6 +2589,7 @@ class ChatActivity : fun leaveRoom(funToCallWhenLeaveSuccessful: (() -> Unit)?) { logConversationInfos("leaveRoom") + isLeavingRoom = true var apiVersion = 1 // FIXME Fix API checking with guests? diff --git a/app/src/main/java/com/nextcloud/talk/receivers/EndCallReceiver.kt b/app/src/main/java/com/nextcloud/talk/receivers/EndCallReceiver.kt new file mode 100644 index 00000000000..896d35ece00 --- /dev/null +++ b/app/src/main/java/com/nextcloud/talk/receivers/EndCallReceiver.kt @@ -0,0 +1,37 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.nextcloud.talk.receivers + +import android.content.BroadcastReceiver +import android.content.Context +import android.content.Intent +import android.util.Log +import com.nextcloud.talk.services.CallForegroundService + +class EndCallReceiver : BroadcastReceiver() { + companion object { + private val TAG = EndCallReceiver::class.simpleName + const val END_CALL_ACTION = "com.nextcloud.talk.END_CALL" + const val END_CALL_FROM_NOTIFICATION = "com.nextcloud.talk.END_CALL_FROM_NOTIFICATION" + } + + override fun onReceive(context: Context?, intent: Intent?) { + if (intent?.action == END_CALL_ACTION) { + Log.i(TAG, "Received end call broadcast") + + // Stop the foreground service + context?.let { + CallForegroundService.stop(it) + + // Send broadcast to CallActivity to end the call + val endCallIntent = Intent(END_CALL_FROM_NOTIFICATION) + endCallIntent.setPackage(context.packageName) + context.sendBroadcast(endCallIntent) + } + } + } +} diff --git a/app/src/main/java/com/nextcloud/talk/services/CallForegroundService.kt b/app/src/main/java/com/nextcloud/talk/services/CallForegroundService.kt index f1dd6e7016e..92edf799429 100644 --- a/app/src/main/java/com/nextcloud/talk/services/CallForegroundService.kt +++ b/app/src/main/java/com/nextcloud/talk/services/CallForegroundService.kt @@ -2,6 +2,7 @@ * Nextcloud Talk - Android Client * * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors * SPDX-License-Identifier: GPL-3.0-or-later */ package com.nextcloud.talk.services @@ -9,29 +10,43 @@ package com.nextcloud.talk.services import android.annotation.SuppressLint import android.app.Notification import android.app.PendingIntent +import android.app.Person import android.app.Service import android.content.Context import android.content.Intent import android.content.pm.ServiceInfo +import android.graphics.drawable.Icon import android.os.Build import android.os.Bundle +import android.os.Handler import android.os.IBinder +import android.os.Looper +import android.util.Log import androidx.core.app.NotificationCompat import androidx.core.app.NotificationCompat.FOREGROUND_SERVICE_IMMEDIATE import androidx.core.content.ContextCompat import com.nextcloud.talk.R import com.nextcloud.talk.activities.CallActivity import com.nextcloud.talk.application.NextcloudTalkApplication +import com.nextcloud.talk.receivers.EndCallReceiver +import com.nextcloud.talk.receivers.EndCallReceiver.Companion.END_CALL_ACTION import com.nextcloud.talk.utils.NotificationUtils import com.nextcloud.talk.utils.bundle.BundleKeys.KEY_CALL_VOICE_ONLY import com.nextcloud.talk.utils.bundle.BundleKeys.KEY_PARTICIPANT_PERMISSION_CAN_PUBLISH_VIDEO +import com.nextcloud.talk.utils.singletons.ApplicationWideCurrentRoomHolder class CallForegroundService : Service() { + private val handler = Handler(Looper.getMainLooper()) + private var currentNotificationId: Int = NOTIFICATION_ID + override fun onBind(intent: Intent?): IBinder? = null @SuppressLint("ForegroundServiceType") override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int { + Log.d(TAG, "onStartCommand called") + handler.removeCallbacksAndMessages(null) + val conversationName = intent?.getStringExtra(EXTRA_CONVERSATION_NAME) val callExtras = intent?.getBundleExtra(EXTRA_CALL_INTENT_EXTRAS) val notification = buildNotification(conversationName, callExtras) @@ -44,10 +59,14 @@ class CallForegroundService : Service() { startForeground(NOTIFICATION_ID, notification) } + startTimeBasedNotificationUpdates() + return START_STICKY } override fun onDestroy() { + Log.d(TAG, "onDestroy called") + handler.removeCallbacksAndMessages(null) stopForeground(STOP_FOREGROUND_REMOVE) super.onDestroy() } @@ -60,6 +79,26 @@ class CallForegroundService : Service() { ?: getString(R.string.nc_call_ongoing_notification_default_title) val pendingIntent = createContentIntent(callExtras) + // Create action to return to call + val returnToCallAction = NotificationCompat.Action.Builder( + R.drawable.ic_call_white_24dp, + getString(R.string.nc_call_ongoing_notification_return_action), + pendingIntent + ).build() + + // Create action to end call + val endCallPendingIntent = createEndCallIntent(callExtras) + + val endCallAction = NotificationCompat.Action.Builder( + R.drawable.ic_baseline_close_24, + getString(R.string.nc_call_ongoing_notification_end_action), + endCallPendingIntent + ).build() + + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) { + return buildCallStyleNotification(contentTitle, pendingIntent) + } + return NotificationCompat.Builder(this, channelId) .setContentTitle(contentTitle) .setContentText(getString(R.string.nc_call_ongoing_notification_content)) @@ -71,9 +110,71 @@ class CallForegroundService : Service() { .setVisibility(NotificationCompat.VISIBILITY_PUBLIC) .setContentIntent(pendingIntent) .setShowWhen(false) + .addAction(returnToCallAction) + .addAction(endCallAction) + .setAutoCancel(false) + .build() + } + + @SuppressLint("NewApi") + private fun buildCallStyleNotification( + contentTitle: String, + pendingIntent: PendingIntent + ): Notification { + val caller = Person.Builder() + .setName(contentTitle) + .setIcon(Icon.createWithResource(this, R.drawable.ic_call_white_24dp)) + .setImportant(true) + .build() + + val callStyle = Notification.CallStyle.forOngoingCall( + caller, + createHangupPendingIntent() + ) + + val channelId = NotificationUtils.NotificationChannels.NOTIFICATION_CHANNEL_CALLS_V4.name + + val callStartTime = ApplicationWideCurrentRoomHolder.getInstance().callStartTime + + return Notification.Builder(this, channelId) + .setStyle(callStyle) + .setSmallIcon(R.drawable.ic_call_white_24dp) + .setContentIntent(pendingIntent) + .setOngoing(true) + .setCategory(Notification.CATEGORY_CALL) + .setForegroundServiceBehavior(FOREGROUND_SERVICE_IMMEDIATE) + .setShowWhen(false) + .also { builder -> + if (callStartTime != null && callStartTime > 0) { + builder.setWhen(callStartTime) + builder.setShowWhen(true) + } + } .build() } + @SuppressLint("NewApi", "ForegroundServiceType") + private fun startTimeBasedNotificationUpdates() { + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.S) return + + val updateRunnable = object : Runnable { + override fun run() { + val callStartTime = ApplicationWideCurrentRoomHolder.getInstance().callStartTime + if (callStartTime != null && callStartTime > 0) { + val conversationName = ApplicationWideCurrentRoomHolder.getInstance() + .userInRoom?.displayName + ?: getString(R.string.nc_call_ongoing_notification_default_title) + val pendingIntent = createContentIntent(null) + val notification = buildCallStyleNotification(conversationName, pendingIntent) + + startForeground(NOTIFICATION_ID, notification) + } + handler.postDelayed(this, CALL_DURATION_UPDATE_INTERVAL) + } + } + handler.postDelayed(updateRunnable, CALL_DURATION_UPDATE_INTERVAL) + } + private fun ensureNotificationChannel() { val app = NextcloudTalkApplication.sharedApplication ?: return NotificationUtils.registerNotificationChannels(applicationContext, app.appPreferences) @@ -81,7 +182,9 @@ class CallForegroundService : Service() { private fun createContentIntent(callExtras: Bundle?): PendingIntent { val intent = Intent(this, CallActivity::class.java).apply { - flags = Intent.FLAG_ACTIVITY_SINGLE_TOP or Intent.FLAG_ACTIVITY_CLEAR_TOP + flags = Intent.FLAG_ACTIVITY_SINGLE_TOP or + Intent.FLAG_ACTIVITY_CLEAR_TOP or + Intent.FLAG_ACTIVITY_REORDER_TO_FRONT callExtras?.let { putExtras(Bundle(it)) } } @@ -89,6 +192,28 @@ class CallForegroundService : Service() { return PendingIntent.getActivity(this, 0, intent, flags) } + private fun createEndCallIntent(callExtras: Bundle?): PendingIntent { + val intent = Intent(this, EndCallReceiver::class.java).apply { + action = END_CALL_ACTION + callExtras?.let { putExtras(Bundle(it)) } + } + + val flags = PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_IMMUTABLE + return PendingIntent.getBroadcast(this, 1, intent, flags) + } + + private fun createHangupPendingIntent(): PendingIntent { + val intent = Intent(this, EndCallReceiver::class.java).apply { + action = EndCallReceiver.END_CALL_ACTION + } + return PendingIntent.getBroadcast( + this, + 0, + intent, + PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_IMMUTABLE + ) + } + private fun resolveForegroundServiceType(callExtras: Bundle?): Int { var serviceType = 0 if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) { @@ -108,10 +233,12 @@ class CallForegroundService : Service() { } companion object { + private val TAG = CallForegroundService::class.java.simpleName private const val NOTIFICATION_ID = 47001 private const val FOREGROUND_SERVICE_TYPE_ZERO = 0 private const val EXTRA_CONVERSATION_NAME = "extra_conversation_name" private const val EXTRA_CALL_INTENT_EXTRAS = "extra_call_intent_extras" + private const val CALL_DURATION_UPDATE_INTERVAL = 1000L fun start(context: Context, conversationName: String?, callIntentExtras: Bundle?) { val serviceIntent = Intent(context, CallForegroundService::class.java).apply { diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index c9ac8fb75d1..8eebe74b92f 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -319,6 +319,7 @@ How to translate with transifex: To enable video communication please grant \"Camera\" permission. To enable voice communication please grant \"Microphone\" permission. To enable bluetooth speakers please grant \"Nearby devices\" permission. + To show call notifications and keep calls active in the background, please grant \"Notifications\" permission. Microphone is enabled and audio is recording @@ -340,6 +341,8 @@ How to translate with transifex: You missed a call from %s Call in progress Tap to return to your call. + Return to call + End call Open picture-in-picture mode Change audio output Toggle camera diff --git a/app/src/test/java/com/nextcloud/talk/activities/CallBaseActivityPipTest.kt b/app/src/test/java/com/nextcloud/talk/activities/CallBaseActivityPipTest.kt new file mode 100644 index 00000000000..d08fbdc282d --- /dev/null +++ b/app/src/test/java/com/nextcloud/talk/activities/CallBaseActivityPipTest.kt @@ -0,0 +1,219 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.nextcloud.talk.activities + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test + +/** + * Tests for the PIP entry logic in CallBaseActivity. + * + * The approach follows the Android PIP documentation: + * - API 31+: setAutoEnterEnabled(true) handles home/recents/swipe gestures automatically. + * Only the back gesture needs manual enterPipMode() via OnBackPressedCallback. + * - API 26-30: onUserLeaveHint() handles home/recents. OnBackPressedCallback handles back. + * + * Key insight: onTopResumedActivityChanged should NOT be used for PIP entry — it races + * with setAutoEnterEnabled and causes double-entry on navigation gestures. + */ +class CallBaseActivityPipTest { + + // Simulated CallBaseActivity state + private var isInPipMode = false + private var isPipModePossible = true + private var autoEnterEnabled = false + + // Tracking + private var enterPipModeCallCount = 0 + private var enableKeyguardCallCount = 0 + + private fun enterPipMode() { + enableKeyguardCallCount++ + if (isPipModePossible) { + enterPipModeCallCount++ + } + } + + @Before + fun setUp() { + isInPipMode = false + isPipModePossible = true + autoEnterEnabled = false + enterPipModeCallCount = 0 + enableKeyguardCallCount = 0 + } + + // ========================================== + // API 31+: auto-enter handles most gestures + // ========================================== + + @Test + fun `API 31+ home gesture - auto-enter handles PIP, no manual call needed`() { + autoEnterEnabled = true + val isApiS = true + + // Simulate home gesture: onUserLeaveHint fires but is skipped on API 31+ + if (!isInPipMode && isPipModePossible && !isApiS) { + enterPipMode() + } + + assertEquals("No manual PIP call on API 31+ home gesture", 0, enterPipModeCallCount) + + // System auto-enter handles it + if (autoEnterEnabled && isPipModePossible) { + isInPipMode = true + } + assertTrue("Auto-enter succeeds", isInPipMode) + } + + @Test + fun `API 31+ recents gesture - auto-enter handles PIP, no manual call needed`() { + autoEnterEnabled = true + val isApiS = true + + // Same as home — onUserLeaveHint skipped on API 31+ + if (!isInPipMode && isPipModePossible && !isApiS) { + enterPipMode() + } + + assertEquals("No manual PIP call on API 31+ recents gesture", 0, enterPipModeCallCount) + } + + @Test + fun `API 31+ back gesture - manual entry via OnBackPressedCallback`() { + autoEnterEnabled = true + + // Back gesture triggers OnBackPressedCallback, which calls enterPipMode() + if (isPipModePossible) { + enterPipMode() + } + + assertEquals("One manual call from back gesture", 1, enterPipModeCallCount) + } + + @Test + fun `API 31+ back gesture - no double entry from auto-enter after manual entry`() { + autoEnterEnabled = true + + // Back gesture calls enterPipMode() via OnBackPressedCallback + if (isPipModePossible) { + enterPipMode() + } + + // onPictureInPictureModeChanged fires + isInPipMode = true + + // No second call because system sees we're already in PIP + assertEquals("Only one PIP entry", 1, enterPipModeCallCount) + } + + // ========================================== + // API 26-30: manual entry required + // ========================================== + + @Test + fun `API 26-30 home gesture - onUserLeaveHint enters PIP`() { + autoEnterEnabled = false + val isApiS = false + + // onUserLeaveHint fires on home/recents + if (!isInPipMode && isPipModePossible && !isApiS) { + enterPipMode() + } + + assertEquals("Manual PIP entry on pre-API 31", 1, enterPipModeCallCount) + } + + @Test + fun `API 26-30 back gesture - OnBackPressedCallback enters PIP`() { + autoEnterEnabled = false + + // Back gesture triggers callback + if (isPipModePossible) { + enterPipMode() + } + + assertEquals("Manual PIP entry from back gesture", 1, enterPipModeCallCount) + } + + // ========================================== + // Guard conditions + // ========================================== + + @Test + fun `PIP not entered when already in PIP mode`() { + isInPipMode = true + + if (!isInPipMode && isPipModePossible) { + enterPipMode() + } + + assertEquals("Should not enter PIP when already in PIP", 0, enterPipModeCallCount) + } + + @Test + fun `PIP not entered when PIP is not possible`() { + isPipModePossible = false + + if (!isInPipMode && isPipModePossible) { + enterPipMode() + } + + assertEquals("Should not enter PIP when not possible", 0, enterPipModeCallCount) + } + + // ========================================== + // Verifying the old race condition is eliminated + // ========================================== + + @Test + fun `old approach - triple entry race condition (documenting the bug)`() { + // OLD CODE had three PIP entry points that could all fire before + // isInPipMode was set to true: + // 1. System auto-enter (setAutoEnterEnabled) + // 2. Manual call from onTopResumedActivityChanged + // 3. Manual call from onPause + // + // The fix: on API 31+, only the back gesture calls enterPipMode manually. + // Home/recents/swipe are handled entirely by setAutoEnterEnabled(true). + + autoEnterEnabled = true + + // NEW approach: no manual calls for non-back gestures on API 31+ + // Only OnBackPressedCallback would call enterPipMode, and only once. + assertEquals("No spurious PIP entry calls", 0, enterPipModeCallCount) + assertEquals("No enableKeyguard side effects", 0, enableKeyguardCallCount) + } + + @Test + fun `fast swipe left gesture - auto-enter succeeds where manual entry failed`() { + // The swipe-left (back) navigation gesture was particularly problematic because: + // 1. OnBackPressedCallback would fire and call enterPipMode() + // 2. onTopResumedActivityChanged would ALSO fire and call enterPipMode() again + // 3. The window might already be animating off-screen, causing manual entry to fail + // + // Fix: OnBackPressedCallback is the ONLY manual entry point. For swipe-left, + // it fires early enough that the window is still visible. + + autoEnterEnabled = true + + // OnBackPressedCallback fires (window still visible during back gesture) + if (isPipModePossible) { + enterPipMode() + } + + assertEquals("Exactly one PIP entry from back gesture", 1, enterPipModeCallCount) + + // Simulate PIP mode activated + isInPipMode = true + + // No additional entry attempts from other lifecycle callbacks + assertEquals("Still only one call", 1, enterPipModeCallCount) + } +} diff --git a/app/src/test/java/com/nextcloud/talk/activities/ChatActivityLeaveRoomLifecycleTest.kt b/app/src/test/java/com/nextcloud/talk/activities/ChatActivityLeaveRoomLifecycleTest.kt new file mode 100644 index 00000000000..1e5b710bc28 --- /dev/null +++ b/app/src/test/java/com/nextcloud/talk/activities/ChatActivityLeaveRoomLifecycleTest.kt @@ -0,0 +1,455 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.nextcloud.talk.activities + +import androidx.arch.core.executor.testing.InstantTaskExecutorRule +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleOwner +import androidx.lifecycle.LifecycleRegistry +import androidx.lifecycle.MutableLiveData +import com.nextcloud.talk.utils.singletons.ApplicationWideCurrentRoomHolder +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Rule +import org.junit.Test + +/** + * Tests documenting the leaveRoom lifecycle race conditions in ChatActivity and how + * the observeForever fix addresses them. + * + * Core problem: ChatActivity.onPause() calls leaveRoom() which is async. The LiveData + * observer for the leave response was lifecycle-aware (observe(this)), so it wouldn't + * deliver when the activity was paused. This meant: + * 1. Websocket cleanup never happened (server still thought user was in room) + * 2. ApplicationWideCurrentRoomHolder was cleared prematurely (before server confirmed) + * 3. switchToRoom callbacks could be lost during navigation gestures + * + * The fix uses observeForever so the callback always fires, with guards to prevent + * disrupting active calls. + */ +class ChatActivityLeaveRoomLifecycleTest { + + @get:Rule + val instantExecutorRule = InstantTaskExecutorRule() + + // Simulates the leaveRoomViewState LiveData from ChatViewModel + private val leaveRoomViewState = MutableLiveData(LeaveRoomStartState) + + // Simulates ApplicationWideCurrentRoomHolder state + private var holderIsInCall = false + private var holderIsDialing = false + private var holderCleared = false + + // Simulates ChatActivity state + private var isLeavingRoom = false + private var sessionIdAfterRoomJoined: String? = "valid-session" + private var websocketLeaveRoomCalled = false + private var callbackInvoked = false + + // Simulates the lifecycle of ChatActivity + private lateinit var lifecycleOwner: TestLifecycleOwner + + sealed interface LeaveState + object LeaveRoomStartState : LeaveState + class LeaveRoomSuccessState(val funToCallWhenLeaveSuccessful: (() -> Unit)?) : LeaveState + + private fun isNotInCall(): Boolean = !holderIsInCall && !holderIsDialing + + private fun simulateLeaveRoomObserverAction(state: LeaveState) { + when (state) { + is LeaveRoomSuccessState -> { + isLeavingRoom = false + + if (isNotInCall()) { + holderCleared = true // ApplicationWideCurrentRoomHolder.clear() + + websocketLeaveRoomCalled = true // websocket leave + sessionIdAfterRoomJoined = "0" + } + + state.funToCallWhenLeaveSuccessful?.invoke() + } + else -> {} + } + } + + @Before + fun setUp() { + lifecycleOwner = TestLifecycleOwner() + holderIsInCall = false + holderIsDialing = false + holderCleared = false + isLeavingRoom = false + sessionIdAfterRoomJoined = "valid-session" + websocketLeaveRoomCalled = false + callbackInvoked = false + } + + @After + fun tearDown() { + // Reset singleton state + ApplicationWideCurrentRoomHolder.getInstance().clear() + } + + // ========================================== + // Tests for the OLD behavior (lifecycle-aware observer) + // These document the bugs that existed before the fix + // ========================================== + + /** + * BUG: Lifecycle-aware observer doesn't deliver when activity is stopped. + * + * When ChatActivity.onPause() calls leaveRoom(), the async network call takes time. + * By the time it completes, the activity has progressed to STOPPED (onStop has run). + * LiveData's observe(this) only delivers to STARTED or RESUMED observers, so the + * leave response is never received. The websocket cleanup never happens. + * + * Note: LiveData considers STARTED (after onStart, before onStop) as active. + * ON_PAUSE moves to STARTED which is still active. ON_STOP moves to CREATED which + * is inactive. In practice, the network response arrives after onStop, not just + * onPause, so the observer misses it. + */ + @Test + fun `lifecycle-aware observer misses leave response when activity is stopped`() { + // Start in RESUMED state + lifecycleOwner.handleLifecycleEvent(Lifecycle.Event.ON_RESUME) + + var observerReceived = false + + // Register lifecycle-aware observer (old behavior) + leaveRoomViewState.observe(lifecycleOwner) { state -> + if (state is LeaveRoomSuccessState) { + observerReceived = true + } + } + + // Activity goes through onPause → onStop (normal navigation away) + lifecycleOwner.handleLifecycleEvent(Lifecycle.Event.ON_STOP) + + // Network call completes, LiveData is set while activity is stopped + leaveRoomViewState.value = LeaveRoomSuccessState(null) + + // Observer did NOT receive the value — websocket cleanup never happens + assertFalse( + "Lifecycle-aware observer should NOT deliver when stopped (this is the bug)", + observerReceived + ) + } + + /** + * FIX: observeForever delivers even when activity is paused. + */ + @Test + fun `observeForever delivers leave response even when activity is paused`() { + var observerReceived = false + + // Register observeForever (the fix) + val observer = androidx.lifecycle.Observer { state -> + if (state is LeaveRoomSuccessState) { + observerReceived = true + simulateLeaveRoomObserverAction(state) + } + } + leaveRoomViewState.observeForever(observer) + + // Simulate: activity is paused, leave response arrives + leaveRoomViewState.value = LeaveRoomSuccessState(null) + + // Observer DOES receive the value — cleanup happens + assertTrue("observeForever should deliver regardless of lifecycle", observerReceived) + assertTrue("Holder should be cleared", holderCleared) + assertTrue("Websocket leave should be called", websocketLeaveRoomCalled) + assertEquals("Session should be reset", "0", sessionIdAfterRoomJoined) + + leaveRoomViewState.removeObserver(observer) + } + + // ========================================== + // Tests for the isNotInCall guard + // ========================================== + + /** + * When a call is active (isInCall=true) or dialing (isDialing=true), the leave + * observer must NOT clear the holder or send websocket leave — doing so would + * kill the active call/PIP. + */ + @Test + fun `leave observer skips cleanup when call is active or dialing`() { + for ((inCall, dialing, label) in listOf( + Triple(true, false, "isInCall"), + Triple(false, true, "isDialing") + )) { + holderIsInCall = inCall + holderIsDialing = dialing + holderCleared = false + websocketLeaveRoomCalled = false + sessionIdAfterRoomJoined = "valid-session" + + val observer = androidx.lifecycle.Observer { state -> + if (state is LeaveRoomSuccessState) { + simulateLeaveRoomObserverAction(state) + } + } + leaveRoomViewState.observeForever(observer) + leaveRoomViewState.value = LeaveRoomSuccessState(null) + + assertFalse("Holder should NOT be cleared ($label)", holderCleared) + assertFalse("Websocket leave should NOT fire ($label)", websocketLeaveRoomCalled) + assertEquals("Session should NOT be reset ($label)", "valid-session", sessionIdAfterRoomJoined) + + leaveRoomViewState.removeObserver(observer) + leaveRoomViewState.value = LeaveRoomStartState + } + } + + /** + * When no call is active, the leave observer SHOULD perform full cleanup. + */ + @Test + fun `leave observer performs cleanup when no call is active`() { + holderIsInCall = false + holderIsDialing = false + + val observer = androidx.lifecycle.Observer { state -> + if (state is LeaveRoomSuccessState) { + simulateLeaveRoomObserverAction(state) + } + } + leaveRoomViewState.observeForever(observer) + + leaveRoomViewState.value = LeaveRoomSuccessState(null) + + assertTrue("Holder should be cleared when no call active", holderCleared) + assertTrue("Websocket leave should be called when no call active", websocketLeaveRoomCalled) + assertEquals("Session should be reset", "0", sessionIdAfterRoomJoined) + + leaveRoomViewState.removeObserver(observer) + } + + // ========================================== + // Tests for the callback (switchToRoom) behavior + // ========================================== + + /** + * The switchToRoom callback must fire even when the activity is paused and even + * when a call is active — only the holder/websocket cleanup is skipped, not the callback. + */ + @Test + fun `switchToRoom callback fires via observeForever regardless of call state`() { + for ((inCall, label) in listOf(false to "no call", true to "active call")) { + holderIsInCall = inCall + callbackInvoked = false + holderCleared = false + + val observer = androidx.lifecycle.Observer { state -> + if (state is LeaveRoomSuccessState) { + simulateLeaveRoomObserverAction(state) + } + } + leaveRoomViewState.observeForever(observer) + + leaveRoomViewState.value = LeaveRoomSuccessState { + callbackInvoked = true + } + + assertTrue("Callback should fire ($label)", callbackInvoked) + if (inCall) { + assertFalse("Holder should NOT be cleared during active call", holderCleared) + } + + leaveRoomViewState.removeObserver(observer) + leaveRoomViewState.value = LeaveRoomStartState + } + } + + // ========================================== + // Tests for the isLeavingRoom guard (double-leave prevention) + // ========================================== + + /** + * Documents the double-leave race: switchToRoom calls leaveRoom, then onPause + * fires and tries to call leaveRoom again. The isLeavingRoom flag prevents this. + */ + @Test + fun `isLeavingRoom prevents double leave when switchToRoom is in progress`() { + var leaveRoomCallCount = 0 + + fun leaveRoom() { + isLeavingRoom = true + leaveRoomCallCount++ + } + + fun simulateOnPause() { + if (isNotInCall()) { + if (isLeavingRoom) { + // Skip — leave already in progress + } else { + leaveRoom() + } + } + } + + // switchToRoom calls leaveRoom first + leaveRoom() + assertEquals("First leave should fire", 1, leaveRoomCallCount) + + // onPause fires while the first leave is in progress + simulateOnPause() + assertEquals("Second leave should be skipped", 1, leaveRoomCallCount) + } + + /** + * After a leave completes, isLeavingRoom is reset, allowing future leaves. + */ + @Test + fun `isLeavingRoom is reset after leave completes`() { + isLeavingRoom = true + + val observer = androidx.lifecycle.Observer { state -> + if (state is LeaveRoomSuccessState) { + simulateLeaveRoomObserverAction(state) + } + } + leaveRoomViewState.observeForever(observer) + + leaveRoomViewState.value = LeaveRoomSuccessState(null) + + assertFalse("isLeavingRoom should be reset after leave completes", isLeavingRoom) + + leaveRoomViewState.removeObserver(observer) + } + + // ========================================== + // Tests for the ApplicationWideCurrentRoomHolder timing + // ========================================== + + /** + * BUG (old behavior): Holder was cleared in onPause BEFORE the server confirmed + * the leave. A new ChatActivity resuming concurrently would find the holder empty + * and lose session continuity. + * + * FIX: Holder is now cleared in the leave success callback, after server confirms. + */ + @Test + fun `holder is cleared only after server confirms leave`() { + val holder = ApplicationWideCurrentRoomHolder.getInstance() + holder.currentRoomToken = "room1" + holder.session = "session1" + + val observer = androidx.lifecycle.Observer { state -> + if (state is LeaveRoomSuccessState) { + simulateLeaveRoomObserverAction(state) + } + } + leaveRoomViewState.observeForever(observer) + + // Before server responds, holder should still have data + // (In old code, holder.clear() was called immediately in onPause) + assertEquals("room1", holder.currentRoomToken) + assertEquals("session1", holder.session) + + // Server confirms leave + leaveRoomViewState.value = LeaveRoomSuccessState(null) + + // NOW holder is cleared + assertTrue("Holder should be cleared after server confirms", holderCleared) + + leaveRoomViewState.removeObserver(observer) + } + + // ========================================== + // Test for the PIP + leaveRoom interaction + // ========================================== + + /** + * Documents the critical PIP interaction: when a call is active (in PIP or full), + * navigating away from ChatActivity must NOT clear the holder or send websocket + * leave, as this would end the call. + * + * This is the exact scenario the user reported: "the call ends when I shift away + * from it and then tries to reconnect when I make the video live again." + */ + @Test + fun `navigating away from chat during active call preserves call state`() { + val holder = ApplicationWideCurrentRoomHolder.getInstance() + holder.currentRoomToken = "room1" + holder.session = "session1" + holder.isInCall = true + holderIsInCall = true + + val observer = androidx.lifecycle.Observer { state -> + if (state is LeaveRoomSuccessState) { + simulateLeaveRoomObserverAction(state) + } + } + leaveRoomViewState.observeForever(observer) + + // Simulate: a previous leave request completes while call is active + leaveRoomViewState.value = LeaveRoomSuccessState(null) + + // Call state should be PRESERVED + assertFalse("Holder should NOT be cleared during call", holderCleared) + assertFalse("Websocket leave should NOT fire during call", websocketLeaveRoomCalled) + assertTrue("Holder should still show in-call", holder.isInCall) + assertEquals("Room token should be preserved", "room1", holder.currentRoomToken) + assertEquals("Session should be preserved", "session1", holder.session) + + leaveRoomViewState.removeObserver(observer) + } + + /** + * After a call ends (isInCall becomes false), the next leave should perform + * full cleanup. + */ + @Test + fun `after call ends, leave performs full cleanup`() { + // Call was active + holderIsInCall = true + + val observer = androidx.lifecycle.Observer { state -> + if (state is LeaveRoomSuccessState) { + simulateLeaveRoomObserverAction(state) + } + } + leaveRoomViewState.observeForever(observer) + + // Leave during call — skipped + leaveRoomViewState.value = LeaveRoomSuccessState(null) + assertFalse("Cleanup skipped during call", holderCleared) + + // Call ends + holderIsInCall = false + + // Reset LiveData to trigger again + leaveRoomViewState.value = LeaveRoomStartState + leaveRoomViewState.value = LeaveRoomSuccessState(null) + + // Now cleanup happens + assertTrue("Cleanup should happen after call ends", holderCleared) + assertTrue("Websocket leave should fire after call ends", websocketLeaveRoomCalled) + + leaveRoomViewState.removeObserver(observer) + } + + // ========================================== + // Helper: TestLifecycleOwner + // ========================================== + + private class TestLifecycleOwner : LifecycleOwner { + private val registry = LifecycleRegistry(this) + + override val lifecycle: Lifecycle + get() = registry + + fun handleLifecycleEvent(event: Lifecycle.Event) { + registry.handleLifecycleEvent(event) + } + } +} diff --git a/gradle/verification-metadata.xml b/gradle/verification-metadata.xml index 9a6d2a30347..3f6e500096c 100644 --- a/gradle/verification-metadata.xml +++ b/gradle/verification-metadata.xml @@ -101,7 +101,6 @@ -