Press back or navigate to other fragment before call postponeEnterTransition cause crash during sharedElementTransition [138251858]

Fixed

Bug

Status Update

No update yet.

Description

lc...@gmail.com

created issue #1

Jul 24, 2019 02:02AM

Component used: Navigation
Version used: 2.1.0-beta02
Devices/Android versions reproduced on:

When using sharedElementTransition from Fragment A to Fragment B, we call Fragment#postponeEnterTransition first at Fragment B onCreateView.
Then if we press back button or navigate to other fragment before Fragment#startPostponedEnterTransition of Fragment B was called, the following exception will happen when trying to navigate to other fragments.

StackTrace:
E/AndroidRuntime: FATAL EXCEPTION: main
Process: com.example.fragmenttransitionbug, PID: 8900
java.lang.NullPointerException: Attempt to invoke virtual method 'boolean androidx.fragment.app.FragmentManagerImpl.isPrimaryNavigation(androidx.fragment.app.Fragment)' on a null object reference
at androidx.fragment.app.Fragment.performPrimaryNavigationFragmentChanged(Fragment.java:2662)
at androidx.fragment.app.FragmentManagerImpl.dispatchParentPrimaryNavigationFragmentChanged(FragmentManagerImpl.java:2805)
at androidx.fragment.app.FragmentManagerImpl.setPrimaryNavigationFragment(FragmentManagerImpl.java:2800)
at androidx.fragment.app.BackStackRecord.executeOps(BackStackRecord.java:422)
at androidx.fragment.app.FragmentManagerImpl.completeExecute(FragmentManagerImpl.java:1989)
at androidx.fragment.app.FragmentManagerImpl$StartEnterTransitionListener.completeTransaction(FragmentManagerImpl.java:3379)
at androidx.fragment.app.FragmentManagerImpl.executePostponedTransaction(FragmentManagerImpl.java:1769)
at androidx.fragment.app.FragmentManagerImpl.removeRedundantOperationsAndExecute(FragmentManagerImpl.java:1800)
at androidx.fragment.app.FragmentManagerImpl.execPendingActions(FragmentManagerImpl.java:1727)
at androidx.fragment.app.FragmentManagerImpl$2.run(FragmentManagerImpl.java:150)
at android.os.Handler.handleCallback(Handler.java:873)
at android.os.Handler.dispatchMessage(Handler.java:99)
at android.os.Looper.loop(Looper.java:193)
at android.app.ActivityThread.main(ActivityThread.java:6669)
at java.lang.reflect.Method.invoke(Native Method)
at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:493)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:858)

Is this the correct behavior?
Since it's a common pattern when using Glide to load a remote image with sharedElementTransition.
We want to call Fragment#startPostponedEnterTransition after the remote image was ready to show.
But the loading can take some time which cause the above situation to happen.
A sample project was attached.

If this is a bug in the library, we would appreciate if you could attach:
- Sample project to trigger the issue.
- A screenrecord or screenshots showing the issue (if UI related).

fragmentTransition.tar

31 MB

Download

Comments

il...@google.com <il...@google.com> #2Jul 24, 2019 05:28AM

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

ma...@cyberagent.co.jp <ma...@cyberagent.co.jp> #3Jul 24, 2019 05:58AM

After update to Fragments 1.2.0-alpha01, it seems the same exception still happens (Also tried update appcompat:1.1.0-rc01)
The latest example project was attached

fragmentTransition.tar

28 MB

Download

lc...@gmail.com <lc...@gmail.com> #4Jul 24, 2019 06:01AM

Sorry, <ma...@cyberagent.co.jp> is also my mail.

il...@google.com <il...@google.com> #5Jul 24, 2019 06:09PM

Assigned to il...@google.com.

Thanks for confirming, I was able to reproduce your issue.

il...@google.com <il...@google.com> Jul 24, 2019 09:35PM

Accepted by il...@google.com.

ap...@google.com <ap...@google.com> #6Jul 24, 2019 10:31PM

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

commit e6ce39f90140f19068852a99713e19680ff20559
Author: Ian Lake <ilake@google.com>
Date: Wed Jul 24 14:30:55 2019

Remove cancelled postponed transactions when popped

When popping a postponed fragment transaction,
remove the transaction from the list of postponed
transactions when cancelling it.

This prevents cases where we attempt to execute
the postponed transaction despite it already
being cancelled.

Test: new test passes
BUG: 138251858
Change-Id: Ie2f23923ad761a35c43df8716e46e51183dbc010

M fragment/fragment/src/androidTest/java/androidx/fragment/app/PrimaryNavFragmentTest.kt
M fragment/fragment/src/main/java/androidx/fragment/app/FragmentManager.java

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

https://goto.google.com/android-sha1/e6ce39f90140f19068852a99713e19680ff20559

an...@google.com <an...@google.com> #7Jul 24, 2019 10:40PM

The following changes were cherrypicked through Coastguard (

https://android-build.googleplex.com/coastguard/)
Release Track: (

https://android-build.googleplex.com/coastguard/dashboard/6552735953321984/#/cherrypickHistory/all)
Created: 2019-07-24 22:38:18.725872+00:00
Changes: aosp/1090768

il...@google.com <il...@google.com> #8Jul 24, 2019 10:56PM

Marked as fixed.

Turns out that this was actually a more serious, long standing issue with Fragments that was just exposed by more recent changes.

We'll be releasing a new 1.1.0-rc04 for Fragments with this fix and update Navigation 2.1 to depend on it.