fix(theme): decode the custom palettes through generated serializers so their role keys survive minification - #1049
Merged
Merged
Conversation
…so their role keys survive minification
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
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.
Summary
Custom themes have never applied in a release build, and since
1f88972dthey crash the app at launch. Both are the same defect.LocalDataManagerread the saved palettes withgson.fromJson(raw, CustomThemePalettes::class.java). Gson recoversMap<CustomColorRole, Long>from the field's generic signature, andio.github.aedev.flow.ui.themeis named by no keep rule inapp/proguard-rules.pro— so R8 strips that signature, Gson reads the map raw, and hands backStringkeys. Confirmed in the R8 output of:app:minifyGithubReleaseWithR8, not inferred:No
dalvik.annotation.Signatureanywhere in the three dex files mentionsCustomColorRole.Two consequences, one cause:
CustomThemeColors.colorOf(role)looked an enum up in a String-keyed map, missed, and fell back to a default — the palette silently never applied ([Bug]: Custom Theme not saving at all #964).1f88972daddedWidgetThemeSignature.persistedForm(), which sorts those entries byit.key.name. It is the first code to actually touch a key, so it throwsClassCastException: java.lang.String cannot be cast to CustomColorRoleonDispatchers.Main.immediatewhileFlowAppcollects the DataStore flow — an unrecoverable crash at launch for anyone who has ever opened the custom theme editor. Reported in the discussion on a Samsung SM-A165F, and bisected there to this build (#1288 fine, #1289 broken).The fix
Palette persistence moves to a new
data/local/CustomThemePalettePersistence.ktthat encodes and decodes through kotlinx.serialization, callingStoredPalettes.serializer()explicitly. There is no reflection and no generic signature to strip, so this cannot regress if a keep rule moves again — which is the same failure mode as #996, now hit for the second time. Roles are stored by name, so a role the app no longer knows drops out of the palette instead of failing the whole decode.The wire format is unchanged — Gson already wrote role names as JSON object keys — so existing saves and settings backups read straight back. No user has to re-create their theme.
Fixing the read also un-breaks the write path, so the second commit of the reformat aside, the diff is five lines of behaviour.
style(data): apply the project ktlint rules to LocalDataManageris kept as its own commit: touching the file made it ratchet-eligible, and separating it keeps the actual fix reviewable.Related issue
Closes #964
Change type
Validation
./gradlew :app:assembleGithubDebug./gradlew :app:testGithubDebugUnitTest— full suite greenAlso run:
:app:compileFossDebugKotlin— passes.:app:minifyGithubReleaseWithR8plusdexdump -aover the three dex files — this is the evidence for the root cause quoted above.spotlessKotlinApplyover the four touched files; the build script was restored afterwards.New tests, 8 in total:
CustomThemePalettePersistenceTest— round trip, decoding a palette written by the previous Gson reader, an unknown role dropping out, the legacy comma-separated migration, and unreadable JSON falling back to defaults.WidgetThemeSignatureTest— the direct regression test:persistedForm()over a decoded palette signs without a cast failure, is stable across two calls, and changes when a colour changes.Test device and Android version: none — not yet run on hardware. The crash and the theme both want a device pass before release.
Risk and compatibility
WidgetThemeSignature.persistedForm()is left unguarded on the main dispatcher. Wrapping it would hide the next bug rather than fix one, so the cause is fixed instead — worth a deliberate decision if we would rather it never be able to take the app down.