Crash when rotating screen on custom destination (catch 22 situation) [110763345]

Fixed

Bug

Status Update

No update yet.

Description

sl...@czerwinski.info.pl

created issue #1

Jun 25, 2018 10:25AM

Component used: Navigation Component
Version used: 1.0.0-alpha02
Devices/Android versions reproduced on: Emulator API 28, Samsung J5 API 25

Sample project attached.

I'm trying to create a custom Navigator and custom NavDestination that would animate ConstraintLayout (but any custom NavDestination would cause this issue).
Intended behaviour is shown in attached video: Layout_animations.webm

However, when I rotate the screen, while the navigation is on the custom destination, the application crashes.
The crash is caused by an unknown destination, which can only be added after custom navigator.
If I add custom destination in nav_main.xml, the app will also crash, because the navigator is not yet added.

This issue could be fixed by extending NavHostFragment, but I would need to override the whole onCreate() method.
I would need to add my custom navigator right after the line:
mNavController = new NavController(context);

I can think of 2 possible solutions in the library:
1. Delegate creating of NavController, so that a custom NavController can be provided in the XML layout file.
2. Call a new protected method "NavHostFragment.onNavControllerCreated(NavController)" right after the NavController is created, that could be overridden in a custom NavHostFragment, before the graph is created.

If there is any solution to my problem that I didn't think of, please provide a more exhaustive example in the documentation:

https://developer.android.com/topic/libraries/architecture/navigation/navigation-add-new

Stack trace:

2018-06-25 10:54:28.356 14543-14543/it.czerwinski.customnavigatorexample E/AndroidRuntime: FATAL EXCEPTION: main
Process: it.czerwinski.customnavigatorexample, PID: 14543
java.lang.RuntimeException: Unable to start activity ComponentInfo{it.czerwinski.customnavigatorexample/it.czerwinski.customnavigatorexample.MainActivity}: android.view.InflateException: Binary XML file line #10: Binary XML file line #10: Error inflating class fragment
at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2925)
at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:3060)
at android.app.ActivityThread.handleRelaunchActivityInner(ActivityThread.java:4792)
at android.app.ActivityThread.handleRelaunchActivity(ActivityThread.java:4711)
at android.app.servertransaction.ActivityRelaunchItem.execute(ActivityRelaunchItem.java:69)
at android.app.servertransaction.TransactionExecutor.executeCallbacks(TransactionExecutor.java:110)
at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:70)
at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1800)
at android.os.Handler.dispatchMessage(Handler.java:106)
at android.os.Looper.loop(Looper.java:164)
at android.app.ActivityThread.main(ActivityThread.java:6649)
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:826)
Caused by: android.view.InflateException: Binary XML file line #10: Binary XML file line #10: Error inflating class fragment
Caused by: android.view.InflateException: Binary XML file line #10: Error inflating class fragment
Caused by: java.lang.IllegalStateException: unknown destination during restore: it.czerwinski.customnavigatorexample:id/animatedLayoutDestination
at androidx.navigation.NavController.onGraphCreated(NavController.java:424)
at androidx.navigation.NavController.setGraph(NavController.java:386)
at androidx.navigation.NavController.restoreState(NavController.java:742)
at androidx.navigation.fragment.NavHostFragment.onCreate(NavHostFragment.java:215)
at androidx.fragment.app.Fragment.performCreate(Fragment.java:2400)
at androidx.fragment.app.FragmentManagerImpl.moveToState(FragmentManager.java:1418)
at androidx.fragment.app.FragmentManagerImpl.moveToState(FragmentManager.java:1684)
at androidx.fragment.app.FragmentManagerImpl.onCreateView(FragmentManager.java:3772)
at androidx.fragment.app.FragmentController.onCreateView(FragmentController.java:120)
at androidx.fragment.app.FragmentActivity.dispatchFragmentsOnCreateView(FragmentActivity.java:395)
at androidx.fragment.app.FragmentActivity.onCreateView(FragmentActivity.java:377)
at android.view.LayoutInflater.createViewFromTag(LayoutInflater.java:780)
at android.view.LayoutInflater.createViewFromTag(LayoutInflater.java:730)
at android.view.LayoutInflater.rInflate(LayoutInflater.java:863)
at android.view.LayoutInflater.rInflateChildren(LayoutInflater.java:824)
at android.view.LayoutInflater.inflate(LayoutInflater.java:515)
at android.view.LayoutInflater.inflate(LayoutInflater.java:423)
at android.view.LayoutInflater.inflate(LayoutInflater.java:374)
at androidx.appcompat.app.AppCompatDelegateImpl.setContentView(AppCompatDelegateImpl.java:467)
at androidx.appcompat.app.AppCompatActivity.setContentView(AppCompatActivity.java:140)
at it.czerwinski.customnavigatorexample.MainActivity.onCreate(MainActivity.kt:15)
at android.app.Activity.performCreate(Activity.java:7130)
at android.app.Activity.performCreate(Activity.java:7121)
at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1262)
at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2905)
at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:3060)
at android.app.ActivityThread.handleRelaunchActivityInner(ActivityThread.java:4792)
at android.app.ActivityThread.handleRelaunchActivity(ActivityThread.java:4711)
2018-06-25 10:54:28.356 14543-14543/it.czerwinski.customnavigatorexample E/AndroidRuntime: at android.app.servertransaction.ActivityRelaunchItem.execute(ActivityRelaunchItem.java:69)
at android.app.servertransaction.TransactionExecutor.executeCallbacks(TransactionExecutor.java:110)
at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:70)
at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1800)
at android.os.Handler.dispatchMessage(Handler.java:106)
at android.os.Looper.loop(Looper.java:164)
at android.app.ActivityThread.main(ActivityThread.java:6649)
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:826)

