Android: Improve WRITE_EXTERNAL_STORAGE denial

[Ebola16]

Now just a cleanup PR. The original intention of this PR has been split into the mentioned PRs.

[JosJuice]

We should never ask the user to restart the app. Android doesn't make it clear how to restart an app, or whether a certain set of actions constitutes restarting an app or not (is going back to the home screen and launching the app again a restart, for instance?)

You should be able solve the problem of needing to call setPlatformTabsAndStartGameFileCacheService by checking whether the permission has been granted when running MainActivity.onResume and calling directory initialization and setPlatformTabsAndStartGameFileCacheService then. This should also let you revert the changes to the file picker.

activity.shouldShowRequestPermissionRationale(WRITE_EXTERNAL_STORAGE) calls the same code regardless of what that evaluates to but the OS handles it differently by adding a "Do not show again" option. I ended up leaving the duplicate code in since removing it always displays the "Do not show again" option.

You should be able to remove the duplicate code by calling shouldShowRequestPermissionRationale and ignoring the result. And either way, please add a comment to the code about the reason you're doing this, because it isn't obvious at all otherwise.

[Ebola16]

You should be able solve the problem of needing to call setPlatformTabsAndStartGameFileCacheService by checking whether the permission has been granted when running MainActivity.onResume and calling directory initialization and setPlatformTabsAndStartGameFileCacheService then. This should also let you revert the changes to the file picker.

If the permission is granted within FilePicker, then directory initialization won't occur until MainActivity.onResume is called. That will cause "Open File" to seg fault upon confirming file selection. I think the FilePicker change unfortunately needs to stay.

We should never ask the user to restart the app. Android doesn't make it clear how to restart an app, or whether a certain set of actions constitutes restarting an app or not (is going back to the home screen and launching the app again a restart, for instance?)

I suppose users will figure out that the only way to get the permission prompt again is to force close and restart the app. I tried adding PermissionsHandler.checkWritePermission to MainActivity.onResume but that resulted in a spam of permission prompts.

You should be able to remove the duplicate code by calling shouldShowRequestPermissionRationale and ignoring the result. And either way, please add a comment to the code about the reason you're doing this, because it isn't obvious at all otherwise.

Done

[Ebola16]

It sounds like #9062 should solve some of these problems but I can't test it right now. I'll look into this again later.

[mbc07]

I think a long time goal should be dropping WRITE_EXTERNAL_STORAGE permission altogether. AFAICT, Dolphin only requests this permission to be able to write to Internal Storage\dolphin-emu, so it won't be required anymore once Dolphin starts storing the user profile on the "proper" external app data directory (Internal Storage\Android\data\org.dolphinemu.dolphinemu). Given this transition is somewhat a blocker to properly implement scoped storage support, sooner or later we'll have to do this...

[JosJuice]

@Ebola16 PR #9062 doesn't fix anything in itself, but it does make it very easy to gate Open File behind a permission check + toast (just like I did in PR #8962 with Import WAD). However, I will try to look into making Open File work properly when permissions are granted in the file picker. I'll handle it in a separate PR. (The actual place where it crashes was a little hard to track down, but it turned out to the the call to GameFile.getTitle in EmulationActivity.launchFile, which makes sense.)

@mbc07 Dolphin currently also needs WRITE_EXTERNAL_STORAGE for opening any game file. But that too has to be changed in order to comply with scoped storage, so yes, eventually we will be able to run Dolphin without needing that permission at all. However, we might want to still request it on older versions of Android in case we can't get certain features working with scoped storage. Being able to configure the NAND path is the main one I'm thinking of.

[Ebola16]

Ok I'll mark this as a draft until your Open File PR is merged. And I agree with @JosJuice's response to @mbc07.

[JosJuice]

These three PRs combined should fix it: #9077 #9078 #9079

[Ebola16]

Now just a cleanup PR. The original intention of this PR has been split into the mentioned PRs. Ready for review.

[JosJuice]

I have no particular opinion about the changes left in this PR, but none of the changes are anything that I would oppose merging.