New DownloaderService
This merge request is too big, but I have all this work complete, so I'm submitting it now for review. If there are bigger issues with parts, I can rebase it down to the uncontroversial bits to get things merged.
This replaces ApkDownloader, AsyncDownload, and AsyncDownloadWrapper, and puts the whole APK download procedure into a custom Service that is based on the source code for IntentService. It can't be a subclass of IntentService because it needs to be cancelable. This does not yet add back a notification for the downloading, e.g. #592 (closed). That was handled before by the Android DownloadManager stuff, and was not replaced since that was disabled.
My current implementation does not filter out duplicate requests like discussed in #601 (closed). While its possible to do, I think it'll complicate the code a lot, and I really think that should be handled elsewhere. The UI should prevent the possibility of the user being able to submit duplicate install requests. If not, even if DownloaderService filtered them out, it would still be a buggy UX since the user would be clicking install again or something like that even though the install was in progress.
This also moves the APK verification logic to the Installer side. The downloading side can check the file size to see if the whole thing is downloaded. And to be extra safe, it should not be possible to submit an APK for installation without it going through the verification procedure. So the only method for installing APKs, Installer.install(), is where the verification now happens. Also, the installer now always copies the APK to be installed into the safe location RE: the Cure53 audit issue. This way, the APK download and cache dirs can be merged into one, making resumable downloads and cache management easy.
ping @mvdan @pserwylo @dschuermann more comments in the commit messages
Merge request reports
Activity
mentioned in merge request !250 (merged)
Added 25 commits:
- c86af9d5...09324bbb - 15 commits from branch
fdroid:master - 5d8353d8 - use custom proguard config for running tests
- ab7c21a6 - use IntentService style to download all APKs via a queue
- 6764dc48 - eliminate Interfaces used only internal to AppDetails class
- 399ff4df - port everything over to new Downloader.ACTION_PROGRESS
- 8f203045 - send Downloader progress on a 100 ms timer
- 94fabb97 - keep the core Downloader classes pure Java for easy testing
- 9da5781c - purge unused code from Installer classes
- 3b465cd6 - add AndroidManifest parser for reading directly from APKs
- 91cd7c43 - convert Installer's Exception into generic InstallFailedException
- b2039dad - download APKs into dir named after repo's hostname
Toggle commit list- c86af9d5...09324bbb - 15 commits from branch
mentioned in merge request !253 (merged)
Added 18 commits:
- b2039dad...d846e18d - 10 commits from branch
fdroid:master - 1343bd70 - use IntentService style to download all APKs via a queue
- 98b4daa5 - eliminate Interfaces used only internal to AppDetails class
- 284aa347 - port everything over to new Downloader.ACTION_PROGRESS
- 64192dae - send Downloader progress on a 100 ms timer
- 440cbeb5 - add AndroidManifest parser for reading directly from APKs
- 51797333 - convert Installer's Exception into generic InstallFailedException
- 3189d1f9 - verify APKs on install, and purge ApkDownloader
- 8998b0fb - download APKs into dir named after repo's hostname
Toggle commit list- b2039dad...d846e18d - 10 commits from branch
Reassigned to @pserwylo
Ok, this is still big, but I think I've covered basically everything in #601 (closed), and its working for me, so I thought I'd just leave it all here to be reviewed as one thing. It can be still split up if need be.
Added 23 commits:
- cdc14071...907507d2 - 2 commits from branch
fdroid:master - 87104b94 - use IntentService style to download all APKs via a queue
- 354fa10c - eliminate Interfaces used only internal to AppDetails class
- 075f3ad1 - port everything over to new Downloader.ACTION_PROGRESS
- 977ba6c7 - send Downloader progress on a 100 ms timer
- 5c3709a3 - add AndroidManifest parser for reading directly from APKs
- b13b57c4 - convert Installer's Exception into generic InstallFailedException
- 0ee354e5 - verify APKs on install, and purge ApkDownloader
- 93124af6 - download APKs into dir named after repo's hostname
- e79e0224 - HttpDownloader: handle SSL errors like any other download error
- d849e679 - implement resumable APK downloads for HttpDownloader
- 639dd41d - show download errors in a Toast
- 2bb066c0 - add method to check if a URL is being handled by DownloaderService
- 9b60517b - put stray preference handling into Preferences
- 6fa5be3b - add basic in progress Notification to DownloaderService
- 5fce3204 - add "Auto Download" pref to enable auto-downloading of updates
- a4cb4885 - port all but Provider tests to JUnit4 semantics
- 00dc8325 - HttpDownloaderTest: delete test files only if test succeeds
- aada06f1 - put up a notification for each completed download
- 2278b8fd - run UpdateService at lowest priority possible
- 9e48614a - DO NOT MERGE dev gradle setup
- 5097d86c - DO NOT MERGE update to latest gradle plugin
Toggle commit list- cdc14071...907507d2 - 2 commits from branch
- Last reply by username-removed-24982
142 msg.arg1 = startId; 143 msg.obj = intent; 144 msg.what = what++; 145 serviceHandler.sendMessage(msg); 146 Log.i(TAG, "QUEUE_WHATS.put(" + uriString + ", " + msg.what); 147 QUEUE_WHATS.put(uriString, msg.what); 148 } else { 149 Log.e(TAG, "Received Intent with unknown action: " + intent); 150 } 151 } 152 153 private void createNotification(String urlString) { 154 NotificationCompat.Builder builder = 155 new NotificationCompat.Builder(this) 156 .setAutoCancel(true) 157 .setContentTitle(getString(R.string.downloading)) Add a pending intent here so user can go back to the app detail page while the download is in progress (clicks download in progress notification and goes back to app page) , In case he wants to cancel it.
Edited by username-removed-273687
Using
App Name + Versionwill be more user friendly compared to usinghttps://f-droid.org/packagename/org/.../for notifications in progress.Edited by username-removed-273687-
The "ready to install" notifications are very rudamentary right now, just functional placeholders really. We should redesign all of the notifications together, in a separate merge request, since they'll all need to change based on the work here, as well as the new work on the privileged installer and pushing the index updating further into the background.
mentioned in merge request !255 (merged)
Added 23 commits:
- 2278b8fd...5fd591bb - 8 commits from branch
fdroid:master - 33901eda - use IntentService style to download all APKs via a queue
- 8bb6e802 - eliminate Interfaces used only internal to AppDetails class
- 79ceb92e - port everything over to new Downloader.ACTION_PROGRESS
- cde0ccf0 - send Downloader progress on a 100 ms timer
- ab03ba25 - add AndroidManifest parser for reading directly from APKs
- 33e0a1d1 - convert Installer's Exception into generic InstallFailedException
- 2b41bb1e - verify APKs on install, and purge ApkDownloader
- 289c1e0e - download APKs into dir named after repo's hostname
- 7f043eaa - implement resumable APK downloads for HttpDownloader
- 84bb606b - show download errors in a Toast
- ab998b25 - add method to check if a URL is being handled by DownloaderService
- dab3f449 - put stray preference handling into Preferences
- 8af585f5 - add basic in progress Notification to DownloaderService
- cb026a4d - add "Auto Download" pref to enable auto-downloading of updates
- 65c1ac39 - put up a notification for each completed download
Toggle commit list- 2278b8fd...5fd591bb - 8 commits from branch
Added 15 commits:
- aef179d9 - use IntentService style to download all APKs via a queue
- 72ae0378 - eliminate Interfaces used only internal to AppDetails class
- c17426cb - port everything over to new Downloader.ACTION_PROGRESS
- 9a51d6fd - send Downloader progress on a 100 ms timer
- efb883d5 - add AndroidManifest parser for reading directly from APKs
- 0d19d730 - convert Installer's Exception into generic InstallFailedException
- cf8abcd2 - verify APKs on install, and purge ApkDownloader
- 823075d1 - download APKs into dir named after repo's hostname
- 19c90156 - implement resumable APK downloads for HttpDownloader
- 4849b914 - show download errors in a Toast
- fb813d7c - add method to check if a URL is being handled by DownloaderService
- 7d6f342e - put stray preference handling into Preferences
- f83167fa - add basic in progress Notification to DownloaderService
- 4c5f5d33 - add "Auto Download" pref to enable auto-downloading of updates
- 8d6506fc - put up a notification for each completed download
Toggle commit listLGTM @eighthave , all that's left is @pserwylo to take a look.
Added 18 commits:
- 8d6506fc...d98d59a8 - 3 commits from branch
fdroid:master - 20c66a82 - use IntentService style to download all APKs via a queue
- 2e3d2c84 - eliminate Interfaces used only internal to AppDetails class
- 5e59f812 - port everything over to new Downloader.ACTION_PROGRESS
- fc9459d6 - send Downloader progress on a 100 ms timer
- 0b8796e5 - add AndroidManifest parser for reading directly from APKs
- d07c8b5d - convert Installer's Exception into generic InstallFailedException
- ee37f26c - verify APKs on install, and purge ApkDownloader
- fa766180 - download APKs into dir named after repo's hostname
- 3265c863 - implement resumable APK downloads for HttpDownloader
- 49635c22 - show download errors in a Toast
- 721d4a30 - add method to check if a URL is being handled by DownloaderService
- 77e041d6 - put stray preference handling into Preferences
- adcdc417 - add basic in progress Notification to DownloaderService
- d114f428 - add "Auto Download" pref to enable auto-downloading of updates
- 74d1c952 - put up a notification for each completed download
Toggle commit list- 8d6506fc...d98d59a8 - 3 commits from branch
-
@dschuermann this does touch the installer stuff enough so I'd like you to have a look at it. I just rebased it on top your !256 (merged)
@eighthave review done, I have no problem with moving the hash verification inside the Installer class. Looks good!
mentioned in issue #103 (closed)
mentioned in issue #94
