Multiple MainActivities when opening via sync notification

[Helium314]

How to Reproduce

• Start StreetComplete
• scan for quests
• while scanning for quests, switch to a different app (so a SC notification appears)
• open SC by clicking the notification
• (optional) move map to a different position
• try going back to home screen by pressing back button
• still see StreetComplete (with the optional step, notice that map is now at earlier position)

Expected Behavior
Return to home screen when pressing back button, like it happens without the steps above

Versions affected
52.0 and likely older ones

Investigating what is going on reveals that MainActivity.onCreate is being called when the notification is clicked, but no MainActivity is destroyed before (this happens when pressing back button).
Further, SelectedOverlayController gets an additional MainMapFragment listener when opening SC via notification.

(I tried using various intent flags in createSyncNotification to re-use the existing MainActivity, but wasn't able to get it working)
Edit: works when using android:launchMode="singleInstance", but this causes issues with StreetMeasure on some Android Versions, see Helium314#426. Though maybe that one could be avoided using some intent flags...

[westnordost]

Hm, this is bad. Does this mean that when pressing the equivalent of alt+tab, there'll be two StreetCompletes? Also, how bad is this really? When the other StreetComplete is also there, it should be inactive (i.e. no listeners etc) so it should not be using any resources.

Looking at the documentation for launchMode, singleTask is indeed what we'd want to have. Or singleInstance? Hm, to be honest, I don't really understand it from the documentation alone. I'd think that most applications would want to run just once. Which is why this confuses me a bit. Why is this not standard?
On the other hand, this is about the Activity, not the application.... hm.

(Edit: Grammar)

[Helium314]

Does this mean that when pressing the equivalent of alt+tab, there'll be two StreetCompletes?

Only when going back to SC using the notification

no listeners etc

There is a MainMapFragment listener on SelectedOverlayController remaining. Could probably be avoided by adding/removing in onResume / onPause

how bad is this really?

Maybe not that bad, because I assume it will happen very rarely (more of an issue for SCEE with the nearby quest monitor #48).
Though, could there be bad things if there is a quest open in the old ("hidden") MainFragment?

singleTask is indeed what we'd want to have

I think so too... I'll have a look whether it does what we want.
(maybe there is a difference if the user goes to settings before clicking the notification?)

[Helium314]

singleTask is indeed what we'd want to have

Definitely singleTask:

• standard: clicking notification from settings directs to MainActivity, and when pressing back we go to settings again, then on back go to the original MainActivity
• singleInstance: clicking notification from settings directs to MainActivity, and when pressing back we go to settings, then on back exit the app (very counter-intuitive)
• singleTask: clicking notification from settings directs to MainActivity, and when pressing back exit the app (like it happens normally)

So the next question is whether on some devices singleTask exits the app when starting StreetMeasure, like it happens with singleInstance.

[tapetis]

I don't think changing the launchMode to a non-standard value is a good idea. If I understood your problem correctly, it sounds like setting the Intent.FLAG_ACTIVITY_CLEAR_TOP flag for the Intent in the notification is the correct solution. Did you try that?

I would also argue that not removing all listeners when the Activity is in the background is a problem of its own. But please note that because of Android's multi-window support, these listeners should be added / removed in onStart / onStop and not in onResume / onPause.

[Helium314]

I had tried a lot of flags that are supposed to help (quite sure that was including FLAG_ACTIVITY_CLEAR_TOP), and always a new MainActivity was created.

[Helium314]

Oh, and currently GeoUris are not handled are only handled onMapInitialized, so not when using singleInstance or singleTask.

[tapetis]

Yes, it is a different problem. I just wanted to comment on the question whether there are still active listeners when the activity is in the background and whether that is a problem at all.

The problem described in this issue is caused by Android creating a new activity instance for every new intent by default. This problem should be fixable by setting one or both of the following intent flags for the notification.

• FLAG_ACTIVITY_CLEAR_TOP: This flag ensures that a single instance of MainActivity is at the top of the task (i.e. not multiple as described in this issue). It also ensures that all other activities on top (e.g. the settings menu) are closed so that the back button works according to @Helium314's expectation (I am not sure if I share this expectation).
• FLAG_ACTIVITY_SINGLE_TOP: This flag ensures that no new activity instance is created when it is already at the top of the task (i.e. when the map is currently being displayed) and also leads to the activity not being recreated (i.e. finished and then created again) to deliver the intent. When this flag is set, the new intent will instead be delivered to the onNewIntent function (currently not implemented) while the activity is briefly paused. This flag can also be set to be the default for all incoming intents by configuring the launchMode of the MainActivity to singleTop in AndroidManifest.xml.

The other launch modes mentioned in the comments above can also be used to fix the problem. However, they change the behavior of the app in probably undesirable ways (e.g. multiple StreetComplete tasks shown in the Android Recents screen or startActivityForResult not working correctly).

[westnordost]

Did you try Tim's suggestion(s), @Helium314 ? If it works, it could go into v53.0 due to be released tomorrow

[Helium314]

The flags are supposed to go to the intent in createSyncNotification, right?
There they don't change anything for me.

[tapetis]

It should work if you change FLAG_IMMUTABLE to FLAG_MUTABLE in the following line, otherwise the intent flags I suggested are not applied when the notification is tapped:

StreetComplete/app/src/main/java/de/westnordost/streetcomplete/data/sync/SyncNotification.kt

Line 23 in c5e5ac1

PendingIntent.getActivity(context, 0, intent, FLAG_IMMUTABLE)

But please note that FLAG_MUTABLE is only available starting with API level 31 (before that, all PendingIntent were mutable by default) and that there are some security considerations that should be taken into account when using this flag.

[Helium314]

So there is no way to avoid creating multiple instances of MainActivity on lower API levels except changing the launch mode?

[tapetis]

Prior to API 31, all pending intents are mutable by default. However, this default value is currently overwritten by explicitly setting the FLAG_IMMUTABLE flag. So for these older API levels you just have to not set this flag and for newer API levels explicitly set FLAG_MUTABLE.

[Helium314]

Thanks! Setting 0 instead if FLAG_MUTABLE helps for me.

With FLAG_ACTIVITY_SINGLE_TOP it works, but only if the main activity was last displayed. When going to settings and tapping sync notification, a new main activity is created. Back goes to settings, next back goes to the old main activity.
FLAG_ACTIVITY_CLEAR_TOP seems to behave the same in this respect.

[tapetis]

Weird, both flags work for me as described standalone and in combination on an API 29 emulator. FLAG_ACTIVITY_SINGLE_TOP is supposed to work exactly like you described. It just results in the activity not being recreated when it is already at the top. However, FLAG_ACTIVITY_CLEAR_TOP should close all activities on top.

[westnordost]

@Helium314 are you testing with SCEE or SC? Maybe there are some additional things for SCEE specifically to consider.

In any case, @tapetis , you are the one with the knowledge here, maybe you could just create a PR that changes the pending intent what it should be (from a Android UX best practice point of view)? I lost track of this a bit.

[Helium314]

I've been seeing this with both (SC only after SCEE wasn't working as expected). Maybe it's different on API 28 again? Or maybe my system doesn't behave properly here...