androidx.navigation.dynamicfeatures.fragment.ui.AbstractProgressFragment navigate multiple time [169636207]

Fixed

Bug

Status Update

No update yet.

Description

pq...@gmail.com

created issue #1

Sep 30, 2020 04:14AM

Component used: androidx.navigation:navigation-dynamic-features-fragment

Version used:2.3.0 Devices/Android versions reproduced on: all devices/android versions

override fun onResume() { super.onResume() if (navigated) { findNavController().popBackStack() return } var monitor = installViewModel.installMonitor if (monitor == null) { Log.i(TAG, "onResume: monitor is null, navigating") navigate() monitor = installViewModel.installMonitor } if (monitor != null) { Log.i(TAG, "onResume: monitor is now not null, observing") monitor.status.observe(this, StateObserver(monitor)) } }

Any time onResume() is called, monitor.status observed. When dynamic feature module is installed, each StateObserver will call navigate() and destination activity/fragment will be navigated multiple time

Comments

jb...@google.com <jb...@google.com> Oct 2, 2020 07:20PM

Assigned to be...@google.com.

be...@google.com <be...@google.com> #2Oct 5, 2020 03:18PM

I'd like to add that this automatic removal of the NavGraph at the top of the stack is also making it hard for me to implement proper lateral top-level navigation.
A practical case where this kind of navigation happens is with a Material Design bottom navigation bar, where I have a NavHost above the bottom bar.

Given this navigation scheme:
A - B (top-level)
B1 (first sub-level)

When starting the Activity, the back stack is
0. A
1. NavGraph

If I wanted to navigate to B laterally, I would do:
navController.popBackStack(R.id.navGraph, false)
navController.navigate(R.id.B)

But the resulting back stack is:
0. B

Now, navigating between top-level destinations actually works (though popBackStack returns false); the problem arises again if I navigate to B1:
0. B1
1. B

When navigating from B1 to a top-level destination like A, the stack becomes:
0. A
1. B1
2. B

That is, A is not a top-level destination in the back stack: if I press back from A, I intend to exit the app, but I'm taken back to B1.

It would be nice to have a fixed reference to the bottom of the stack. Right now I'm working around this with a horrible ` while (navController.popBackStack()) continue`.

ap...@google.com <ap...@google.com> #3Nov 4, 2020 06:40PM

Is this going to be fixed?

I have the same issue with a setup as follows:

Navigation graph.

<navigation xmlns:android="

http://schemas.android.com/apk/res/android"
xmlns:app="

http://schemas.android.com/apk/res-auto"
xmlns:tools="

http://schemas.android.com/tools"
app:startDestination="@+id/startupGraph">

<navigation
android:id="@+id/startupGraph"
app:startDestination="@id/launcher_home">
<fragment
android:id="@+id/launcher_home"
android:name="com.example.android.codelabs.navigation.MainFragment"
android:label="@string/home"
tools:layout="@layout/main_fragment" />
</navigation>

<navigation
android:id="@+id/homeGraph"
app:startDestination="@id/flow_step_one">
<fragment
android:id="@+id/flow_step_one"
android:name="com.example.android.codelabs.navigation.FlowStepOneFragment"
tools:layout="@layout/flow_step_one_fragment" />
<fragment
android:id="@+id/flow_step_two"
android:name="com.example.android.codelabs.navigation.FlowStepTwoFragment"
tools:layout="@layout/flow_step_two_fragment" />
</navigation>

</navigation>

MainFragment.

override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
super.onViewCreated(view, savedInstanceState)
view.findViewById<Button>(R.id.navigate_dest_bt).setOnClickListener{
Navigation.findNavController(it)
.navigate(R.id.homeGraph, null, NavOptions.Builder()
.setPopUpTo(R.id.startupGraph, false)
.build())
}
}

FlowStepOneFragment.

view.findViewById<View>(R.id.next_button).setOnClickListener {
Navigation.findNavController(it)
.navigate(R.id.flow_step_two, null, NavOptions.Builder()
.setPopUpTo(R.id.flow_step_one, true)
.build())
}
}
}

Causes the following error:

E/AndroidRuntime: FATAL EXCEPTION: main
Process: com.example.android.codelabs.navigation, PID: 12579
java.lang.IllegalArgumentException: Navigator androidx.navigation.fragment.FragmentNavigator@d70ec6f reported navigation to unknown destination id com.example.android.codelabs.navigation:id/flow_step_two

il...@google.com <il...@google.com> #4Nov 4, 2020 07:14PM

Marked as fixed.

