PopUpTo to parent NavGraph crashes when child nav-graph has non-nullable arguments [249988437]

Fixed

Bug

Status Update

No update yet.

Description

da...@sucharda.cz

created issue #1

Sep 30, 2022 02:32PM

Component used: Navigation
Version used: 2.5.1 and 2.5.2
Last known working version: 2.3.5
Devices/Android versions reproduced on: Huawei P20 Light / AN 9 (EMUI 9.1.0)

So our navigation is kinda complex so I will try to simplify the configuration. If there is a nested graph with arguments which cannot be null (and no default value is supplied) and popUpTo is used to remove nested graph during destination (nested) navigation it tries to recreate nested graph as a parent for the destination being navigated to.

<navigation xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:app="http://schemas.android.com/apk/res-auto"
    android:id="@+id/parentNavGraph"
    app:startDestination="@id/parentFragment">

    <fragment
        android:id="@+id/parentFragment"
        android:label="parentFragment"
        tools:layout="@layout/fragment_parent">

        <action
            android:id="@+id/actionToChildGraph"
            app:destination="@id/childNavGraph"/>

    </fragment>

    <navigation
        android:id="@+id/childNavGraph"
        app:startDestination="@id/childFragment">

        // Note: this is a duplicate due to NavArgs since it will not see "childFragment" args
        <argument
            android:name="argumentLong"
            app:argType="long"
            app:nullable="false"/>

        <fragment
            android:id="@+id/childFragment"
            android:label="childFragment"
            tools:layout="@layout/fragment_child">

            <argument
                android:name="argumentLong"
                app:argType="long"
                app:nullable="false"/>

            <action
                android:id="@+id/actionToChildFragment2"
                app:destination="@id/childFragment2"
                app:popUpTo="@id/childNavgraph"
                app:popUpToInclusive="true"/>

        </fragment>

        <fragment
            android:id="@+id/childFragment2"
            android:label="childFragment2"
            tools:layout="@layout/fragment_child_2"/>

</navigation>

So now if the navigation is triggered like this:

findNavController().navigate(
  ChildFragmentDirections.actionToChildFragment2()
)

It will pop fragments from back stack, but it will not change parent for ChildFragment2 which will be set to childNavGraph and since it is not in the back stack anymore it will try to recreate it with default values. But argumentLong is not nullable and has no default value so it will crash:

// NOTE: this crash is from Navigation version 2.5.1
java.lang.NullPointerException: null cannot be cast to non-null type kotlin.Long
  at androidx.navigation.NavType$Companion$LongType$1.get(NavType.kt:352)
  at androidx.navigation.NavType$Companion$LongType$1.get(NavType.kt:342)
  at androidx.navigation.NavArgument.verify(NavArgument.kt:76)
  at androidx.navigation.NavDestination.addInDefaultArgs(NavDestination.kt:502)
  at androidx.navigation.NavController.addEntryToBackStack(NavController.kt:1865)
  at androidx.navigation.NavController.addEntryToBackStack$default(NavController.kt:1813)
  at androidx.navigation.NavController$navigate$4.invoke(NavController.kt:1721)
  at androidx.navigation.NavController$navigate$4.invoke(NavController.kt:1719)
  at androidx.navigation.NavController$NavControllerNavigatorState.push(NavController.kt:287)
  at androidx.navigation.fragment.FragmentNavigator.navigate(FragmentNavigator.kt:246)
  at androidx.navigation.fragment.FragmentNavigator.navigate(FragmentNavigator.kt:162)
  at androidx.navigation.NavController.navigateInternal(NavController.kt:260)
  at androidx.navigation.NavController.navigate(NavController.kt:1719)
  at androidx.navigation.NavController.navigate(NavController.kt:1545)
  at androidx.navigation.NavController.navigate(NavController.kt:1472)
  at androidx.navigation.NavController.navigate(NavController.kt:1930)

Note: Interesting thing is that it does not crash with non-nullable String argument for some reason (ah I see, it is because String NavType is nullable by default: StringType: NavType<String?> vs LongType: NavType<Long>).

Possible fixes for us:

Do not use popUpTo to pop parent (navGraph) of a Fragment (node) from XML but use popBackStack from code instead.
Add default value to argumentLong.

Basically I am not sure if this an intended behavior or a bug. Gonna mark it as a bug because it worked in 2.3.5. I understand that Destination should have a parent but what is the reason to create a parent that does not exist and will never be displayed in this case, since it was already poped and thus it should not be displayed to the user.

