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 potential ANR when saving form #6134

Merged
merged 4 commits into from
May 16, 2024
Merged

Fix potential ANR when saving form #6134

merged 4 commits into from
May 16, 2024

Conversation

seadowg
Copy link
Member

@seadowg seadowg commented May 10, 2024

Work towards #5838

This removes UI thread DB access when showing a form saved snackbar.

Why is this the best possible solution? Were any other approaches considered?

Not a lot to discuss here! I've moved the DB access to a background thread which results in switching a single getX method for a setX and a Consumable<LiveData> event.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

Only thing affected here is the form saved snack bar, so some testing around that is all that is needed I'd imagine. Reproducing the ANR won't be possible (as always), so it's best just to check that nothing looks wrong with the existing feature set.

Before submitting this PR, please make sure you have:

  • added or modified tests for any new or changed behavior
  • run ./gradlew connectedAndroidTest (or ./gradlew testLab) and confirmed all checks still pass
  • added a comment above any new strings describing it for translators
  • verified that any code or assets from external sources are properly credited in comments and/or in the about file.
  • verified that any new UI elements use theme colors. UI Components Style guidelines

@seadowg seadowg marked this pull request as ready for review May 10, 2024 15:14
@seadowg seadowg requested a review from grzesiek2010 May 10, 2024 15:15
@dbemke
Copy link

dbemke commented May 16, 2024

@seadowg We don't have access to the firebase. Could you give us a hint how to try to reproduce it or what was it connected with? Was it on a specific Android version or a specific device?

@seadowg seadowg merged commit 4fa233e into getodk:master May 16, 2024
6 checks passed
@seadowg seadowg deleted the forms-anr branch May 16, 2024 09:00
@seadowg
Copy link
Member Author

seadowg commented May 16, 2024

@dbemke sorry the testing notes were bad there. I've updated them. As a standing rule: PRs that fix ANR won't require checking that it no longer happens as they can't be reliably reproduced. We use Android tooling (StrictMode) to ensure they aren't possible any longer. The risk will usually be that a bug is introduced while fixing the ANR.

@dbemke
Copy link

dbemke commented May 17, 2024

Tested with Success!

Verified on a device with Android 10

Verified Cases:

  • snackbar after saving a form

@WKobus
Copy link

WKobus commented May 17, 2024

Tested with Success!

Verified on a device with Android 14

@srujner
Copy link

srujner commented May 17, 2024

Tested with Success!

Verified on a device with Android 13

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

Successfully merging this pull request may close these issues.

5 participants