Crash when clearing app data with settings menu open

[solonovamax]

Cause of error

You can cause a crash by clearing the Nextcloud app's data with the settings menu open.

Steps to reproduce:

1. Open settings menu in Nextcloud app
2. Open android settings menu
3. Navigate to android apps menu
4. Select Nextcloud app
5. Select "Storage"
6. Select "Clear data"
7. Return to Nextcloud app. The settings menu is still open.
8. Exit out of the settings menu. Crash.

Stack Trace

Exception in thread "main" java.lang.RuntimeException
    at com.owncloud.android.ui.activities.ActivitiesActivity$$ExternalSyntheticLambda0.get(Unknown Source:2)
    at com.nextcloud.java.util.Optional.orElseThrow(Optional.java:306)
    at com.owncloud.android.ui.activity.FileDisplayActivity$8.run(FileDisplayActivity.java:2026)
    at android.os.Handler.handleCallback(Handler.java:883)
    at android.os.Handler.dispatchMessage(Handler.java:100)
    at android.os.Looper.loop(Looper.java:214)
    at android.app.ActivityThread.main(ActivityThread.java:7615)
    at java.lang.reflect.Method.invoke(Native Method)
    at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:492)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:964)

App information

• ID: com.nextcloud.client
• Version: 30220390
• Build flavor: gplay

Device information

• Brand: lge
• Device: judyln
• Model: LM-G710
• Id: QKQ1.191222.002
• Product: judyln_lao_com

Firmware

• SDK: 29
• Release: 10
• Incremental: 8450100201101

[ezaquarii]

@AlvaroBrey I don't think this is a real bug. This is never going to work.

The crash is caused by clearing the application data, which removes the user from account manager at runtime.

When application data is deleted by the operating system at runtime - the effect is pretty much impossible to predict. I think Android should shut down the application when data is cleared.

The fix would require fixing every activity. Fortunately, we do have a single point of responsibility that gives us a chance: SessionMixin.onStart(). This should give us an opportunity to shut down any activity before it tries to access deleted data. It probably won't fix all edge cases, as not all activities are using it.

TBH, this is such an odd use case that I'd wontfix it.

[AlvaroBrey]

The crash is caused by clearing the application data, which removes the user from account manager at runtime.

When application data is deleted by the operating system at runtime - the effect is pretty much impossible to predict. I think Android should shut down the application when data is cleared.

The fix would require fixing every activity. Fortunately, we do have a single point of responsibility that gives us a chance: SessionMixin.onStart(). This should give us an opportunity to shut down any activity before it tries to access deleted data. It probably won't fix all edge cases, as not all activities are using it.

TBH, this is such an odd use case that I'd wontfix it.

We have a custom "manage space" activity, we're not using the system one. I believe the point of this activity is to delete only settings and database, but not the downloaded files. The problem is that when doing this, the app is not correctly killed, so the app is still accessible via Recents. My previous idea was to kill the app after clearing data [1], but it isn't working properly.

Honestly, at this point I'd rather remove the custom "Manage space" activity and just let the system clear it properly and delete the files. I don't even think not deleting the files is a desirable behaviour: if you clear data and then log in with another account, the downloaded files from the previous setup will never be deleted and will just be there using space.
What do you think, @tobiasKaminsky ?

[1]

android/app/src/main/java/com/owncloud/android/ui/activity/ManageSpaceActivity.kt

Lines 118 to 119 in 719b0bb

	finishAndRemoveTask()
	exitProcess(0)

[tobiasKaminsky]

android/app/src/main/java/com/owncloud/android/ui/activity/ManageSpaceActivity.kt

Line 78 in e57176f

private inner class ClearDataAsyncTask : AsyncTask<Void, Void, Boolean>() {

Manage Space clease data, but remains passcode.

Deleting app cache/file via Android also kills passcode, thus we have this custome one.

[AlvaroBrey]

android/app/src/main/java/com/owncloud/android/ui/activity/ManageSpaceActivity.kt

Line 78 in e57176f

private inner class ClearDataAsyncTask : AsyncTask<Void, Void, Boolean>() {

Manage Space clease data, but remains passcode.

Deleting app cache/file via Android also kills passcode, thus we have this custome one.

But what is the reasoning behind keeping the passcode if we delete all the other data? There will be no accounts or database to look at, with the data deleted, even without passcode.

[tobiasKaminsky]

No, this does not remove account, and thus passcode must remain.
It is "only" to free up local space.

Remove account is entirely removing everything, including all settings/passcode.

[AlvaroBrey]

No, this does not remove account, and thus passcode must remain. It is "only" to free up local space.

Remove account is entirely removing everything, including all settings/passcode.

We might be talking about different things here? ManageSpaceActivity definitely deletes the entire SharedPreferences (except the lock, which it restores afterwards) and database. In fact it deletes everything under the app's private directory. And it also deletes accounts in AccountManager as well.

Also check the conversation in #9751, which is where we added the "delete accounts" step.

In short, as it's currently implemented, the ManageSpaceActivity deletes everything except the passcode, and any downloaded files that may be outside the app's private directory. This is why I am proposing to remove it, as it offers very little benefit over the default Android "clear data", and causes many more headaches.

[solonovamax]

@AlvaroBrey I don't think this is a real bug. This is never going to work.

The crash is caused by clearing the application data, which removes the user from account manager at runtime.

imo, it is a bug.
Specifically because it is never going to work.

It should return the user to the "login" screen if the account cannot be properly located/logged in to.

[solonovamax]

No, this does not remove account, and thus passcode must remain. It is "only" to free up local space.

Remove account is entirely removing everything, including all settings/passcode.

Clearing app data absolutely removes the accounts. It logs you out of all accounts, and (essentially) reverts the nextcloud app to a "clean" install. (Or at least, that's what it should do, imo)

[AlvaroBrey]

No, this does not remove account, and thus passcode must remain. It is "only" to free up local space.
Remove account is entirely removing everything, including all settings/passcode.

We might be talking about different things here? ManageSpaceActivity definitely deletes the entire SharedPreferences (except the lock, which it restores afterwards) and database. In fact it deletes everything under the app's private directory. And it also deletes accounts in AccountManager as well.

Also check the conversation in #9751, which is where we added the "delete accounts" step.

In short, as it's currently implemented, the ManageSpaceActivity deletes everything except the passcode, and any downloaded files that may be outside the app's private directory. This is why I am proposing to remove it, as it offers very little benefit over the default Android "clear data", and causes many more headaches.

Ping @tobiasKaminsky

[joshtrichards]

Fixed by #11518