At the moment two solutions come to mind:

Change new Fragment (node) parent to last destination in back stack after poping. Here I am not really sure if parent is designed this way and if it can be done without breaking something else.
Make LongType and other types nullable same as StringType. Again not sure if this can be done and it probably is not a proper solution (change).

There might of course be other solutions that I cannot see.

Thank you for your time and help.

Comments

da...@sucharda.cz <da...@sucharda.cz> #2Sep 30, 2022 02:38PM

Thank you for reporting this issue. For us to further investigate this issue, please provide the following additional information:

Steps to reproduce
Please provide source code or apk of a sample application to reproduce the issue.
Also kindly mention the steps to be followed for reproducing the issue with the given sample application.

Expected output
What is the expected output?

Current output
What is the current output?

jb...@google.com <jb...@google.com> #3Sep 30, 2022 04:37PM

Seeing this with a FragmentPagerAdapter as well. Our app has three static tabs, and opens by default on tab 1. Previously, the fragments for tab 2 and tab 3 would have their onResume() called when the activity's onResume() was called, even though those tabs weren't visible yet.

Now, since 27.1.0, onResume() on those tabs' fragments is only called when the user navigates to them.

da...@sucharda.cz <da...@sucharda.cz> #4Oct 20, 2022 04:26PM

#3 can you try to provide a repro app? I've lost 4 hours already and can't find the root cause, the basic AS sample does not produce that :(
Maybe it's easier from your code.

da...@sucharda.cz <da...@sucharda.cz> #5Oct 20, 2022 04:30PM

I also tried with the sample app (

https://github.com/googlesamples/android-HorizontalPaging) which doesn't have the issue.

I did find a clue in my app though: the first tab's fragment does getLoaderManager().initLoader() in its onActivityCreated(). If I comment this out, the onResume() methods of all the fragments are called at once as with support lib 27.0.2.

This might be related to this change in the support lib release notes:
> The underlying implementation of Loaders has been rewritten to use Lifecycle.

It's going to be quite a bit of work to get this into a small sample app though. I can't share my own app.

jb...@google.com <jb...@google.com> Nov 8, 2022 05:32PM

Assigned to cl...@google.com.

cl...@google.com <cl...@google.com> #6Mar 15, 2023 08:32PM

Marked as fixed.

Thanks I can also confirm the same thing in my case.

Removing the call to getLoaderManager().restartLoader(id, null, this); from onResume of the fragment then allows proper resuming of all fragments. (In my case all loaders Id are different).

This give a clue at how to reproduce, will see if I can tonight.

ap...@google.com <ap...@google.com> #7Mar 15, 2023 08:32PM

A sample app is here:

https://github.com/calvarez-ov/android-HorizontalPaging

If you build it as-is, you see three onResume() logs at once, when the app is launched:

02-28 14:03:43.534 15309 15309 V MainActivity/DummySectionFragment: onResume 1
02-28 14:03:43.682 15309 15309 V MainActivity/DummySectionFragment: onResume 2
02-28 14:03:43.682 15309 15309 V MainActivity/DummySectionFragment: onResume 3

If you change it to use support library 27.1.0 (Application/build.gradle), you only see the first onResume(). You see the other ones after you manually click on the tabs:

02-28 14:07:15.491 15443 15443 V MainActivity/DummySectionFragment: onResume 1
...
02-28 14:07:21.504 15443 15443 V MainActivity/DummySectionFragment: onResume 3
...
02-28 14:07:25.119 15443 15443 V MainActivity/DummySectionFragment: onResume 2

Note, this sample app doesn't look anything like the real app which has the same problem :) It's just the smallest example I could make to reproduce the issue. There's no ListView or RecyclerView and no adapter to display the loader data. I explicitly call forceLoad() to make the loader's loadInBackground() be called. If I don't do this, I have the bug even with support lib 27.0.2.

pr...@google.com <pr...@google.com> #8Mar 22, 2023 08:23PM

In case this helps troubleshooting: if you post the restartLoader() (or initLoader()) call to a Handler, the bug goes away:

@Override
public void onActivityCreated(@Nullable Bundle savedInstanceState) {
super.onActivityCreated(savedInstanceState);
- getLoaderManager().restartLoader(1337, null, this);
+ new Handler().post(new Runnable() {
+ @Override
+ public void run() {
+ getLoaderManager().restartLoader(1337, null, FragmentWithLoader.this);
+ }
+ });
}