Change to ViewModelProvider$Factory.create breaks backward compatibility in lifecycle-viewmodel 2.8 [341792251]

Fixed

Bug

Status Update

No update yet.

Description

py...@gmail.com

created issue #1

May 21, 2024 05:10AM

Component used: lifecycle-viewmodel

Version used: 2.8

Summary: the migration of ViewModelProvider to KMP seemingly reverted a 2 year old fix to an issue that had to do with desugaring for compileOnly dependencies in a library project.

Lifecycle 2.5.0 alpha accidentally broke backward compatibility, this was quickly fixed in Lifecycle 2.5.0-rc02, see this issue for context: https://issuetracker.google.com/issues/230454566

The fix was here: https://android-review.googlesource.com/c/platform/frameworks/support/+/2108693

The recent migration of ViewModelProvider.Factory to KMP seems to have reverted the fix: https://android-review.googlesource.com/c/platform/frameworks/support/+/2966765

As a result, the original runtime crash has surfaced again in lifecycle-viewmodel 2.8, as reported here: https://github.com/square/leakcanary/issues/2677

Someone created a repro at the time, you could probably reuse that to repro after updating the dependencies: https://github.com/dougnazar/leak_viewmodel

Comments

mg...@google.com <mg...@google.com> May 21, 2024 01:12PM

Assigned to mg...@google.com.

dr...@gmail.com <dr...@gmail.com> #2May 24, 2024 05:32PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit c60f33e229e31ab328ef6b59dab63b264954831c
Author: Alexandre Elias <aelias@google.com>
Date: Fri Jul 10 16:23:09 2020

Semantics no-op cleanups

Partly in response to lmr's broad code review, I did a pass of
superficial API/implementation cleanup. The main changes are:

- I changed each Boolean SemanticsProperty where false is equivalent to
not being present to take "Unit" instead. This is conceptually
clearer: it avoids questions like "can I cancel out a semantics from a
merged child by setting it to false?" Because "property = Unit" looks
weird, I also changed the style of these to "property()".

