Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix notification open not always resuming app #1277

Merged
merged 1 commit into from
Feb 5, 2021

Conversation

jkasten2
Copy link
Member

@jkasten2 jkasten2 commented Feb 4, 2021

Scope

Fix

This fix prevents notification opens affecting the Activity backstack.
Fixes issue where if the app's main Activity was not the last Activity opened tapping on a notification would create another instance of it. Basically this behavior fix aligns with everything an Android Launcher would do.

Solutions

Solution D below was used but all options that were attempted / considered.

A. Android API

Tired searching for a built in Android API that directly resumes the app. This doesn't exist and wasn't expecting it to but looked for completeness.

B. startActivity Intent.FLAG_ACTIVITY_* Flags

I read though the description and tried a few different Intent.FLAG_ACTIVITY_* flags to our startActivity in our startOrResumeApp to get the app to resume without affecting the backstack. However FLAG_ACTIVITY_SINGLE_TOP, FLAG_ACTIVITY_BROUGHT_TO_FRONT, and some others I tried did not have the resume affected in all cases or not at all.

C. Show non-visible Activity

As a "workaround" it is possible to show an non-visible Activity and call finish() right away to bring the app forward to then to put the app back to the last Activity. This would require adding another Activity and it would create extra onResume and onPause events would be mostly acceptable, but could results in unintended side effects.
This something I confirmed as a valid option, but was also discovered by others.

D. Omit package from Intent passed to startActivity

Using Intent.setPackage(null) seems to have a side effect of always resuming the app instead of effecting the backstace. I wasn't able to find any official Google documentation on this effect but it does seems to be what Android Launchers depend on. I confirmed this by checking the source of an open source launcher, OpenLauncherTeam/openlauncher and confirmed package would be null on their Intent.
This solution was found here:
https://stackoverflow.com/a/42002705/1244574

I also confirmed that getLaunchIntentForPackage in ASOP generates an Intent that uses Intent.setClassName that includes both the package name & Activity name so there is no risk in a different app being started if it happened to have the same fully namespaced Activity.

Based on testing on Android 11 (API 30) (both an emu and Samsung device) using setPackage(null) isn't required, the resume behavior works with or without it. I wasn't able to find any documentation on this behavior change so it is unknown if it was intentional.

Tests

Automated

Robolectric does not have an API to get the backstack write an integration test for this. The closest thing I could find is getNextStartedActivity which would only tell me the call order but none of the behavior changes.

Manual

App w/ splash and single Activity (the example app in this repo)

  1. In focus notifi open, no flicker, back button works
  2. Notif open from cold start, back button works.
  3. Notif open after backgrounding app, app resumed correctly, back button works

App w/ two Activities, Main Activity A starts B (the example app in this repo but makeRestartActivityTask in example removed)

  1. In focus notifi open, no flicker, back button works
  2. Notif open after backgrounding app, app resumed Activity B correctly, back button works
  3. Notif open from cold start, kicked out of ram, app resumes to Activity B, back button works
  4. Notif open from cold start, app swiped away, app opens Activity A, back button works

Tested on Emulators - Android 11 & 10.
Tested on OnePlus 6T - Android 10
Tested on LG G7 - Android 9

This change is Reviewable

@jkasten2 jkasten2 force-pushed the fix/notif_open_cold_start_launch_activity branch from 7905deb to 14c340f Compare February 5, 2021 07:36
@jkasten2 jkasten2 changed the base branch from fix/notif_open_cold_start_launch_activity to master February 5, 2021 07:39
* This fix prevents notification opens affecting the Activity backstack
* Fixes issue where if the app's main Activity was not the last Activity
opened tapping on a notification would create another instance of it. 
  - Basically this behavior fix aligns with everything an
  Android Launcher would do.
* This was the original intended behavior when this SDK switched from a
BroadcastReceiver PendingIntent to an Activity PendingIntent when
generating a notification.
@jkasten2
Copy link
Member Author

jkasten2 commented Feb 5, 2021

Ran the following after #1276 (comment) was done to clean up fixups

git rebase --onto fix/notif_open_cold_start_launch_activity fix/notif_open_resume_app~1 fix/notif_open_resume_app
git push -f

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants