Safe Args Kotlin fromBundle() method is being stripped by ProGuard [190082521]

Fixed

Bug

Status Update

No update yet.

Description

ia...@gmail.com

created issue #1

Jun 3, 2021 10:56PM

Component used: Navigation Safe Args Kotlin Plugin

Version used: 2.4.0-alpha02, AGP 4.2.0-beta03

After upgrading to Navigation 2.4.0-alpha02 and building a minified version of my app, my app crashed with an exception that said that the fromBundle method couldn't be found.

I use the by navArgs() extension, which uses reflection to access this method, so I depended on Navigation's included ProGuard keep rules to properly keep that method.

I found that I could work around this issue via this commit:

# Workaround for Kotlin Safe Args issue
-if public class ** implements androidx.navigation.NavArgs
-keepclassmembers public class <1> {
    public static ** Companion;
    ** fromBundle(android.os.Bundle);
}

Essentially just making sure that the Companion class is kept. After adding that ProGuard rule, the fromBundle() class was successfully kept. I'm not sure why that helped, since I'm pretty sure that the format of the fromBundle() didn't change since Navigation 2.3.5?

Reproduction steps:

Check out Muzei
Revert this commit
Add minifyEnabled true to the debug configuration
Run ./gradlew assembleDebug and analyze the resulting APK to verify that the ChooseProviderFragmentArgs.fromBundle() method exists (it won't) 4a) Alternatively, install Muzei, Activate it as your wallpaper, then select the 'Sources' screen. Muzei should not crash.

Comments

il...@google.com <il...@google.com> Jun 3, 2021 11:04PM

Assigned to il...@google.com.

il...@google.com <il...@google.com> #2Jun 10, 2021 12:06AM

Accepted by il...@google.com.

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> #3Jun 10, 2021 01:18AM

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

commit 55af5f168e5a1e6dc73c8d35ac79598cb4ae5bd8
Author: Ian Lake <ilake@google.com>
Date: Wed Jun 09 16:57:14 2021

Move ProGuard rules from navigation-common-ktx to common

NavArgsLazy was moved to navigation-common, but
the related proguard-rules.pro wasn't moved with it.

This meant that apps that didn't specifically include
the (now empty) navigation-common-ktx artifact wouldn't
get the correct set of ProGuard rules.

Test: tested in sample app
BUG: 190082521
Relnote: "ProGuard rules for `by navArgs()` now properly
apply when using `navigation-common` or any artifacts
that depend on it."

Change-Id: I688009632403c98afe728d856ce91cd056ef32a5

M navigation/navigation-common-ktx/build.gradle
M navigation/navigation-common/build.gradle
M navigation/navigation-common/proguard-rules.pro

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

il...@google.com <il...@google.com> #4Jun 10, 2021 03:28PM

Marked as fixed.

This has been fixed internally and will be available in Navigation 2.4.0-alpha03.

Issue 190082521

Description

Issue summary

Comments

il...@google.com <il...@google.com> Jun 3, 2021 11:04PM

il...@google.com <il...@google.com> #2Jun 10, 2021 12:06AM

ap...@google.com <ap...@google.com> #3Jun 10, 2021 01:18AM

il...@google.com <il...@google.com> #4Jun 10, 2021 03:28PM

Add comment

Issue metadata