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

Multiselect causing ANRs #1314

Closed
mchowning opened this issue Aug 28, 2023 · 2 comments
Closed

Multiselect causing ANRs #1314

mchowning opened this issue Aug 28, 2023 · 2 comments
Labels
[Type] Bug Not functioning as intended.

Comments

@mchowning
Copy link
Contributor

mchowning commented Aug 28, 2023

Description

With the 7.45 release, we've seen a significant increase in ANRs. They mostly seem related to multi-selection.

For example:

Step-by-step reproduction 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

Device, Operating system, and Pocket Casts app version

Samsung A13, Android 13, Pocket Casts 7.45

@mchowning mchowning added the [Type] Bug Not functioning as intended. label Aug 28, 2023
This was referenced Aug 28, 2023
@mchowning
Copy link
Contributor Author

mchowning commented Aug 28, 2023

I think there are two issues causing this.

1. Not batching multiselect updates

The main one is that there are a number of places in the app where we iterate through the multiselect items when a user does select all/none, instead of just replacing the selected items with a single, new, list. Fixing this seems pretty straightforward and seems to avoid the ANR issue on its own, so I've created a PR for this as a hotfix (#1315).

2. Storing multiselect state as global state

The second cause has to do with how we are using singletons for the multiselect helper classes. This means that different screens are sharing the same multiselect helpers. One issue with this is that many of the classes are listening, and responding, to changes on these helper classes. This means that screens that are not being presented to the user are responding to multi-select actions the user does on other screens.

I don't see that we need to be using singletons for these (and making them not be singletons avoids the ANR issue even without the other fix). In addition, this fixes the problem of having the multiselect state on one screen effect the multiselect state on another screen (i.e., entering multiselect mode and selecting some episodes effects the behavior on subsequent screens where you enter multi-select mode).

Because this fix seems riskier than the first one, I'm not making it a hotfix, but because it fixes a few other multiselect bugs that seem kinda rough, I'm targeting it as a betafix (#1316). Just let me know if you think a different approach makes more sense.

@mchowning
Copy link
Contributor Author

Closing this because the ANRs were addressed by #1378 and the global state has been removed by #1579 and #1580.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Bug Not functioning as intended.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant