[ActivityResult API] Inconsistent start request codes after restoring [168374000]

Fixed

Bug

Status Update

No update yet.

Description

se...@gmail.com

created issue #1

Sep 14, 2020 07:17AM

Component used: Activity

Version used: 1.2.0-alpha08

Devices/Android versions reproduced on: Pixel 2 XL Android 10

I've received several crash reports with the following stacktrace:

Fatal Exception: java.lang.ClassCastException: androidx.collection.ArrayMap cannot be cast to androidx.activity.result.ActivityResult
    at androidx.fragment.app.FragmentManager$10.onActivityResult(FragmentManager.java:2894)
    at androidx.activity.result.ActivityResultRegistry.dispatchResult(ActivityResultRegistry.java:316)
    at androidx.activity.ComponentActivity$2$1.run(ComponentActivity.java:158)
    at android.os.Handler.handleCallback(Handler.java:789)
    at android.os.Handler.dispatchMessage(Handler.java:98)
    at android.os.Looper.loop(Looper.java:164)
    at android.app.ActivityThread.main(ActivityThread.java:6944)
    at java.lang.reflect.Method.invoke(Method.java)
    at com.android.internal.os.Zygote$MethodAndArgsCaller.run(Zygote.java:327)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1374)

It looks like the issue was starting to happen after these changes:

https://android-review.googlesource.com/c/platform/frameworks/support/+/1302324

https://android-review.googlesource.com/c/platform/frameworks/support/+/1348342

The default start number of request codes is set to 0x00010000, but in onRestoreInstanceState the value is set to the key map size:

https://android.googlesource.com/platform/frameworks/support/+/refs/heads/androidx-master-dev/activity/activity/src/main/java/androidx/activity/result/ActivityResultRegistry.java#281

This line should probably look like this: mNextRc.set(0x00010000 + numKeys);

This issue is a copy of https://issuetracker.google.com/issues/157584993. I've decided to create a new issue in androidx activity component since there is a little chance that anyone will look at the old issue which is closed without a proper fix.

Comments

il...@google.com <il...@google.com> Sep 14, 2020 04:23PM

Assigned to jb...@google.com.

jb...@google.com <jb...@google.com> #2Sep 14, 2020 11:04PM

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

se...@gmail.com <se...@gmail.com> #3Sep 16, 2020 07:45PM

Unfortunately I'm unable to reproduce the issue neither in a sample nor in the real project.

These crashes are received from the users of production app. I'll try to add more logs for better understanding what users do before the crash.

ap...@google.com <ap...@google.com> #4Sep 17, 2020 01:26AM

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

commit b2a3eca325760e3686411e619048e2bc52befda8
Author: Jeremy Woods <jbwoods@google.com>
Date: Wed Sep 16 16:55:46 2020

Ensure request code start is consistent after restore

When the ActivityResultRegistry is recreated due to either config change
or process death, the next request code is started at the number of
current request codes without including the initial offset.

We should make sure to include the initial offset on restore to remain
consistent.

Relnote: "When restoring the ActivityResultRegistry, the request codes
are now properly started from their previous value instead of the value
of the number of existing request codes."
Test: all ActivityResultRegistryTest pass
Bug: 168374000

Change-Id: I463711fe19233877f1b534e4a7a3325fcfd8f983

M activity/activity/src/main/java/androidx/activity/result/ActivityResultRegistry.java

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

jb...@google.com <jb...@google.com> #5Sep 22, 2020 10:42PM

Marked as fixed.

This change ensures the request code matches, if your issue is not solved and you find a way to reproduce it, please file another bug.

ge...@gmail.com <ge...@gmail.com> #6Nov 25, 2020 05:11AM

https://drive.google.com/file/d/1A4DUnITG-Z70YWDvDcWuzdykliZ-CJZP/view?usp=drive_web (AppUpdater-2.6.5.zip)

Issue 168374000