Skip to content

Fix offline areas crash#3660

Open
andreia-ferreira wants to merge 7 commits intomasterfrom
andreia/3407/fix-offline-area-crash
Open

Fix offline areas crash#3660
andreia-ferreira wants to merge 7 commits intomasterfrom
andreia/3407/fix-offline-area-crash

Conversation

@andreia-ferreira
Copy link
Copy Markdown
Collaborator

Fixes #3407

When downloading 2 offline areas in a row, the app would crash. This is because the OfflineAreaSelectorViewModel was annotated with @SharedViewModel, which meant that the navigate SharedFlow outlived the fragment and, when a new OfflineAreaSelectorFragment was created, it reuses the previous VM. Then, on emiting a navigate event, it was collected twice: the current fragment and the one in the backstack - the one which crashed, because the navigation was already on the home screen.

This PR removes the @SharedViewModel usage from that VM, since it's only used by a single fragment anyway. Additionally, the flow collectors in the fragment were refactored to prevent memory leaks (using the viewlifecycle owner to launch coroutines and repeatOnLifecycle(Lifecycle.State.STARTED))

@shobhitagarwal1612 PTAL?

@auto-assign auto-assign bot requested a review from shobhitagarwal1612 April 8, 2026 15:17
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 8, 2026

Codecov Report

❌ Patch coverage is 67.04545% with 29 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.07%. Comparing base (9868ea3) to head (f7a3bbf).

Files with missing lines Patch % Lines
...flineareas/selector/OfflineAreaSelectorFragment.kt 30.00% 20 Missing and 8 partials ⚠️
...lineareas/selector/OfflineAreaSelectorViewModel.kt 96.87% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3660      +/-   ##
============================================
+ Coverage     66.80%   67.07%   +0.26%     
- Complexity     1608     1611       +3     
============================================
  Files           363      363              
  Lines          8980     8983       +3     
  Branches       1159     1162       +3     
============================================
+ Hits           5999     6025      +26     
+ Misses         2332     2306      -26     
- Partials        649      652       +3     
Files with missing lines Coverage Δ
...neareas/selector/model/OfflineAreaSelectorEvent.kt 100.00% <100.00%> (ø)
...neareas/selector/model/OfflineAreaSelectorState.kt 100.00% <100.00%> (ø)
...lineareas/selector/OfflineAreaSelectorViewModel.kt 88.42% <96.87%> (+23.52%) ⬆️
...flineareas/selector/OfflineAreaSelectorFragment.kt 45.71% <30.00%> (-4.94%) ⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Member

@shobhitagarwal1612 shobhitagarwal1612 left a comment

Choose a reason for hiding this comment

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

Can we combine all separate mutable states to a single UiState?

@andreia-ferreira
Copy link
Copy Markdown
Collaborator Author

@shobhitagarwal1612 fully agree! I've refactored the VM to unify the overall state under OfflineAreaSelectorState and one time events with OfflineAreaSelectorEvent. Ready for another look

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.

Crash when downloading offline imagery

2 participants