Scoped Storage: Handle storage being revoked

[david-allison]

Pull Request template

Purpose / Description

If a user uninstalls and selects 'keep data', the user retains access to their preferences, so we retain deckPath and can use it to determine if their collection is no longer accessible

If a user uninstalls AnkiDroid, preserveLegacyExternalStorage no longer works.

This means they have a deckPath of /storage/emulated/0/AnkiDroid which can no longer be accessed.

Their data exists, but cannot be accessed unless:

• They restore from AnkiWeb
• They have a colpkg
• They manually copy the folder over
• They install a non-Google-Play copy

Provide a dialog with options:

• Restore from AnkiWeb
• Restore folder access
  (https://github.com/ankidroid/Anki-Android/wiki/Full-Storage-Access)
• Restore backup (⚠️ Hidden - Not Implemented)
• Get Help (Link to: Google Group)
• Delete collection and create a new one

Fixes

Related: #5304

Approach

How Has This Been Tested?

API 31 emulator, manually with the provided shell script

Checklist

• You have a descriptive commit message with a short title (first line, max 50 chars).
• You have commented your code, particularly in hard-to-understand areas
• You have performed a self-review of your own code
• UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
• UI Changes: You have tested your change using the Google Accessibility Scanner

[github-actions]

Message to maintainers, this PR contains strings changes.

1. Before merging this PR, it is best to run the "Sync Translations" GitHub action, then make and merge a PR from the i18n_sync branch to get translations cleaned out.
2. Then merge this PR, and immediately do another translation PR so the huge change made by this PR's key changes are all by themselves.

Read more about updating strings on the wiki,

• localization-administration
• download-localized-strings

[ShridharGoel]

Can you explain the rationale behind this change?

[david-allison]

If this is not called on the UI thread we get the exception:

Exception: java.lang.IllegalStateException: Must be called from main thread of fragment host

I don't recall the situation which caused this. I strongly suspect it's the line(s):

                deckPicker.launchCatchingTask {
                    CollectionHelper.ankiDroidDirectoryOverride = newAnkiDroidDirectory

                    CollectionManager.withCol {
                        deckPicker.showImportDialog(
                            ImportOptions(
                                importTextFile = false,
                                importColpkg = true,
                                importApkg = false
                            )
                        )
                    }
                }

[dae]

IMHO, you might want to revisit that code instead. withCol is intended to operate on the collection and optionally return a value - doing UI work inside it is generally a code smell. In this case it doesn't even appear to be using the provided collection scope, so as far as I can see, if this code has any effect, it would just be to serialize concurrent access if it's not already serialized. If the importing dialog needs to access the collection, it should be calling withCol itself, or the collection object should be passed in to it (but the former is probably a better option in most cases, since that minimizes the time the collection is held locked from other UI).

[mikehardy]

@david-allison not sure what you want to do with this one - this does indeed seem like a valid suggestion (move where the withCol happens / remove UI thread thing here) but it seems like it will be halt merge here. If we merge as is we are open to ANRs / UI non-responsiveness which is certainly a performance concern / could be considered a correctness concern.

I'm leaning towards merging this PR with this as a known defect peeled into a different issue. What do you think?

[mikehardy]

Concrete suggestion that would work for me then, and stop a back-and-forth: if you agree a new issue works, create the issue, link here, and then I think this is merge-able.

[mikehardy]

Okay - having napped on this (insert old man joke here...) I'm going to peel it to an issue and go with this.

[mikehardy]

#13499 is the peel out issue - I napped on this and I want to keep it moving on the main track, smaller issues should not derail progress

[BrayanDSO]

Unless I'm getting this wrong, this is trivial to solve, because if the withCol collection isn't used, it can be simply removed.

                deckPicker.launchCatchingTask {
                    CollectionHelper.ankiDroidDirectoryOverride = newAnkiDroidDirectory
                    deckPicker.showImportDialog(
                        ImportOptions(
                            importTextFile = false,
                            importColpkg = true,
                            importApkg = false
                        )
                    )
                }

And being on the "likely to bug, but trivial to fix" side of things" makes it better to fix on this PR.

[BrayanDSO]

P.s.: perhaps the launchCatchingTask can be dropped as well

[mikehardy]

Sorry @BrayanDSO - I hit the merge button before I saw this 🙈

[mikehardy]

@ShridharGoel - David responded I think to your comments and re-pushed but with one trivial refactor element missing. I just fixed that and rebased upstream/main back into the PR then pushed that. I think it is ready for review again 🤞 . Thank you!

[BrayanDSO]

Overall, LGTM. Soft approval because I haven't tested it yet