FragmentState writes 2 identical objects into Bundle using ViewModel with SavedStateHandle [253546214]

Fixed

Bug

Status Update

No update yet.

Description

yo...@luchegroup.com

created issue #1

Oct 14, 2022 06:45AM

Artifact used: androidx.fragment:fragment-ktx:1.5.3, androidx.lifecycle:lifecycle-viewmodel-ktx: 2.5.1, androidx.lifecycle:lifecycle-viewmodel-savedstate:2.5.1
Devices/Android versions reproduced on: Nothing A063 running Android 12

PoC project:

https://github.com/KeithYokoma/saved_state_poc

When saving the ViewModel state object using SavedStateHandle, the object is stored in the Bundle of the FragmentState.
The issue is that the Bundle contains 2 identical ViewModel state object with different keys(`androidx.lifecycle.internal.SavedStateHandlesProvider` and `androidx.lifecycle.BundlableSavedStateRegistry.key`), so it takes up twice as much as the ViewState object size. If the ViewState object size gets bigger, it can easily cause `TransactionTooLargeException` whereas the single ViewState size is small enough to fit in the Bundle limit.

as_screenshot_debugger_showing_identical_objects.png

1.4 MB

View

Download

Comments

il...@google.com <il...@google.com> #2Oct 18, 2022 05:07PM

Thanks for the bug report. If you can still reproduce this, can you attach a debugger and report what value for typeId you see in EncryptedSharedPreferences.getDecryptedObject?

yo...@luchegroup.com <yo...@luchegroup.com> #3Oct 20, 2022 07:40AM

The error reporting could be better. The next version will at least make it more clear what's going wrong.

il...@google.com <il...@google.com> #4Oct 20, 2022 10:46PM

Assigned to jb...@google.com.

Project: platform/frameworks/support
Branch: androidx-main

commit 168d2c90f6a88ff922d2ea72ac3a932c900e590b
Author: Daniel Angell <danielangell@google.com>
Date: Thu Oct 13 14:50:01 2022

Improve error reporting for getDecryptedObject()

Bug: 241699427
Test: Test suite passes. Difficult to add tests for specifically.
Change-Id: If6438b611a5a9ead4ae462cfa79f266ca65bc50d

M security/security-crypto/src/main/java/androidx/security/crypto/EncryptedSharedPreferences.java

https://android-review.googlesource.com/2254976

ap...@google.com <ap...@google.com> #5Nov 10, 2022 01:19AM

Project: platform/frameworks/support
Branch: androidx-main

commit 345d09ca4e1908954e8d38979cf9a63d0f030b1d
Author: Jeremy Woods <jbwoods@google.com>
Date: Tue Nov 08 17:15:50 2022

Prevent saving ViewModels in view saved state

Since converted fragents over to creation extras, we have a problem
where ViewModels were being saved in both the fragment and fragment view
saved state registries.

The cause of this is that the FragmentViewLifecycleOwner uses the
ViewModelStore of the fragment so when it calls enableSavedStateHandles
its registry also signs up to save all ViewModels in that store.

This is resolved by no longer calling enabledSavedStateHandles in
FragmentViewLifecycleOwner and making the default CreationExtras use the
fragment components directly.

RelNote: "Fragment will no longer incorrectly save ViewModels as part of
the view registry state."
Test: added test
Bug: 253546214

Change-Id: I10d2b5363d0abe967e92ad90a578d3bf88a2ca3b

M fragment/fragment/src/androidTest/java/androidx/fragment/app/FragmentViewLifecycleOwnerTest.kt
M fragment/fragment/src/main/java/androidx/fragment/app/FragmentViewLifecycleOwner.java

https://android-review.googlesource.com/2293203

jb...@google.com <jb...@google.com> #6Nov 10, 2022 01:28AM

Marked as fixed.

This has been fixed internally and will be available in the Fragment 1.6.0-alpha04 release.

ju...@google.com <ju...@google.com> #7Dec 7, 2022 06:35PM

The following release(s) address this bug.It is possible this bug has only been partially addressed:

androidx.fragment:fragment:1.6.0-alpha04

yo...@luchegroup.com <yo...@luchegroup.com> #8Dec 8, 2022 01:27AM

Thanks for the bug fix, I have confirmed that the issue is resolved.

eu...@gmail.com <eu...@gmail.com> #9Dec 22, 2022 09:32AM

We have a crash now with this change:

java.lang.IllegalStateException: enableSavedStateHandles() wasn't called prior to createSavedStateHandle() call
  at androidx.lifecycle.SavedStateHandleSupport.getSavedStateHandlesProvider(SavedStateHandleSupport.kt:115)
  at androidx.lifecycle.SavedStateHandleSupport.createSavedStateHandle(SavedStateHandleSupport.kt:65)
  at androidx.lifecycle.SavedStateHandleSupport.createSavedStateHandle(SavedStateHandleSupport.kt:103)
  at androidx.lifecycle.AbstractSavedStateViewModelFactory.create(AbstractSavedStateViewModelFactory.java:89)
  at dagger.hilt.android.internal.lifecycle.HiltViewModelFactory.create(HiltViewModelFactory.java:116)
  at androidx.lifecycle.ViewModelProvider.get(ViewModelProvider.kt:187)
  at androidx.lifecycle.ViewModelProvider.get(ViewModelProvider.kt:153)
  at androidx.lifecycle.viewmodel.compose.ViewModelKt.get(ViewModel.kt:215)
  at androidx.lifecycle.viewmodel.compose.ViewModelKt.viewModel(ViewModel.kt:156)
  at our.code.  (points to the place where the ViewModel is injected into a constructor of a Composable view

What we do in the composable code we call viewModel like:

    val extras = MutableCreationExtras()
    extras[SAVED_STATE_REGISTRY_OWNER_KEY] = LocalSavedStateRegistryOwner.current
    extras[VIEW_MODEL_STORE_OWNER_KEY] = checkNotNull(LocalViewModelStoreOwner.current) {
        "No ViewModelStoreOwner was provided via LocalViewModelStoreOwner"
    }
    extras[DEFAULT_ARGS_KEY] = bundleOf("PARAM_KEY" to "PARAM_VALUE")
    return viewModel(extras = extras)

I see that enableSavedStateHandles was deleted from FragmentViewLifecycleOwner.

Can you give some recommendations about situation?

jb...@google.com <jb...@google.com> #10Dec 22, 2022 11:32PM

Please file a separate bug with a minimal sample project that reproduces this issue.