ViewModel compose extension failed to work with Fragment 1.3.3 [186097368]

Fixed

Bug

Status Update

No update yet.

Description

b9...@gmail.com

created issue #1

Apr 22, 2021 06:02PM

FragmentViewLifecycleOwner implement ViewModelStoreOwner in Fragment 1.3.3

    public ViewModelProvider(@NonNull ViewModelStoreOwner owner) {
        this(owner.getViewModelStore(), owner instanceof HasDefaultViewModelProviderFactory
                ? ((HasDefaultViewModelProviderFactory) owner).getDefaultViewModelProviderFactory()
                : NewInstanceFactory.getInstance());
    }

ViewModel compose extension use this constructor by default.
Fragment set ViewTreeViewModelStoreOwner to FragmentViewLifecycleOwner in 1.3.3, which means 1. will inject FragmentViewLifecycleOwner.
FragmentViewLifecycleOwner didn't implement HasDefaultViewModelProviderFactory, so that NewInstanceFactory will be used to create ViewModel. Which means DI framework like Hilt will failed, the ViewModel constructor can't have any parameter.

Comments

b9...@gmail.com <b9...@gmail.com> #2Apr 22, 2021 07:18PM

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

commit b9b1326edfe38588a9882e1ee624798966c5686a
Author: Jeremy Woods <jbwoods@google.com>
Date: Wed Oct 06 16:49:53 2021

Clear focus on Fragments animating out

Since moving to the new state manager, fragment now explicitly handle
focus where in the old world they could ignore it altogether. This
change has the side effect that other things affected by focus (i.e.
soft keyboard input) are affect as well. In particular when we
requestFocus we update the input request, but since we never clear it
users have to manually remove the input.

When we are animating a fragment view out, since we will no longer be
using the exiting view, let clear the focus of before we remove it from
the fragment.

RelNote: "The keyboard will now close automatically when going from
a fragment with an open keyboard to a fragment with a recycler view."
Test: tested in sample app from bug
Bug: 196852211

Change-Id: I8b842dd9a421cfbc9189014b802f5e4b6b9c2a47

M fragment/fragment/src/main/java/androidx/fragment/app/SpecialEffectsController.java

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

il...@google.com <il...@google.com> #3Apr 22, 2021 07:24PM

Assigned to jb...@google.com.

This has been fixed internally and will be available in the Fragment 1.4.0-beta01 release.

b9...@gmail.com <b9...@gmail.com> #4Apr 22, 2021 08:11PM

The issue still persist in API version 27 and below in Fragment 1.4.0

il...@google.com <il...@google.com> #5Apr 22, 2021 08:28PM

True, you'd need to explicitly pass the defaultViewModelProviderFactory to viewModel() to force using the Fragment's ViewModelProviderFactory.

jb...@google.com <jb...@google.com> May 4, 2021 04:54PM

Reassigned to sa...@google.com.

il...@google.com <il...@google.com> #6May 5, 2021 04:17PM

So there's three important use cases that we need to support when using the ViewTreeViewModelStoreOwner API and the ViewModelStoreOwner provided by that object:

Creating and using a custom Factory tied to the Fragment's View SavedStateRegistry (this is very important for AbstractSavedStateViewModelFactory and SavedStateViewModelFactory - this is what was fixed in Fragment 1.3.3 in b/181577191 and why the ViewTreeViewModelStoreOwner API now must return the private FragmentViewLifecycleOwner class.
Using the default factory. Right now, FragmentViewLifecycleOwner does not implement HasDefaultViewModelProviderFactory and, thus, a NewInstanceFactory is always used. This is ~never correct and FragmentViewLifecycleOwner should implement that interface and mimic the logic of Fragment's getDefaultViewModelProviderFactory() to lazily create a SavedStateVieModelFactory.
Developers who override getDefaultViewModelProviderFactory() at the Fragment level (either directly or via Hilt). Due to limitations in the ViewModel APIs (which we hope to address longer term), any AbstractSavedStateViewModelFactory factory here won't be using the right SavedStateRegistry as it should if used via the ViewTreeViewModelStoreOwner APIs (it'll be using the SavedStateRegistry of the Fragment itself). However, this use case is quite important to have working out of the box and as it was before, despite not being exactly right.

These three cases must all work. Therefore, I think the best solution is to:

Have FragmentViewLifecycleOwner implement HasDefaultViewModelProviderFactory
Have that implementation call the Fragment's getDefaultViewModelProviderFactory() method. If that method returns a custom factory (i.e., not the one lazily created by Fragments when that method isn't overridden), use that factory. This will fix Hilt's use case.
If the factory returned by the Fragment's getDefaultViewModelProviderFactory() is the lazily created Fragment factory, instead create a new lazily created SavedStateViewModelFactory that is tied to the SavedStateRegistry of the Fragment's View using the same logic as Fragment uses. This will fix the default factory case.

ap...@google.com <ap...@google.com> #7May 10, 2021 11:08PM

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

commit 9732b80a90541d929a263f0e69e569818fbff839
Author: Sanura N'Jaka <sanura@google.com>
Date: Thu May 06 02:46:38 2021

Have FragmentViewLifecycleOwner implement HasDefaultViewModelProviderFactory

Making FragmentViewLifecycleOwner also implement
HasDefaultViewModelProviderFactory so that we either use the existing
custom factory or create a new SavedStateViewModelFactory that is tied
to the SavedStateRegistry of the Fragment's View and not the Fragment itself.

RelNote: "`FragmentViewLifecycleOwner` now implements
`HasDefaultViewModelProviderFactory` and overrides
`getDefaultViewModelProviderFactory()` so that we can
either use the existing custom factory or create a new
`SavedStateViewModelFactory` tied to the `SavedStateRegistry`
of the fragment's view."

Bug: 186097368
Test: ./gradlew checkApi
Change-Id: I5cbfacde2479fb270c0c344c478260f20b3207d5

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

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

il...@google.com <il...@google.com> #8May 11, 2021 12:03AM

Marked as fixed.

We've fixed this issue for the upcoming Fragment 1.3.4 bug fix release and Fragment 1.4.0-alpha01.

When using Hilt or any fragment that overrides getDefaultViewModelProviderFactory(), that factory will also be used from Compose's viewModel() method.