Fix attachments lost on activity recreation#6377
Merged
aleksandar-apostolov merged 2 commits intov6from Apr 22, 2026
Merged
Conversation
…teHandle Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
Contributor
SDK Size Comparison 📏
|
WalkthroughThis PR introduces message composer attachment state persistence across process death by implementing a Changes
Sequence Diagram(s)sequenceDiagram
participant Factory as ViewModel Factory
participant Controller as MessageComposer<br/>Controller
participant Saver as ComposerStateSaver
participant Handle as SavedStateHandle
participant UI as UI Layer
Note over Factory,UI: Scenario 1: With CreationExtras
Factory->>Handle: Extract SavedStateHandle from extras
Factory->>Saver: Create SavedStateComposerStateSaver(handle)
Factory->>Controller: Init with stateSaver
Controller->>Saver: restoreAttachments()
Saver->>Handle: Read saved parcelable attachments
Handle-->>Saver: Return restored list or null
Saver-->>Controller: List<Attachment>?
Controller->>Controller: Set selectedAttachments if restored
UI->>Controller: User selects attachment
Controller->>Saver: saveAttachments(updated list)
Saver->>Handle: Write parcelable to SavedStateHandle
Note over Factory,UI: Scenario 2: Without CreationExtras
Factory->>Saver: Create NoOpComposerStateSaver
Factory->>Controller: Init with stateSaver
Controller->>Saver: restoreAttachments()
Saver-->>Controller: Always returns null
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/MessageComposerController.kt (1)
602-619:⚠️ Potential issue | 🟡 MinorEdit-mode attachments may be silently dropped after process death.
performMessageAction(Edit)assignsmessageAction.message.attachments(remote attachments with no localuploadfile) toselectedAttachments. The save observer at Line 518-520 will then persist them viaParcelableAttachment, but on restoreSavedStateComposerStateSaverdrops any attachment whoseuploadfile no longer exists — which is always true for remote edit-mode attachments. The user would lose them, and the edit flow itself is not covered by the draft-text mechanism either.Consider either (a) skipping persistence when in edit mode, or (b) preserving non-upload attachments across restore (e.g., keep them without the file-existence gate). Not a blocker but worth a conscious decision.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/MessageComposerController.kt` around lines 602 - 619, performMessageAction(Edit) places remote-only attachments into selectedAttachments which are later saved, but SavedStateComposerStateSaver drops attachments that lack a local upload.file on restore; to fix, either (A) skip persisting attachments when the composer is in edit flow by detecting the Edit action (performed in performMessageAction) or the edit input source (MessageInput.Source.Edit) in the save observer and not serializing selectedAttachments, or (B) change SavedStateComposerStateSaver to preserve ParcelableAttachment instances that represent remote attachments (i.e., do not require upload.file existence) when restoring an edit session; update the save/restore logic accordingly so performMessageAction(Edit), selectedAttachments and the saver use the same edit-mode check and do not silently drop remote attachments.
🧹 Nitpick comments (8)
stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/internal/NoOpComposerStateSaver.kt (1)
22-28: Unresolved KDoc references.The KDoc links
[CreationExtras]and[ViewModelProvider.Factory.create]will not resolve since neither type is imported in this file (and they are Android framework types that this module-agnostic interface intentionally avoids). Consider downgrading them to plain backticks to avoid Dokka warnings.✏️ Proposed tweak
- * Used as a fallback when the ViewModel is created without [CreationExtras] - * (e.g. via the legacy [ViewModelProvider.Factory.create] overload). + * Used as a fallback when the ViewModel is created without `CreationExtras` + * (e.g. via the legacy `ViewModelProvider.Factory.create` overload).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/internal/NoOpComposerStateSaver.kt` around lines 22 - 28, The KDoc in NoOpComposerStateSaver.kt contains unresolved link-style references to Android types; replace the bracketed links [CreationExtras] and [ViewModelProvider.Factory.create] with plain code-style references (`CreationExtras` and `ViewModelProvider.Factory.create`) in the doc for ComposerStateSaver to avoid importing Android framework types and suppress Dokka warnings.stream-chat-android-ui-common/src/test/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/MessageComposerControllerTests.kt (2)
310-348: Consider factoring out theAppSettingsfixture.The
AppSettingsbuilder here is substantial and may be reused in future tests. Extracting a small helper (e.g.,givenDefaultAppSettings()or a module-levelfakeAppSettings()) would keep individual tests focused on the behavior under test. Not blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-chat-android-ui-common/src/test/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/MessageComposerControllerTests.kt` around lines 310 - 348, Factor out the large AppSettings construction into a reusable test helper to reduce duplication: create a helper function (e.g., fakeAppSettings() or givenDefaultAppSettings()) that returns the AppSettings instance used in tests, then replace the inline AppSettings block in the test with givenAppSettings(fakeAppSettings()) or .givenDefaultAppSettings() in the Fixture chain; update other tests accordingly to use the new helper and keep the test focused on restoring attachments.
366-381: Strengthen theclearDataassertion.Currently only
store.clearedis asserted. To defend against regressions where someone reordersclearData()so an empty-list save happens afterclear()(effectively re-persisting empty state), also assert thatstore.restoreAttachments()returns null/empty after the call. Cheap guard, higher signal.💡 Suggested addition
controller.setMessageInput("some text") controller.clearData() assertTrue(store.cleared) + assertTrue(store.restoreAttachments().isNullOrEmpty()) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-chat-android-ui-common/src/test/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/MessageComposerControllerTests.kt` around lines 366 - 381, The test `clearData clears the state store` currently only asserts `store.cleared`; after calling `controller.clearData()` also assert that saved attachments were removed by checking `store.restoreAttachments()` (or equivalent restore method on `FakeComposerStateSaver`) returns null or an empty list; update the test to call `store.restoreAttachments()` and assert it is null/empty to prevent regressions where an empty save could re-persist state after `clear()`.stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/internal/ComposerStateSaver.kt (1)
22-47: Nullable vs empty-list semantics inrestoreAttachments.The return type is
List<Attachment>?but the only caller treats null and empty identically (isNullOrEmpty()). If null vs empty is not meant to carry a different meaning, consider tightening the contract toList<Attachment>(returningemptyList()when nothing is persisted) and document the "no-op" case, or alternatively document in KDoc what each return value means. Low priority.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/internal/ComposerStateSaver.kt` around lines 22 - 47, The restoreAttachments return type is nullable but callers treat null the same as empty; update the ComposerStateSaver contract by changing restoreAttachments(): List<Attachment>? to restoreAttachments(): List<Attachment> and ensure implementations return emptyList() when nothing is persisted, or alternatively keep the nullable signature but add KDoc on ComposerStateSaver.restoreAttachments explaining the semantic difference between null and empty (and prefer the non-null empty-list approach); update implementations of saveAttachments/restoreAttachments to comply and adjust any callers if necessary (search for ComposerStateSaver.restoreAttachments usages to confirm).stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/viewmodel/messages/MessageListViewModelFactory.kt (1)
143-145: Fallback path silently disables attachment persistence.When a caller instantiates
MessageComposerViewModelvia the zero-extrascreate(modelClass)path, this factory returns a controller wired withNoOpComposerStateSaver, so attachments are not persisted across process death. In practice Android'sViewModelProviderwill prefercreate(modelClass, extras)when the owner supplies extras (Activity/Fragment withSavedStateRegistryOwner), so the common case is covered — but any customViewModelStoreOwnerthat doesn't expose saved state, or any directfactory.create(MessageComposerViewModel::class.java)call, silently loses persistence with no warning.Consider a brief KDoc on the class (and/or a debug log in the NoOp branch) noting that persistence requires the extras-aware
createoverload, so integrators know they need aSavedStateRegistryOwner-backed owner.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/viewmodel/messages/MessageListViewModelFactory.kt` around lines 143 - 145, The factory's fallback path returns a controller using NoOpComposerStateSaver which silently disables attachment persistence when callers use the zero-extras create(modelClass) overload; update MessageListViewModelFactory to clearly signal this by adding a short KDoc on the class (or the create(modelClass) overload) stating that attachment persistence requires using the extras-aware create(modelClass, extras) with a SavedStateRegistryOwner, and add a debug/log message in the branch that constructs the controller with NoOpComposerStateSaver (reference NoOpComposerStateSaver, createMessageComposerViewModel, and MessageComposerViewModel) to warn integrators when persistence is disabled.stream-chat-android-ui-common/src/test/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/internal/ParcelableAttachmentTest.kt (3)
30-48: "Round-trip" tests don't actually marshal through aParcel.These tests only exercise the
toParcelable()→toAttachment()conversion functions; they never write/read theParcelableAttachmentto an actualandroid.os.Parcel. That means any issue introduced by@Parcelizecode generation (e.g., anextraDatavalue that passesisParcelSafe()but failsParcel.writeValue, or type-coercion quirks likeInt↔LongafterreadValue) will not be caught here — defeating the main reason this class is@Parcelize.Consider adding a real Parcel round-trip covering at least
round-trip preserves all fieldsandround-trip preserves extraData with primitives, e.g. under Robolectric:val parcel = Parcel.obtain() try { original.toParcelable().writeToParcel(parcel, 0) parcel.setDataPosition(0) val restored = ParcelableAttachment.CREATOR.createFromParcel(parcel).toAttachment() // assertions... } finally { parcel.recycle() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-chat-android-ui-common/src/test/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/internal/ParcelableAttachmentTest.kt` around lines 30 - 48, The tests currently only call toParcelable() → toAttachment() and must be changed to perform a real Android Parcel round-trip to validate `@Parcelize` behavior: for the test methods (e.g., `round-trip preserves all fields` and `round-trip preserves extraData with primitives`) obtain a Parcel, call original.toParcelable().writeToParcel(parcel, 0), reset with parcel.setDataPosition(0), recreate via ParcelableAttachment.CREATOR.createFromParcel(parcel).toAttachment(), assert fields, and finally parcel.recycle(); update assertions to use the recreated attachment instead of direct toParcelable().toAttachment() so Parcel write/read edge-cases are covered.
177-192: Consider adding coverage for nested unsafe values.The current "unsafe" cases put a non-parcelable object directly as a top-level
extraDatavalue. Given thatisParcelSafe()is described as recursively validating nested lists/maps, a test with an unsafe value nested inside aListorMap(e.g.,"list" to listOf("ok", object {})or"nested" to mapOf("k" to object {})) would guard against regressions in the recursive check.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-chat-android-ui-common/src/test/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/internal/ParcelableAttachmentTest.kt` around lines 177 - 192, Add tests to cover nested unsafe values by creating attachments whose extraData contains a non-parcelable object nested inside a List or Map (e.g., "list" -> listOf("ok", object {}) and "nested" -> mapOf("k" to object {})) and assert that attachments.areExtraDataParcelSafe() returns false; place these new tests alongside the existing tests in ParcelableAttachmentTest (use the same test naming style, e.g., `attachments_with_nested_non_parcelable_extraData_are_NOT_parcel_safe` and `nested_unsafe_value_makes_entire_list_unsafe`) to validate the recursive isParcelSafe behavior.
102-126: Use named arguments for clarity:InProgress(bytesUploaded = 100, totalBytes = 1000)instead of positional arguments.The current parameter order is correct (100 bytes uploaded out of 1000 total), but named arguments improve readability and make intent explicit, especially for numeric values that are difficult to interpret positionally.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-chat-android-ui-common/src/test/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/internal/ParcelableAttachmentTest.kt` around lines 102 - 126, In the test function `fields not in ParcelableAttachment are null or default on restore` in ParcelableAttachmentTest, replace the positional constructor call `Attachment.UploadState.InProgress(100, 1000)` with a named-argument form `Attachment.UploadState.InProgress(bytesUploaded = 100, totalBytes = 1000)` to make the intent explicit; update the `original` Attachment creation to use the named parameters for `uploadState` while keeping all other fields unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/internal/ParcelableAttachment.kt`:
- Around line 77-82: The isParcelSafe() implementation is too restrictive;
replace its whitelist logic by probing actual Parcel.writeValue behavior: in
isParcelSafe() obtain a Parcel via Parcel.obtain(), attempt
parcel.writeValue(this) (and optional parcel.setDataPosition(0) /
parcel.readValue(null) to ensure round-trip), catch any Throwable and return
false on error, finally recycle the parcel; update references to isParcelSafe()
(used by SavedStateComposerStateSaver.saveAttachments()) so types that
Parcel.writeValue accepts (Parcelable, Serializable, CharSequence, etc.) are
preserved instead of being cleared.
---
Outside diff comments:
In
`@stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/MessageComposerController.kt`:
- Around line 602-619: performMessageAction(Edit) places remote-only attachments
into selectedAttachments which are later saved, but SavedStateComposerStateSaver
drops attachments that lack a local upload.file on restore; to fix, either (A)
skip persisting attachments when the composer is in edit flow by detecting the
Edit action (performed in performMessageAction) or the edit input source
(MessageInput.Source.Edit) in the save observer and not serializing
selectedAttachments, or (B) change SavedStateComposerStateSaver to preserve
ParcelableAttachment instances that represent remote attachments (i.e., do not
require upload.file existence) when restoring an edit session; update the
save/restore logic accordingly so performMessageAction(Edit),
selectedAttachments and the saver use the same edit-mode check and do not
silently drop remote attachments.
---
Nitpick comments:
In
`@stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/internal/ComposerStateSaver.kt`:
- Around line 22-47: The restoreAttachments return type is nullable but callers
treat null the same as empty; update the ComposerStateSaver contract by changing
restoreAttachments(): List<Attachment>? to restoreAttachments():
List<Attachment> and ensure implementations return emptyList() when nothing is
persisted, or alternatively keep the nullable signature but add KDoc on
ComposerStateSaver.restoreAttachments explaining the semantic difference between
null and empty (and prefer the non-null empty-list approach); update
implementations of saveAttachments/restoreAttachments to comply and adjust any
callers if necessary (search for ComposerStateSaver.restoreAttachments usages to
confirm).
In
`@stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/internal/NoOpComposerStateSaver.kt`:
- Around line 22-28: The KDoc in NoOpComposerStateSaver.kt contains unresolved
link-style references to Android types; replace the bracketed links
[CreationExtras] and [ViewModelProvider.Factory.create] with plain code-style
references (`CreationExtras` and `ViewModelProvider.Factory.create`) in the doc
for ComposerStateSaver to avoid importing Android framework types and suppress
Dokka warnings.
In
`@stream-chat-android-ui-common/src/test/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/internal/ParcelableAttachmentTest.kt`:
- Around line 30-48: The tests currently only call toParcelable() →
toAttachment() and must be changed to perform a real Android Parcel round-trip
to validate `@Parcelize` behavior: for the test methods (e.g., `round-trip
preserves all fields` and `round-trip preserves extraData with primitives`)
obtain a Parcel, call original.toParcelable().writeToParcel(parcel, 0), reset
with parcel.setDataPosition(0), recreate via
ParcelableAttachment.CREATOR.createFromParcel(parcel).toAttachment(), assert
fields, and finally parcel.recycle(); update assertions to use the recreated
attachment instead of direct toParcelable().toAttachment() so Parcel write/read
edge-cases are covered.
- Around line 177-192: Add tests to cover nested unsafe values by creating
attachments whose extraData contains a non-parcelable object nested inside a
List or Map (e.g., "list" -> listOf("ok", object {}) and "nested" -> mapOf("k"
to object {})) and assert that attachments.areExtraDataParcelSafe() returns
false; place these new tests alongside the existing tests in
ParcelableAttachmentTest (use the same test naming style, e.g.,
`attachments_with_nested_non_parcelable_extraData_are_NOT_parcel_safe` and
`nested_unsafe_value_makes_entire_list_unsafe`) to validate the recursive
isParcelSafe behavior.
- Around line 102-126: In the test function `fields not in ParcelableAttachment
are null or default on restore` in ParcelableAttachmentTest, replace the
positional constructor call `Attachment.UploadState.InProgress(100, 1000)` with
a named-argument form `Attachment.UploadState.InProgress(bytesUploaded = 100,
totalBytes = 1000)` to make the intent explicit; update the `original`
Attachment creation to use the named parameters for `uploadState` while keeping
all other fields unchanged.
In
`@stream-chat-android-ui-common/src/test/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/MessageComposerControllerTests.kt`:
- Around line 310-348: Factor out the large AppSettings construction into a
reusable test helper to reduce duplication: create a helper function (e.g.,
fakeAppSettings() or givenDefaultAppSettings()) that returns the AppSettings
instance used in tests, then replace the inline AppSettings block in the test
with givenAppSettings(fakeAppSettings()) or .givenDefaultAppSettings() in the
Fixture chain; update other tests accordingly to use the new helper and keep the
test focused on restoring attachments.
- Around line 366-381: The test `clearData clears the state store` currently
only asserts `store.cleared`; after calling `controller.clearData()` also assert
that saved attachments were removed by checking `store.restoreAttachments()` (or
equivalent restore method on `FakeComposerStateSaver`) returns null or an empty
list; update the test to call `store.restoreAttachments()` and assert it is
null/empty to prevent regressions where an empty save could re-persist state
after `clear()`.
In
`@stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/viewmodel/messages/MessageListViewModelFactory.kt`:
- Around line 143-145: The factory's fallback path returns a controller using
NoOpComposerStateSaver which silently disables attachment persistence when
callers use the zero-extras create(modelClass) overload; update
MessageListViewModelFactory to clearly signal this by adding a short KDoc on the
class (or the create(modelClass) overload) stating that attachment persistence
requires using the extras-aware create(modelClass, extras) with a
SavedStateRegistryOwner, and add a debug/log message in the branch that
constructs the controller with NoOpComposerStateSaver (reference
NoOpComposerStateSaver, createMessageComposerViewModel, and
MessageComposerViewModel) to warn integrators when persistence is disabled.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ba3a3c80-77be-424f-9e5b-201467584579
📒 Files selected for processing (15)
stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/viewmodel/messages/MessagesViewModelFactory.ktstream-chat-android-compose/src/test/kotlin/io/getstream/chat/android/compose/viewmodel/messages/MessageComposerViewModelTest.ktstream-chat-android-ui-common/api/stream-chat-android-ui-common.apistream-chat-android-ui-common/build.gradle.ktsstream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/MessageComposerController.ktstream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/internal/ComposerStateSaver.ktstream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/internal/NoOpComposerStateSaver.ktstream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/internal/ParcelableAttachment.ktstream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/internal/SavedStateComposerStateSaver.ktstream-chat-android-ui-common/src/test/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/MessageComposerControllerTests.ktstream-chat-android-ui-common/src/test/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/internal/ParcelableAttachmentTest.ktstream-chat-android-ui-common/src/test/kotlin/io/getstream/chat/android/ui/common/feature/messages/composer/internal/SavedStateComposerStateSaverTest.ktstream-chat-android-ui-components/api/stream-chat-android-ui-components.apistream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/viewmodel/messages/MessageListViewModelFactory.ktstream-chat-android-ui-components/src/test/kotlin/io/getstream/chat/android/ui/viewmodels/messages/MessageComposerViewModelTest.kt
aleksandar-apostolov
approved these changes
Apr 22, 2026
Contributor
aleksandar-apostolov
left a comment
aleksandar-apostolov
left a comment
There was a problem hiding this comment.
LGTM, posted 1 question.
|
aleksandar-apostolov
deleted the
bug/AND-1145_fix_attachments_lost_on_activity_recreation
branch
Contributor
|
🚀 Available in v6.37.3 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Goal
Fix selected composer attachments being lost when the activity is destroyed and recreated (e.g., "Don't keep activities" developer option, process death). Previously, all in-memory composer state was lost on activity recreation since
MessageComposerControllerstored everything in plainMutableStateFlowfields.Implementation
Introduced a
ComposerStateSaverabstraction that persists selected attachments viaSavedStateHandle:ComposerStateSaver— framework-agnostic interface the controller uses for save/restore (no Android imports in the controller)SavedStateComposerStateSaver—SavedStateHandle-backed implementation injected by the ViewModel factoryNoOpComposerStateSaver— fallback for the legacycreate(modelClass)factory path (no persistence)ParcelableAttachment—@Parcelizewrapper containing only fields populated at compose time (upload,type,name,fileSize,mimeType,title,extraData)Key design decisions:
extraDatavalues are validated viaisParcelSafe(). If any value is non-parcelable, saving is skipped entirely (falls back to current behavior — no crash, no silent data loss)uploadfile no longer exists on disk are silently droppedalsoSendToChannelis not persisted (only relevant in thread mode which isn't persisted)UI Changes
No UI changes.
Testing
ParcelableAttachmentTest,SavedStateComposerStateSaverTest,MessageComposerControllerTestsSummary by CodeRabbit