Skip to content

multipleEventCutter #491

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
davidjiagoogle wants to merge 2 commits into
base: main
Choose a base branch
from
Open

multipleEventCutter #491

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
38bbf5a
multipleEventCutter
davidjiagoogle Apr 2, 2026
dcbd388
Address PR #491 comments: improve error handling and add KDoc
davidjiagoogle Apr 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
209 changes: 113 additions & 96 deletions ...r/impl/src/main/java/com/google/jetpackcamera/ui/controller/impl/CaptureControllerImpl.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ class CaptureControllerImpl(
private var recordingJob: Job? = null
private val job = Job(parent = coroutineContext[Job])
private val scope = CoroutineScope(coroutineContext + job)
private val multipleEventsCutter = MultipleEventsCutter()

override fun captureImage(contentResolver: ContentResolver) {
if (externalCaptureMode == ExternalCaptureMode.VideoCapture) {
Expand All @@ -102,56 +103,66 @@ class CaptureControllerImpl(
)
return
}
Log.d(TAG, "captureImage")
scope.launch {
val (saveLocation, progress) = nextSaveLocation(
saveMode,
externalCaptureMode,
externalCapturesCallback
)
captureImageInternal(
saveLocation = saveLocation,
doTakePicture = {
cameraSystem.takePicture(contentResolver, saveLocation) {
trackedCaptureUiState.update { old ->
old.copy(lastBlinkTimeStamp = System.currentTimeMillis())
}
}.savedUri
},
onSuccess = { savedUri ->
val event = if (progress != null) {
ImageCaptureEvent.SequentialImageSaved(savedUri, progress)
} else {
if (saveLocation is SaveLocation.Cache) {
ImageCaptureEvent.SingleImageCached(savedUri)
} else {
ImageCaptureEvent.SingleImageSaved(savedUri)
}
}
if (saveLocation !is SaveLocation.Cache) {
imageWellController.updateLastCapturedMedia()
} else {
savedUri?.let {
scope.launch {
postCurrentMediaToMediaRepository(
mediaRepository,
MediaDescriptor.Content.Image(it, null, true)
)
multipleEventsCutter.processEvent {
Copy link
Copy Markdown
Collaborator

@temcguir temcguir Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of wrapping this in processEvent, cameraSystem.takePicture() and cameraSystem.startVideoRecording() should internally update an 'in-progress' state on CameraState. This will flow down to the UI state, disable the capture button, and naturally prevent a second click.

Log.d(TAG, "captureImage")
scope.launch {
try {
val (saveLocation, progress) = nextSaveLocation(
saveMode,
externalCaptureMode,
externalCapturesCallback
)
captureImageInternal(
saveLocation = saveLocation,
doTakePicture = {
cameraSystem.takePicture(contentResolver, saveLocation) {
trackedCaptureUiState.update { old ->
old.copy(lastBlinkTimeStamp = System.currentTimeMillis())
}
}.savedUri
},
onSuccess = { savedUri ->
val event = if (progress != null) {
ImageCaptureEvent.SequentialImageSaved(savedUri, progress)
} else {
if (saveLocation is SaveLocation.Cache) {
ImageCaptureEvent.SingleImageCached(savedUri)
} else {
ImageCaptureEvent.SingleImageSaved(savedUri)
}
}
if (saveLocation !is SaveLocation.Cache) {
imageWellController.updateLastCapturedMedia()
} else {
savedUri?.let {
scope.launch {
postCurrentMediaToMediaRepository(
mediaRepository,
MediaDescriptor.Content.Image(it, null, true)
)
}
}
}
captureEvents.trySend(event)
multipleEventsCutter.done()
},
onFailure = { exception ->
val event = if (progress != null) {
ImageCaptureEvent.SequentialImageCaptureError(exception, progress)
} else {
ImageCaptureEvent.SingleImageCaptureError(exception)
}
}
}
captureEvents.trySend(event)
},
onFailure = { exception ->
val event = if (progress != null) {
ImageCaptureEvent.SequentialImageCaptureError(exception, progress)
} else {
ImageCaptureEvent.SingleImageCaptureError(exception)
}

captureEvents.trySend(event)
captureEvents.trySend(event)
multipleEventsCutter.done()
}
)
} catch (exception: Exception) {
Log.e(TAG, "captureImage error: $exception")
multipleEventsCutter.done()
captureEvents.trySend(ImageCaptureEvent.SingleImageCaptureError(exception))
}
)
}
}
}

Expand All @@ -169,69 +180,75 @@ class CaptureControllerImpl(
return
}
Log.d(TAG, "startVideoRecording")
recordingJob = scope.launch {
val cookie = "Video-${videoCaptureStartedCount.incrementAndGet()}"
val (saveLocation, _) = nextSaveLocation(
saveMode,
externalCaptureMode,
externalCapturesCallback
)
try {
cameraSystem.startVideoRecording(saveLocation) {
var snackbarToShow: SnackbarData?
when (it) {
is OnVideoRecordEvent.OnVideoRecorded -> {
Log.d(TAG, "cameraSystem.startRecording OnVideoRecorded")
val event = if (saveLocation is SaveLocation.Cache) {
VideoCaptureEvent.VideoCached(it.savedUri)
} else {
VideoCaptureEvent.VideoSaved(it.savedUri)
}
multipleEventsCutter.processEvent {
recordingJob = scope.launch {
val cookie = "Video-${videoCaptureStartedCount.incrementAndGet()}"
try {
val (saveLocation, _) = nextSaveLocation(
saveMode,
externalCaptureMode,
externalCapturesCallback
)
cameraSystem.startVideoRecording(saveLocation) {
var snackbarToShow: SnackbarData?
when (it) {
is OnVideoRecordEvent.OnVideoRecorded -> {
Log.d(TAG, "cameraSystem.startRecording OnVideoRecorded")
val event = if (saveLocation is SaveLocation.Cache) {
VideoCaptureEvent.VideoCached(it.savedUri)
} else {
VideoCaptureEvent.VideoSaved(it.savedUri)
}

if (saveLocation !is SaveLocation.Cache) {
imageWellController.updateLastCapturedMedia()
} else {
scope.launch {
postCurrentMediaToMediaRepository(
mediaRepository,
MediaDescriptor.Content.Video(it.savedUri, null, true)
if (saveLocation !is SaveLocation.Cache) {
imageWellController.updateLastCapturedMedia()
} else {
scope.launch {
postCurrentMediaToMediaRepository(
mediaRepository,
MediaDescriptor.Content.Video(it.savedUri, null, true)
)
}
}

captureEvents.trySend(event)
// don't display snackbar for successful capture
snackbarToShow = if (saveLocation is SaveLocation.Cache) {
null
} else {
SnackbarData(
cookie = cookie,
stringResource = R.string.toast_video_capture_success,
withDismissAction = true,
testTag = VIDEO_CAPTURE_SUCCESS_TAG
)
}
multipleEventsCutter.done()
}

captureEvents.trySend(event)
// don't display snackbar for successful capture
snackbarToShow = if (saveLocation is SaveLocation.Cache) {
null
} else {
SnackbarData(
is OnVideoRecordEvent.OnVideoRecordError -> {
Log.d(TAG, "cameraSystem.startRecording OnVideoRecordError")
captureEvents.trySend(VideoCaptureEvent.VideoCaptureError(it.error))
snackbarToShow = SnackbarData(
cookie = cookie,
stringResource = R.string.toast_video_capture_success,
stringResource = R.string.toast_video_capture_failure,
withDismissAction = true,
testTag = VIDEO_CAPTURE_SUCCESS_TAG
testTag = VIDEO_CAPTURE_FAILURE_TAG
)
multipleEventsCutter.done()
}
}

is OnVideoRecordEvent.OnVideoRecordError -> {
Log.d(TAG, "cameraSystem.startRecording OnVideoRecordError")
captureEvents.trySend(VideoCaptureEvent.VideoCaptureError(it.error))
snackbarToShow = SnackbarData(
cookie = cookie,
stringResource = R.string.toast_video_capture_failure,
withDismissAction = true,
testTag = VIDEO_CAPTURE_FAILURE_TAG
)
snackbarToShow?.let { data ->
snackBarController?.addSnackBarData(data)
}
}

snackbarToShow?.let { data ->
snackBarController?.addSnackBarData(data)
}
Log.d(TAG, "cameraSystem.startRecording success")
} catch (exception: Exception) {
Log.e(TAG, "cameraSystem.startVideoRecording error", exception)
multipleEventsCutter.done()
captureEvents.trySend(VideoCaptureEvent.VideoCaptureError(exception))
}
Log.d(TAG, "cameraSystem.startRecording success")
} catch (exception: IllegalStateException) {
Log.d(TAG, "cameraSystem.startVideoRecording error", exception)
}
}
}
Expand Down
41 changes: 41 additions & 0 deletions ...er/impl/src/main/java/com/google/jetpackcamera/ui/controller/impl/MultipleEventsCutter.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* Copyright (C) 2026 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.google.jetpackcamera.ui.controller.impl

import kotlinx.atomicfu.atomic

/**
* A helper class that prevents multiple clicks.
*/
internal class MultipleEventsCutter {
private val isProcessing = atomic(false)

/**
* Executes the [event] if no other event is currently being processed.
*/
fun processEvent(event: () -> Unit) {
Comment thread
davidjiagoogle marked this conversation as resolved.
if (isProcessing.compareAndSet(expect = false, update = true)) {
event()
}
Comment on lines +30 to +32
Copy link
Copy Markdown
Collaborator

@temcguir temcguir Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Silently dropping events here is the root cause of the test flakiness. The testing framework assumes the click was successful, but the underlying system ignores it. We should remove this class entirely and rely on the UI being disabled while a capture is in progress.

}

/**
* Signals that the current event processing is complete.
*/
fun done() {
Comment thread
davidjiagoogle marked this conversation as resolved.
isProcessing.value = false
}
}
Loading