Fix crash on rapid double-clicking video record button

[davidjiagoogle]

Summary

Fixes an issue (b/535645067) where double-clicking or rapidly tapping the video recording button in quick succession crashed the app with IllegalStateException: A recording is already in progress.

Changes Made

1. Added CaptureButtonUiState.Enabled.Recording.Pending: Created a new UI state representing the initialization window when video recording is starting.
2. Mapped VideoRecordingState.Starting: Mapped VideoRecordingState.Starting to CaptureButtonUiState.Enabled.Recording.Pending instead of Idle in CaptureButtonUiStateAdapter.kt.
3. Updated CaptureButtonComponents.kt: Added Recording.Pending to the ignored/no-op states during click handling (onKeyUp), preventing rapid consecutive clicks from sending duplicate StartRecordingEvents.
4. Defensive Error Handling in CameraSession.kt: Replaced throwing an unhandled IllegalStateException with logging a warning when receiving a duplicate StartRecordingEvent during active recording.
5. Unit Tests: Updated CaptureButtonUiStateAdapterTest.kt to assert the new Pending state.

Bug: b/535645067

[gemini-code-assist]

Code Review

This pull request introduces a new Pending state to CaptureButtonUiState.Enabled.Recording to represent when a video recording request has been sent but is still initializing. This state is mapped from VideoRecordingState.Starting in the adapter and handled in the capture button components and tests. Additionally, CameraSession now logs a warning instead of throwing an exception if a start recording event is received while a recording is already in progress. Feedback was provided regarding a potential bug where releasing the capture button during the Pending state would ignore the key-up event, causing the recording to continue indefinitely; a code suggestion was provided to handle this state in onKeyUp.

[gemini-code-assist]

If a user starts a video recording via a long-press but releases the button quickly while the UI state is still in CaptureButtonUiState.Enabled.Recording.Pending (before transitioning to PressedRecording), the release event is ignored. This is because the isLongPressing block in onKeyUp (lines 251-258) only checks for PressedRecording to trigger onStopRecording(). As a result, the recording will continue indefinitely with the UI stuck in the pressed state even though the user has released their finger.\n\nTo fix this, please update the check in onKeyUp to also handle the Pending state:\n\nkotlin\nif (!isLocked &&\n (currentUiState.value is CaptureButtonUiState.Enabled.Recording.PressedRecording ||\n currentUiState.value is CaptureButtonUiState.Enabled.Recording.Pending)\n) {\n Log.d(TAG, "Stopping recording")\n onStopRecording()\n}\n

[temcguir]

I agree with @gemini-code-assist's inline comment to allow stopping the recording by releasing the long-press while in the Starting state.

However, applying the bot's fix will expose a coroutine race condition in CaptureControllerImpl.kt. Because CameraSession.kt silently ignores StopRecordingEvents if it isn't actively recording yet, a rapid start/stop sequence will freeze the camera into an endless recording loop. If the UI dispatcher executes the stop coroutine before the start coroutine sends its event, the Stop event is sent to the channel first (and ignored), and then the Start event triggers the recording infinitely.

Could we also include a quick fix in CaptureControllerImpl.kt in this PR to synchronously cancel the job?

    // CaptureControllerImpl.kt
    override fun stopVideoRecording() {
        Log.d(TAG, "stopVideoRecording")
        // Cancel job synchronously before sending Stop to prevent the race condition
        recordingJob?.cancel() 
        recordingJob = null
        scope.launch {
            cameraSystem.stopVideoRecording()
        }
    }

[gemini-code-assist]

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

[temcguir]

It would be awesome to add a quick unit test to PreviewViewModelTest.kt to ensure this specific fast-release cancellation doesn't regress.

First, add a diagnostic counter to FakeCameraSystem.kt:

    var numVideoRecordingStarts = 0
    
    override suspend fun startVideoRecording(
        saveLocation: SaveLocation,
        onVideoRecord: (OnVideoRecordEvent) -> Unit
    ) {
        if (!useCasesBinded) {
            throw IllegalStateException("Usecases not bound")
        }
        numVideoRecordingStarts++
        recordingInProgress = true
    }

Then we can add the following test directly inside PreviewViewModelTest.kt to verify that rapidly firing "stop" immediately cancels the "start" job:

    @Test
    fun fastStartAndStopVideoRecording_cancelsRecording() = runTest(StandardTestDispatcher()) {
        startCameraUntilRunning()
        
        // Start and stop immediately without advancing the dispatcher
        previewViewModel.captureController.startVideoRecording()
        previewViewModel.captureController.stopVideoRecording()
        
        // Let the coroutines execute
        advanceUntilIdle()
        
        // Verify that because we cancelled the job synchronously, the start implementation
        // was bypassed completely to protect us from the channel race condition!
        assertThat(cameraSystem.numVideoRecordingStarts).isEqualTo(0)
    }

[temcguir]

Given we are mapping this directly from VideoRecordingState.Starting, could we rename Pending to Starting here (and throughout the PR)? It feels like it captures the actual state of the camera backend much more accurately than "Pending".

            /**
             * The video recording request has been sent and is starting.
             */
            data object Starting : Recording

[temcguir]

I agree with @gemini-code-assist's inline comment to allow stopping the recording by releasing the long-press while in the Starting state.

However, applying the bot's fix will expose a coroutine race condition in CaptureControllerImpl.kt. Because CameraSession.kt silently ignores StopRecordingEvents if it isn't actively recording yet, a rapid start/stop sequence will freeze the camera into an endless recording loop. If the UI dispatcher executes the stop coroutine before the start coroutine sends its event, the Stop event is sent to the channel first (and ignored), and then the Start event triggers the recording infinitely.

Could we also include a quick fix in CaptureControllerImpl.kt in this PR to synchronously cancel the job?

    // CaptureControllerImpl.kt
    override fun stopVideoRecording() {
        Log.d(TAG, "stopVideoRecording")
        // Cancel job synchronously before sending Stop to prevent the race condition
        recordingJob?.cancel() 
        recordingJob = null
        scope.launch {
            cameraSystem.stopVideoRecording()
        }
    }