- I moved the Semantics id generator closer to where it's used, in
SemanticsModifierCore. I made it internal and an AtomicInt.
(Note that integer ids are heavily used in the Android
AccessibilityNodeInfo APIs so I can't simply remove them entirely.)

- I deleted dead code. Some examples include SemanticsHintOverrides,
a public API not connected to anything, and SemanticsPropertyKey
merge() open method which is never called. (In both cases I have
a different plan in mind for accessibility.)

Fixes: 145951226
Fixes: 145955412
Test: existing tests
Relnote: "Single-value semantics properties now use a calling style.
For example, 'semantics { hidden = true }' is now written as:
'semantics { hidden() }'."

Change-Id: Ic1afd12ea22c926babc9662f1804d80b33aa0cfc

M ui/integration-tests/benchmark/src/androidTest/java/androidx/ui/benchmark/test/LayoutNodeModifierBenchmark.kt
M ui/ui-core/api/0.1.0-dev15.txt
M ui/ui-core/api/current.txt
M ui/ui-core/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-core/api/public_plus_experimental_current.txt
M ui/ui-core/api/restricted_0.1.0-dev15.txt
M ui/ui-core/api/restricted_current.txt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/graphics/vector/VectorTest.kt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/semantics/SemanticsTests.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidActuals.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeView.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeViewAccessibilityDelegateCompat.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidPopup.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/Expect.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsConfiguration.kt
D ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsHintOverrides.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsModifier.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsNode.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsOwner.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsWrapper.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/semantics/SemanticsProperties.kt
M ui/ui-foundation/api/0.1.0-dev15.txt
M ui/ui-foundation/api/current.txt
M ui/ui-foundation/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-foundation/api/public_plus_experimental_current.txt
M ui/ui-foundation/api/restricted_0.1.0-dev15.txt
M ui/ui-foundation/api/restricted_current.txt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Clickable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Dialog.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Scroller.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Selectable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Toggleable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/semantics/FoundationSemanticsProperties.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ButtonTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CardTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CheckboxScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/RadioButtonScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ScaffoldTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SnackbarTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SurfaceTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ripple/RippleIndicationTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/textfield/TextFieldScreenshotTest.kt
M ui/ui-material/src/main/java/androidx/ui/material/AppBar.kt
M ui/ui-material/src/main/java/androidx/ui/material/TextFieldImpl.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/AssertsTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/CallSemanticsActionTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ErrorMessagesTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/FindersTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/PrintToStringTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ScrollToTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/TextActionsTest.kt
M ui/ui-test/src/main/java/androidx/ui/test/Actions.kt
M ui/ui-test/src/main/java/androidx/ui/test/Filters.kt
M ui/ui-text/api/0.1.0-dev15.txt
M ui/ui-text/api/current.txt
M ui/ui-text/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-text/api/public_plus_experimental_current.txt
M ui/ui-text/api/restricted_0.1.0-dev15.txt
M ui/ui-text/api/restricted_current.txt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/CoreTextField.kt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/TextSemanticsProperties.kt

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

ap...@google.com <ap...@google.com> #3May 29, 2024 04:12PM

Marked as fixed.

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

commit cdd35792067eba37f387920c537b10279a3bb110
Author: Marcello Galhardo <mgalhardo@google.com>
Date: Tue May 21 15:49:35 2024

Fix regression: ViewModel crash with compile-only dependency

* The conversion to Kotlin Multiplatform (KMP) caused the regression of aosp/2108693. This commit reintroduces the original fix, adapted to KMP.
* Due to significant code changes resulting from the conversion to KMP and the unavailability of all exception types, a new `createViewModel` function has been introduced. This function fallbacks differently depending on the platform it is running on.

RelNote: "`ViewModelProvider` will no longer crash when mixing previous versions of compileOnly Lifecycle dependencies with versions 2.8+."
Fixes:

b/341792251

Test: ViewModelTest
Change-Id: I803839b1c34bc8c70d6ebeb9ba90c426b340416c

A lifecycle/lifecycle-viewmodel/src/androidInstrumentedTest/kotlin/androidx/lifecycle/AndroidViewModelProviderTest.kt
A lifecycle/lifecycle-viewmodel/src/androidMain/kotlin/androidx/lifecycle/viewmodel/internal/ViewModelProviderImpl.android.kt
M lifecycle/lifecycle-viewmodel/src/commonMain/kotlin/androidx/lifecycle/viewmodel/internal/ViewModelProviderImpl.kt
A lifecycle/lifecycle-viewmodel/src/desktopMain/kotlin/androidx/lifecycle/viewmodel/internal/ViewModelProviderImpl.desktop.kt
A lifecycle/lifecycle-viewmodel/src/nonJvmMain/kotlin/androidx/lifecycle/viewmodel/internal/ViewModelProviderImpl.nonJvm.kt

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

mg...@google.com <mg...@google.com> #4Jun 3, 2024 02:45PM

For future reference on how to reproduce the issue:

Clone the repro project from this GitHub repository and update the dependencies.
- Note: the project in this GitHub repository also reproduces the issue.
Build the application by running:
```
./gradlew app:assembleDebug
```
- Note: The issue will not occur if you run the app from the Android Studio UI.

Install the application using the command:

adb install app/build/outputs/apk/debug/app-debug.apk`

Launch the application with the following command:

adb shell am start -n com.example.leak_viewmodel/com.example.leak_viewmodel.MainActivity -c android.intent.category.LAUNCHER -a android.intent.action.MAIN

And to validate that the solution works, change the repro project as following:

Create a new libs/ folder and included the custom .aar file with the solution from aosp/3096018.

Add a new module called :viewmodel-aar that serves as a wrapper to expose the .aar as an api:

dependencies {
    api(files("/Users/<USER_NAME>/Developer/leak_viewmodel/libs/lifecycle-viewmodel-debug.aar"))
}

Add a dependencySubstitution rule to the root build.gradle file:

allprojects {
    configurations.configureEach {
        resolutionStrategy {
            dependencySubstitution {
                substitute(module("androidx.lifecycle:lifecycle-viewmodel-android"))
                        .using(project(":viewmodel-aar"))
            }
        }
    }
}

Ran all the commands described in "how to reproduce the issue" above.

Message last modified on Jun 3, 2024 03:17PM

pr...@google.com <pr...@google.com> #5Jun 12, 2024 10:45PM

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

androidx.lifecycle:lifecycle-viewmodel:2.8.2
androidx.lifecycle:lifecycle-viewmodel-android:2.8.2
androidx.lifecycle:lifecycle-viewmodel-desktop:2.8.2
androidx.lifecycle:lifecycle-viewmodel-iosarm64:2.8.2
androidx.lifecycle:lifecycle-viewmodel-iossimulatorarm64:2.8.2
androidx.lifecycle:lifecycle-viewmodel-iosx64:2.8.2
androidx.lifecycle:lifecycle-viewmodel-linuxx64:2.8.2
androidx.lifecycle:lifecycle-viewmodel-macosarm64:2.8.2
androidx.lifecycle:lifecycle-viewmodel-macosx64:2.8.2

pr...@google.com <pr...@google.com> #6Aug 13, 2024 06:59PM