The original issue was fixed as a result of

https://android-review.googlesource.com/833717 and a number of other internal changes and that will be available in alpha08.

Discovered another issue when doing this specific set of navigation actions, so leaving this open until that part is also fixed.

b9...@gmail.com <b9...@gmail.com> #5Dec 14, 2020 05:37PM

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

commit d3507cb9dab82264070c72a37dbde1dfb43ef578
Author: Ian Lake <ilake@google.com>
Date: Tue Dec 04 14:24:32 2018

Split pop and dispatch logic

When using popUpTo, navigate() would
internally call popBackStack(), resulting
in additional dispatch calls to
OnDestinationChangedListener before the
navigate() operation itself was done.

By separating the logic, we can ensure that
we don't prematurely remove NavGraph destinations
that should still exist after the navigate()
operation and that developers only see a
single final OnDestinationChangedListener callback.

Test: new test
BUG: 113611083
Change-Id: Ib63e4156521259a945ffecf679ccb404ac192017

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

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

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

be...@google.com <be...@google.com> #6Dec 14, 2020 05:58PM

onViewCreated on Fragment is a GNDN function, so no need to override it.

Can you provide more information as to how your installation won't execute?

b9...@gmail.com <b9...@gmail.com> #7Dec 14, 2020 07:28PM

Move install monitor observation to onViewCreated

DefaultProgressFragment won't execute super(AbstractProgressFragment)'s onViewCreated(). So after this change, DefaultProgressFragment can't monitor the installation progress.

b9...@gmail.com <b9...@gmail.com> #8Dec 14, 2020 07:33PM

TL;DR

Call super.onViewCreated(view, savedInstanceState) in DefaultProgressFragment will solve this issue.

ap...@google.com <ap...@google.com> #9Dec 15, 2020 05:32PM

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

commit 524a4f074b688975041f6ddd9db9eb31a88410e8
Author: Ben Weiss <benweiss@google.com>
Date: Tue Dec 15 12:26:06 2020

Add missing onViewCreated call

DefaultProgressFragment did not call super.onViewCreated which
could lead to installation progress not completing.

Bug: 169636207
Test: N/A
RelNote: Fix stuck installation progress
Change-Id: Ib27a77bc4572fc69cea38d738255c987449d6137

M navigation/navigation-dynamic-features-fragment/src/main/java/androidx/navigation/dynamicfeatures/fragment/ui/DefaultProgressFragment.kt

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

b9...@gmail.com <b9...@gmail.com> #10Feb 4, 2021 04:07PM

Why this fix doesn't included in 2.3.3?

il...@google.com <il...@google.com> #11Feb 10, 2021 01:03AM

Re #10, yes, you're right this didn't make it into Navigation 2.3.3. Sorry about that. We'll look at doing a Navigation 2.3.4 with this fix.

b9...@gmail.com <b9...@gmail.com> #12Mar 6, 2021 03:47PM

#11 Still no 2.3.4 released.

za...@gmail.com <za...@gmail.com> #13Jul 4, 2022 02:51PM

I believe I'm facing another issue because of this fix:

When navigating to an activity that belongs to a dynamic module, the AbstractProgressFragment is not removed from the back stack. The line that's supposed to do it was moved from onResume to onViewCreated:

if (navigated) {
    findNavController().popBackStack()
    return
}

And onViewCreated is not called since we're coming back from another activity.

Moving that block back to the onResume method should solve the problem. In the meantime (as a workaround), I guess we can do as follows:

class CustomProgressFragment : AbstractProgressFragment() {

    private var navigated = false

    override fun onResume() {
        super.onResume()
        // Workaround:
        // AbstractProgressFragment is not being removed from the back stack when returning from another activity
        if (navigated) findNavController().popBackStack()
    }

    override fun onInstalled() {
        testing = true
    }
   
   ...
   ...
}

za...@gmail.com <za...@gmail.com> #14Jul 4, 2022 02:54PM

Update: Regarding the workaround I mentioned, this is the correct implementation of the onInstalled method

    override fun onInstalled() {
        navigated = true
    }

il...@google.com <il...@google.com> #15Jul 13, 2022 11:18PM

Re comment#13 - please file a separate bug with a sample project that reproduces your issue. onViewCreated() is indeed called when you return to the AbstractProgressFragment from another fragment, which is the only thing your newly loaded graph should have as its start destination. You...don't happen to have a start destination that is an activity destination? You shouldn't be using Dynamic Navigation at all in that case (as the system will auto-download the module your activity is in upon request).