Error while disabling repository during apk download

I was investigating the following ACRA crash trying to reproduce it:

ANDROID_VERSION=4.4.4
APP_VERSION_NAME=0.100-alpha6
STACK_TRACE=java.lang.NullPointerException
at org.fdroid.fdroid.net.DownloaderService.getNotificationTitle(DownloaderService.java:188)
at org.fdroid.fdroid.net.DownloaderService.createNotification(DownloaderService.java:167)
at org.fdroid.fdroid.net.DownloaderService.handleIntent(DownloaderService.java:274)
at org.fdroid.fdroid.net.DownloaderService$ServiceHandler.handleMessage(DownloaderService.java:107)
at android.os.Handler.dispatchMessage(Handler.java:102)
at android.os.Looper.loop(Looper.java:136)
at android.os.HandlerThread.run(HandlerThread.java:61)

I believe it is due to the DownloaderService trying to get the name of an app from the database in order to update a notification during progress updates. However, if a user disables a repo during an update, then that app is removed from the app table in the database. Next time the notification needs to be updated, it queries the database to get the name, but fails because the app doesn't exist.

Not sure on the best way to deal with this.

On the one hand, the symptom can be solved by changing the way the app name is fetched. Currently it is:

App app = AppProvider.Helper.findByPackageName(getContentResolver(), packageName, new String[]{AppProvider.DataColumns.NAME});
title = getString(R.string.downloading_apk, app.name);

And the app.name could be replaced with app == null ? "" : app.name.

However, that is just a bandaid. Not only will the notification be silly (showing "Downloading " instead of "Downloading F-Droid"), it wont solve the problem of what to do once downloading is complete. If there is a "Tap to install" notification being shown, but the repo containing the app has been disabled, then tapping the notification will go to F-Droid with a "App not found" toast. This is because we can no longer get to the AppDetails activity for an app which has downloaded. This is something I did not take into consideration when discussing the design of this feature unfortunately.

I feel that it would be solved when we have an InstallerService that guides the process from Installing -> Checking Cache -> Downloading -> Installing. Then, we don't need to rely on the AppDetails screen to install the app. Rather, tapping the notification can skip the middle man and go straight to the package manager. In order to do this, apk downloads should be scheduled by providing an App app, Apk apkVersionToDownload instead of a String packageName, String urlToDownload. This way, once somebody changes the repo configuration so that the app is no longer in the database, our InstallerService will still have everything required to complete the install:

• The hash of the apk
• The name of the app (for notification purposes)
• Anything else?

mentioned in merge request !274 (closed)

Once !274 (closed) is merged, this can be closed and perhaps we should create a new issue for the fact that touching "Tap to install" doesn't do anything meaningfull if the repository hosting the app has been disabled in the client.

Reassigned to @pserwylo

This is an edge case for sure, but it certainly shouldn't crash. One question to answer is whether we want "Tap to install" to go straight to installing. I guess in theory there is a security risk: any app could post a fake "Tap to install" and the user would assume it came from some place reputable. But malware could also just make a fake AppDetails screen, so there isn't much we can do to prevent that. Also, when downloading an APK with a browser, the notification becomes in effect "Tap to install" where I tested: Firefox on OmniRom and Browser on Xiaomi.

So given that, I think the "Tap to install" notification should just directly launch the install process via the upcoming InstallManagerService.

This isn't a terrible crash or regression, so removing it as a blocker for 0.100.

Milestone removed

This isn't a terrible crash or regression, so removing it as a blocker for 0.100.

Can I ask that we leave this in, given that it has a fix waiting in !274 (closed) (that at least stops F-Droid crashing, if not being a perfect fix as per my earlier comments). Specifically:

it wont solve the problem of what to do once downloading is complete

But given !274 (closed) should be merged before 0.100 anyway, may as well include this in the milestone.

FYI the crashing has been fixed for 0.100 by the inclusion of !278 (merged). The problem of the intent not working is still there, but as discussed, it is a bit of an edge case. As long as it isn't crashing I'm happy at this point, but will still leave this open until the InstallService is around and the notification knows how to go directly to the package manager rather than F-Droid.

Milestone changed to 0.101

Milestone changed to %0.102

I believe this should be fixed in master, perhaps in 0.101 due to a recent MR from @eighthave (can't see which one at a glance)

Status changed to closed