Google Maps Wont restoreState properly when using restoreState = true while navigating.

[Del-S]

Environment details

1. Library: com.google.maps.android:maps-compose:6.2.0
2. Navigation library: androidx.navigation:navigation-compose:2.7.7

Steps to reproduce

1. Clone this repo.
2. Add google api key to manifest file.
3. Navigate to Map screen and then try to navigate to Map screen again.

Observation

Lifecycle observer goes to ON_DESTROY and never makes it's way to ON_RESUME, which keeps the map in "destroyed" state and it is not usable (cannot scroll, zoom or anything else).

While not using restoreState or saveState set to true the map works fine. Also when downgrading compose map library to 6.0.0 it works properly.

It might be related to already resolved issue: #112. But due to refactor in #436 it probably happens again. Not really connected to animations this time. It is probably connected to the restoreState.

Sample video of the issue.

[bubenheimer]

@Del-S I am not a project maintainer, but I guided the implementation of #436. What I see in the video is the user clicking the Map icon to switch to the Map screen, and then clicking the Map icon again while on the Map screen, resulting in the Map no longer working.

Can you explain why you think that there is a problem with GoogleMap()?

What the user is doing in the video should result in a no-op. It needs to be guarded by androidx.navigation and/or user code: "user tries to go to screen, but is on screen already".

Instead I see some lifecycle changes happening when I debug. I don't see how this is GoogleMap()'s problem.

I see that there is a need to harden navigation-related code against lifecycle problems in user code, these issues point at some recipes:
https://issuetracker.google.com/issues/317230685
https://issuetracker.google.com/issues/324170134

[Del-S]

@bubenheimer Hi, I would agree with that if that did not happen with Deeplinking (and fast clicking in some cases) as well. I just created the easiest case to implement. Basically when you are on a screen and you navigate to it with new intent (and/or new data) it should just restore the state, if it can right, and then deliver the new data into it. But the map (or the listener) end's up in "destroyed" state even when it is on screen and should be interactible. Not sure that should ever happen.

But if you say that it is "us" issue then I guess I can do nothing with that. I just can't imagine it's a good thing that the map is not usable even when on screen. 🤷

[bubenheimer]

@Del-S What I see in the case of the workflow from the video is the GoogleMap View lifecycle ending up in CREATED state after the user clicks the map navigation button while on the Map screen; the View sees Lifecycle.Event.ON_PAUSE, then Lifecycle.Event.ON_STOP, which puts it into CREATED state. I do not see the lifecycle ending up in DESTROYED state; there is no Lifecycle.Event.ON_DESTROY.

When I fast click to switch from Map screen to Main screen then Map screen while the transition animation to Main screen is ongoing I can get the GoogleMap View lifecycle temporarily stuck in CREATED state, upon re-entering the Map screen. Once I switch back to Main screen a second time after that, the GoogleMap View lifecycle goes to DESTROYED, and everything is back to normal when re-entering the GoogleMap screen the next time, the GoogleMap View is re-created in this case.

The behavior in the latter case suggests to me that guarding the screen switches with lifecycle barriers would help; similar guardrails may help in the former case, too.

The former case does not seem to behave for me in the manner that you describe: the lifecycle does not go to DESTROYED.

Also, what I observe is the GoogleMap View behaving as desired once it actually goes to DESTROYED lifecycle state.

If you have a repro for the case that actually interests you, it may help, to see what's going on in that case.

It may also help you to dig more into potential problems with Navigation behaviors, as opposed to GoogleMap behaviors.

I can only look at it from the perspective of what GoogleMap() sees in terms of lifecycle signals and view hierarchy changes, and its response to them. I don't see anything actionable at this point. Someone else may have other insights.

[bubenheimer]

I've looked at it once more and I see that there is more complexity involved. Will look into it some more.

[bubenheimer]

What I see now (in the case from the video) is that the LifecycleOwner changes. The View does see the ON_DESTROY lifecycle event from the old lifecycle owner, and around the same time a new LifecycleOwner is installed on the composition. The View is not detached from the hierarchy, and the composition is not disposed.

Someone with better knowledge of androidx.navigation needs to look at this:

1. Either the repro is invalid usage of Navigation.
2. Or it is valid. In this case I'd expect AndroidView to provide an option to handle this, as it affects all Views. Once a View sees ON_DESTROY it should be assumed to be dead. I don't see ready-made support for observing changes of LifecycleOwner directly from a View to prevent it from dying or to resurrect it. (Which again makes me wonder if this is a valid example.) Navigation may have something else to offer to address the situation at the View level.

If necessary, I think it could be worked around in different ways. For example, one could observe changes to LifecycleOwner from the composition, and cleanly recreate AndroidView in this case. Essentially key(lifecycleOwner) { AndroidView(...) }. This is pretty destructive, and more subtle alternatives may be preferable.

[Del-S]

Ah right it makes sense that the LifecycleOwner changes that seems to be the main "problem".

The repo is not a proper way of using bottom navigation -> I agree that the secondary click should result with a no-op but still there are cases when you are navigating to the screen for which you already have a savedState . For example deeplinking like I mentioned. I could add a deeplink to the same location with restore state because I do not want the map to reset on me when I deliver a new data into the screen (using a notification for example). And if the map cannot actually "restore" the state it is not really that good right?

We wanted to allow:

1. Deeplinking with the restoreState.
2. Restoring the state when the user navigates away and back to the screen. Example: map is used, user moves it from default position to some other and zooms. Clicks away to check other part of the app and then returns by clicking on the bottom menu with the Map again. We would like to have the map in the state that they left, which is not currently possible because if we restoreState it will not be usable. Well I am actually not sure if maps supports this. :D

Hmmm I'm thinking about key(lifecycleOwner) { AndroidView(...) } and shouldn't only the lifecycleObserver and maybe the onAttachStateListener be recreated when LifecycleOwner changes and then provided to the map view? Basically recreate only parts/component (whatever the name) that are dependent on the LifecycleOwner? Not sure if that can be done, I'm not an expert in compose maps.

Side-note: during the holidays I am planning to refactor navigation in the app so I might try/find some ways to make this work. :)

Anyway thanks for the help. 👍

[bubenheimer]

@Del-S for the time being you could try applying the key(lifecycleOwner) workaround yourself, wrap it around the GoogleMap(). Thinking about it, you will lose everything you remembered in the GoogleMap {...} block, so not actually a feasible workaround in the general case. Maybe you can make it work for your specific case.

[bubenheimer]

@Del-S the other workaround that you are suggesting may be feasible. I did not mention it, because the exact desired implementation and behavior at the time of View seeing ON_DESTROY and lifecycleOwner being replaced seem a little unclear, and it will take a bit of extra hacking to make it work, you could try it in a private fork. As my other workaround idea wouldn't pan out, this may be worth pursuing for you for now, if you do not find a different Navigation approach. You could possibly override setTag() on the MapView, too, and wait for a LifecyleOwner tag to be set. setTag because passing things around between Composition and View can get a tad messy in general. Also, it could be useful to understand how composition detects the change of LifecycleOwner and replicate that.

Because this looks somewhat murky, I think an actual solution should be deferred to AndroidView() and/or Navigation; like I said, all Views in AndroidView would share this problem, not just MapView. If it is a valid problem.

[Del-S]

Ok, great. Thanks for the help. 👍