Sample Project.zip

20 MB

Download

Layout_animations.webm

1.8 MB

Download

Comments

il...@google.com <il...@google.com> Jun 25, 2018 11:01PM

Assigned to il...@google.com.

il...@google.com <il...@google.com> #2Jun 27, 2018 06:27PM

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

sl...@czerwinski.info.pl <sl...@czerwinski.info.pl> #3Jun 28, 2018 09:37AM

The first solution does not work, because after screen rotation, the graph (probably a restored graph) is set when Activity.setContentView() is called.

The workaround with createFragmentNavigator() works, though it's not very elegant. Maybe the method could be renamed to createNavigators() and return a list or an array of Navigators?
Anyway, it is a good enough solution for now, so it is not very urgent.

In conclusion:
* I think that a better support for custom destinations should be provided before final release.
* I would suggest describing the "Add support for new destination types" section in the documentation in more details.

Thank you

il...@google.com <il...@google.com> #4Jun 28, 2018 04:06PM

I suspect the issue you're running into for the first solution is that you're using the setGraph(R.navigation.graph) method, which does indeed cause it to automatically inflate the graph by id on rotation. Using setGraph(navGraph) where navGraph is a navGraph object you've inflated with navController.getNavInflater().inflate(R.navigation.graph) will never automatically be recreated.

Definitely something we can do better on though :)

il...@google.com <il...@google.com> Oct 17, 2018 11:27PM

Accepted by il...@google.com.

ap...@google.com <ap...@google.com> #5Oct 18, 2018 04:25AM

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

commit a2e68921841727237528c4ac7076d50af64663c8
Author: Ian Lake <ilake@google.com>
Date: Wed Oct 17 16:46:28 2018

Make setGraph(int) and setGraph(NavGraph) act identically

Remove the auto restore functionality from NavController's
setGraph(int) method so that it functions identically to
setGraph(NavGraph).

This ensures that developers that manually are calling
setGraph() after adding custom navigators don't get
recreated too early.

Test: updated NavController test
BUG: 110763345
Change-Id: I51ae7c74451669d6bfed0fe84df445c2ff272e6e

M navigation/runtime/src/androidTest/java/androidx/navigation/NavControllerTest.kt
M navigation/runtime/src/main/java/androidx/navigation/NavController.java

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

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

il...@google.com <il...@google.com> #6Oct 18, 2018 04:29AM

Marked as fixed.

We've decided to remove the automatic recreation that was unique to setGraph(int) - it now acts identically to setGraph(NavGraph), so the flow in #2 can be used to ensure that you have custom Navigators added before calling setGraph.

This fix will be available in 1.0.0-alpha07

ap...@google.com <ap...@google.com> #7Oct 24, 2018 11:11AM

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

commit f40bad0faef05bded2d226711338e489309885ba
Author: Ian Lake <ilake@google.com>
Date: Tue Oct 23 21:03:24 2018

Ensure NavHostFragment calls setGraph on restore

If a graph is set on NavHostFragment via
NavHostFragment.create() or through
app:navGraph, it should be restored immediately
after the call to restoreState.

Test: testapp now works
BUG: 110763345
Change-Id: I7a0c656480e27a66637667519eabe61820b09a6c

M navigation/fragment/src/main/java/androidx/navigation/fragment/NavHostFragment.java

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

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