AppCompatActivity saved bundle size increases after every orientation change [191893160]

Fixed

Bug

Status Update

No update yet.

Description

sq...@gmail.com

created issue #1

Jun 23, 2021 07:27PM

Component used: AppCompatActivity

Version used: 1.3.0

Devices/Android versions reproduced on: Emulator API 28, 30, Samsung Galaxy S21 API 30, most likely all versions are affected

Each time I change the orientation of my screen, the saved instance bundle size increases by 3 elements in two different ArrayLists.

Here is the minimal example that will illustrate the issue. The additional code in the onSaveInstanceState is only there to display the issue.

class MainActivity : AppCompatActivity() {

    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        setContentView(R.layout.activity_main)
    }

    override fun onSaveInstanceState(outState: Bundle) {
        super.onSaveInstanceState(outState)

        val rcs = outState.getIntegerArrayList("KEY_COMPONENT_ACTIVITY_REGISTERED_RCS")
        val keys = outState.getStringArrayList("KEY_COMPONENT_ACTIVITY_REGISTERED_KEYS")

        Log.d("MainActivity", "${rcs?.size}: $rcs, $keys")
    }
}

Here is the full output when rotated 6 times: https://pastebin.com/yfE04Fmc

Mini-extract (2 rotations):

3: [1332505437, 1835553837, 670316111], [FragmentManager:StartIntentSenderForResult, ...
6: [1332505437, 91080073, 1835553837, 381123153, 1187376284, 670316111], ...
...

As you can see, each time the screen is rotated, 3 elements are added to these two entries. After rotating the screen for a while, the instance state becomes significantly bigger for no apparent reason.

I traced it back to (but can be wrong) ActivityResultRegistry, and it seems that something in the support library doesn't clean up the state with the registry.

I also tried simply putting the app in the background, which triggers the onSaveInstanceState but that did not increase the bundle size. It looks like the activity must be re-created.

When I inherit from Activity instead, the issue disappears.

Comments

al...@google.com <al...@google.com> Jun 23, 2021 07:53PM

Reassigned to jb...@google.com.

jb...@google.com <jb...@google.com> #2Jun 23, 2021 08:57PM

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

sq...@gmail.com <sq...@gmail.com> #3Jun 23, 2021 09:01PM

Just pushed it to git: https://github.com/sQu1rr/fragment-result-registry hope it helps

ap...@google.com <ap...@google.com> #4Jul 1, 2021 08:20PM

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

commit 82faca9ddbd48485b8752cc702891e45a00bcd92
Author: Jeremy Woods <jbwoods@google.com>
Date: Wed Jun 23 21:40:35 2021

Fix ActivityResultRegistry save/restore

The ActivityResultRegistry is currently restored after the on
ComponentActivity call to super.onCreate(). This means that any
callbacks registered before that could have already been saved waiting
to be restored so we end up improperly registering several duplicate
callbacks.

We should handle this on two fronts, one in ActivityResultRegistry for
cases outside of ComponentActivity and inside of ComponentActivity.

For ComponentActivity we should save earlier as part of an OnContextAvailableListener to ensure that we restore the callbacks as soon as possible.

For ActivityResultRegistry, we should make sure that even if we register
before a call on onRestoreInstanceState, we don't duplicate keys.
Specifically, we need to restore the old requestCode to ensure that any
launched results that could potentially be dispatched still have a valid
requestCode, otherwise the dispatch would be ignored.

RelNote: "The ActivityResultRegistry callbacks are now properly saved
and restored so callbacks are not duplicated in the savedState."
Test: Added new test
Bug: 191893160

Change-Id: I9781617370ad24f768249df42d2ab148915097cb

M activity/activity/src/androidTest/AndroidManifest.xml
M activity/activity/src/androidTest/java/androidx/activity/ComponentActivityResultTest.kt
M activity/activity/src/androidTest/java/androidx/activity/result/ActivityResultRegistryTest.kt
M activity/activity/src/main/java/androidx/activity/ComponentActivity.java
M activity/activity/src/main/java/androidx/activity/result/ActivityResultRegistry.java

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

jb...@google.com <jb...@google.com> #5Jul 12, 2021 08:26PM

Marked as fixed.

This has been fixed internally and will be part of both the Activity 1.3.0-rc02 and 1.2.4 releases.

am...@gmail.com <am...@gmail.com> #6Jul 3, 2024 09:02PM

Hi 👋

Issue 191893160