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

Avoid multiselect ANRs #1315

Merged
merged 2 commits into from
Aug 29, 2023
Merged

Avoid multiselect ANRs #1315

merged 2 commits into from
Aug 29, 2023

Conversation

mchowning
Copy link
Contributor

@mchowning mchowning commented Aug 28, 2023

Description

With the 7.45 release we've seen a significant increase in ANRs: #1314. This PR avoids the ANRs by addressing part of the underlying issue.

Note that this PR is targeting a new 7.45.1 hotfix branch because I think this should be a hotfix since it is causing thousands of ANRs a day. For that reason, I am keeping this PR small and lean (basically the simplest changes that address the ANRs), and I will open a separate PR that builds on this for a betafix release (#1316).

This fix is basically just to make sure that we avoid iterating through the multiselect items when we select or deselect all of the multiselect items.

Testing Instructions

  1. Make sure you have a bunch of episodes in your Up Next queue
  2. Go to a filter with a lot of episodes
  3. Enter multiselection
  4. Select all
  5. Tap the miniplayer and then swipe up to get your Up Next queue (don't close the filters screen)
  6. Enter multiselect mode
  7. Select all
  8. Select None
  9. Observe the app locks up for a while
Before After
This video just shows a freeze, not a full ANR, but I've seen these steps result in ANRs on my device multiple times
Screen.Recording.2023-08-28.at.5.12.06.PM.mov
Screen.Recording.2023-08-28.at.5.26.53.PM.mov

Checklist

  • If this is a user-facing change, I have added an entry in CHANGELOG.md
  • Ensure the linter passes (./gradlew spotlessApply to automatically apply formatting/linting)
  • I have considered whether it makes sense to add tests for my changes
  • All strings that need to be localized are in modules/services/localization/src/main/res/values/strings.xml
  • Any jetpack compose components I added or changed are covered by compose previews

@mchowning mchowning force-pushed the fix/multiselect-anr branch from 548bedc to 3d37751 Compare August 28, 2023 20:31
@mchowning mchowning mentioned this pull request Aug 28, 2023
5 tasks
@mchowning mchowning marked this pull request as ready for review August 28, 2023 21:55
@mchowning mchowning requested a review from a team as a code owner August 28, 2023 21:55
Copy link
Contributor

@ashiagr ashiagr left a comment

Choose a reason for hiding this comment

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

This is much improved. Thanks for taking a look and fixing it. 🙏

@ashiagr ashiagr merged commit 66a94b1 into release/7.45.1 Aug 29, 2023
@ashiagr ashiagr deleted the fix/multiselect-anr branch August 29, 2023 04:21
@ashiagr ashiagr added this to the 7.45 milestone Aug 29, 2023
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