Skip to content

Don't crash at startup when an old notification channel is in use - #2247

Draft
mpretty-cyro wants to merge 1 commit into
devfrom
fix/notification-channel-migration-fgs
Draft

mpretty-cyro wants to merge 1 commit into
devfrom
fix/notification-channel-migration-fgs

Conversation

@mpretty-cyro

Copy link
Copy Markdown
Collaborator

Contributor checklist

  • I have tested my contribution on these devices:
  • Virtual device: Pixel 6 profile, API 37 (arm64), play flavour, debug
  • My contribution is fully baked and ready to be merged as is
  • I ensure that all the open issues my contribution fixes are mentioned in the commit message of my first commit using the Fixes #1234 syntax

Description

Crash cluster: SecurityException from NotificationManager.deleteNotificationChannel in NotificationChannelManager.<init>
(remote cause enforceDeletingChannelHasNoFgService). The channel migration deleted every channel and crashed if a running
foreground service was posting to one; because it's built at startup, the app crashed on launch.

It now deletes only channels that aren't current, skips any the system refuses, and leaves MIGRATED_FROM_OLD_CHANNELS unset so
those are retried on a later launch (without touching the current channels).

Testing: NotificationChannelManagerTest (Robolectric): both tests fail without the change and pass with it.

Deleting an old notification channel throws SecurityException while a
running foreground service posts to it, and NotificationChannelManager is
built at startup, so the app crashed on launch. The migration also
re-deleted every channel, including the current ones, whenever it was
retried. Delete only channels that aren't current, skip any the system
refuses, and leave the migration flag unset so those are retried on a
later launch.

@mpretty-cyro mpretty-cyro left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

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.

1 participant