signalapp/Signal-Android
signalapp/Signal-Android
1
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases 16 Wiki Activity

Fix transfer control progress reporting bugs.

Browse Source
This commit is contained in:
Cody Henthorne 2026-06-15 12:28:34 -04:00 committed by Greyson Parrelli
parent 3f682be609
commit d22a2c0a50
3 changed files with 29 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
app/src/main/java/org/thoughtcrime/securesms/components/transfercontrols/TransferControlView.kt
View File

@ -120,25 +120,13 @@ class TransferControlView @JvmOverloads constructor(context: Context, attrs: Att
}
if (event.type == PartProgressEvent.Type.COMPRESSION) {
val mutableMap = it.compressionProgress.toMutableMap()
val updateEvent = Progress.fromEvent(event)
val existingEvent = mutableMap[attachment]
if (existingEvent == null || updateEvent.completed > existingEvent.completed) {
mutableMap[attachment] = updateEvent
} else if (updateEvent.completed < 0.bytes) {
mutableMap.remove(attachment)
}
return@updateState it.copy(compressionProgress = mutableMap.toMap())
val progress = it.compressionProgress.toMutableMap()
progress.applyProgress(attachment, Progress.fromEvent(event))
return@updateState it.copy(compressionProgress = progress.toMap())
} else {
val mutableMap = it.networkProgress.toMutableMap()
val updateEvent = Progress.fromEvent(event)
val existingEvent = mutableMap[attachment]
if (existingEvent == null || updateEvent.completed > existingEvent.completed) {
mutableMap[attachment] = updateEvent
} else if (updateEvent.completed < 0.bytes) {
mutableMap.remove(attachment)
}
return@updateState it.copy(networkProgress = mutableMap.toMap())
val progress = it.networkProgress.toMutableMap()
progress.applyProgress(attachment, Progress.fromEvent(event))
return@updateState it.copy(networkProgress = progress.toMap())
}
}
}
@ -227,6 +215,14 @@ class TransferControlView @JvmOverloads constructor(context: Context, attrs: Att
updateState { it.copy(isClickable = clickable) }
}
private fun MutableMap<Attachment, Progress>.applyProgress(attachment: Attachment, update: Progress) {
if (update.completed < 0.bytes) {
remove(attachment)
} else {
put(attachment, update)
}
}
private inline fun verboseLog(message: () -> String) {
if (VERBOSE_DEVELOPMENT_LOGGING) {
Log.d(TAG, "[$viewId] ${message()}")

4
app/src/main/java/org/thoughtcrime/securesms/components/transfercontrols/TransferControls.kt
View File

@ -11,6 +11,8 @@ import org.thoughtcrime.securesms.attachments.Attachment
import org.thoughtcrime.securesms.database.AttachmentTable
import org.thoughtcrime.securesms.mms.Slide
import org.thoughtcrime.securesms.util.MediaUtil
import org.whispersystems.signalservice.api.crypto.AttachmentCipherStreamUtil
import org.whispersystems.signalservice.internal.crypto.PaddingInputStream
/**
* Pure, Android-View-free logic for the transfer controls UI.
@ -152,7 +154,7 @@ object TransferControls {
} else if (state.isUpload) {
ProgressLabel.Bytes(state.networkProgress.sumCompleted(), state.networkProgress.sumTotal())
} else {
val total = state.slides.sumOf { it.fileSize }.bytes
val total = state.slides.sumOf { AttachmentCipherStreamUtil.getCiphertextLength(PaddingInputStream.getPaddedSize(it.fileSize)) }.bytes
val completed = state.networkProgress.sumCompleted().let { if (it > total) total else it }
ProgressLabel.Bytes(completed, total)
}

20
app/src/test/java/org/thoughtcrime/securesms/components/transfercontrols/TransferControlsTest.kt
View File

@ -21,6 +21,8 @@ import org.thoughtcrime.securesms.attachments.Attachment
import org.thoughtcrime.securesms.database.AttachmentTable
import org.thoughtcrime.securesms.mms.Slide
import org.thoughtcrime.securesms.util.MediaUtil
import org.whispersystems.signalservice.api.crypto.AttachmentCipherStreamUtil
import org.whispersystems.signalservice.internal.crypto.PaddingInputStream
class TransferControlsTest {
@ -187,22 +189,24 @@ class TransferControlsTest {
}
@Test
fun `download label uses fixed slide size as denominator, not network total`() {
fun `download label denominator is ciphertext size derived from slide, not network total`() {
val slides = listOf(slide(AttachmentTable.TRANSFER_PROGRESS_STARTED, size = 1000))
// Network total (2000) is intentionally larger than the slide's fixed file size (1000) to prove the denominator
// comes from the slide size, which does not ramp up mid-transfer.
// Completed is reported in ciphertext bytes, so the denominator must be the matching ciphertext length derived from the
// slide's plaintext size. The network event's total (2000) is intentionally different to prove it is not the source.
val state = stateOf(slides, networkProgress = progressOf(slides, completed = 500, total = 2000))
val render = TransferControls.deriveRenderState(state) as TransferControlsRenderState.InProgress
assertEquals(TransferControls.ProgressLabel.Bytes(500L.bytes, 1000L.bytes), render.label)
val expectedTotal = AttachmentCipherStreamUtil.getCiphertextLength(PaddingInputStream.getPaddedSize(1000))
assertEquals(TransferControls.ProgressLabel.Bytes(500L.bytes, expectedTotal.bytes), render.label)
}
@Test
fun `download label clamps completed to total`() {
fun `download label clamps completed to ciphertext total`() {
val slides = listOf(slide(AttachmentTable.TRANSFER_PROGRESS_STARTED, size = 1000))
// Network bytes include encryption overhead, so completed can edge past the file size; it should clamp to total.
val state = stateOf(slides, networkProgress = progressOf(slides, completed = 1100, total = 1100))
val expectedTotal = AttachmentCipherStreamUtil.getCiphertextLength(PaddingInputStream.getPaddedSize(1000))
// Incremental-MAC overhead means transmitted bytes can edge just past the computed ciphertext length; clamp to total.
val state = stateOf(slides, networkProgress = progressOf(slides, completed = expectedTotal + 100, total = expectedTotal + 100))
val render = TransferControls.deriveRenderState(state) as TransferControlsRenderState.InProgress
assertEquals(TransferControls.ProgressLabel.Bytes(1000L.bytes, 1000L.bytes), render.label)
assertEquals(TransferControls.ProgressLabel.Bytes(expectedTotal.bytes, expectedTotal.bytes), render.label)
}
@Test