Until the notifications are reworked, this is the simplest solution.
In the near future, the notifications will likely be combined into one
more intelligent notification with better defined semantics.
Installer manager cosmetic changes
This branch as some cosmetic code changes related to the `InstallManagerService`. I have a few more that I'll be throwing in here too as I go along. By and large, it is renaming, annotating, minor refactoring. The semantics should not change at all with this change, any fixes to the `InstallManagerService` will be done through separate MRs. They are from issues listed in [this comment](https://gitlab.com/fdroid/fdroidclient/merge_requests/278#note_11762414) during my CR and testing of !278.
I realise that cosmetic changes often come with baggage because we all have slightly different ideas of what Java code should look like, but in general, I will strive to only make changes if I believe strongly that they will:
* Improve understanding of the code (i.e. renaming, extracting methods)
* Prevent future errors by new contributors (e.g. @NonNull/@Nullable annotations, extracting classes to encapsulate logic which requires developers to know they need to change multiple things at once)
I try not to make changes because:
* I have a different opinion about formatting.
* Hopefully any other unreasonable or disagreeable reason.
See merge request !282
Type parameters can be ommited if defined and declared in same statement.
`onStart()` is deprecated and not required, as we target APIs > 5.
`Intent.FLAG_ACTIVITY_CLEAR_TASK` is not supported on APIs < 11 but we target 8.
WifiStateChangeService fixes
`WifiStateChangeService` had a number of issues, including being run often when there was no change, and being run twice at start-up. Also, its _complete_ broadcast was being sent twice, in effect. That made for a lot of flaky behavior for the things that rely on that information, namely wifi swap.
See merge request !289
Before, it would change fields in a final Repo instance, which means that
things could be out of sync when accessed. Now it swaps out the old one
with a new Repo instance in one step.
The local repo variables are now declared volatile so that they are more
predictable when accessed from various threads (WifiStateChangeService,
SwapService, etc.)
askServerToSwapWithUs(NewRepoConfig) was unused, so I removed it.
The IntentService provides the nice incoming Intent queue. It also runs
the Intent in a thread, so even the initial check is now in a very low
priority thread. The queuing prevents the incoming Intents from competing.
This also simplifies the code since the lifecycle is more automatic now.
The `android:process` statement in AndroidManifest.xml causes another
process to be created to run CrashReportActivity. This was causing lots of
things to be started/run twice including CleanCacheService and
WifiStateChangeService.
Lint says that only GET_META_DATA and GET_SHARED_LIBRARY_FILES are allowed.
This contradicts Android's documentation where GET_UNINSTALLED_PACKAGES
is also allowed.
Fixes#605
This was crashing when coming to SwapAppsView because some of the flow
changed related to the new DownloaderService and InstallManagerService.
Also, this lazy loading is a tiny optimization that we cannot afford right
now, there are far too many lifecycle bugs with swap.
Be more explicit about what we're running. This also means that we wont
run the "read log here" stuff if the build failed, which didn't make any
sense. That should only be run if the unit tests fail.
Advantages:
* Failing unit tests don't keep the android tests from running
* CI should be overall faster as the tasks get run in parallel and the
former `gradle` task was the longest by far
This will later simplify the multiplexing of the android tests onto
multiple emulators.
Enable a few more PMD rules
I did one last pass over https://pmd.github.io/pmd-5.4.1/pmd-java/rules/index.html and these are the only sane rules left that I could find.
So I'm pretty much done with adding PMD stuff :) Don't know if anyone feels strongly in favour of adding findbugs, but I think checkstyle+PMD is good for now.
See merge request !283
Enable PMD java-basic
This is the fixes necessary to enable PMD's `java-basic` ruleset. I think there will be a few rules in there that will largely be annoying, so we'll need to ultimately decide whether to use `// NOPMD` or just specify the rules we want from `java-basic`.
See merge request !280
This should fix the PMD error:
"Check the value returned by the skip() method of an InputStream to see if
the requested number of bytes has been skipped."
Installer manager fixes
Builds on the recently merged `InstallManagerService` to fix a few minor UX bugs:
* Cancellation of pending downloads now removes notifications.
* No longer shows "Downloading Downloading {AppName}" in notification, just "Downloading {AppName}"
* If cached file is same size but corrupted, remove it and then continue with the process of downloading + installing
See merge request !281
The check was set up to only cancel when the `AppDetails` for that app
was shown. This is the correct behaviour for the 'complete' event, but
not the cancel. The cancel event should always result in the relevant
notification being removed.
If we are capable of bailing earlier rather than later, then we should. This way,
if a hash doesn't match, the file will be removed and a new download will begin,
as expected. The alternative is to let the installer catch the unmatching hashes.
By then though, it is too late to really do anything meaningfull and it becomes
more difficult to recover in a way that the user would expect.
Due to the earlier refactoring of `getNotificationTitle()` (or something like that)
to `getAppName()`, it was still returning `getString(downloading_apk, appName)` instead
of just the app name.
InstallManagerService
This provides an over-arching `Service` for managing the whole install process, from checking the cache, downloading files, handling the notification. Ultimately, it should probably also handle starting and tracking progress of the final installation steps.
Note: this does undo some of the `Notification` handling stuff, putting it back to one notification per APK. I did that to get that part working OK for the short term, giving us time to figure out what the whole picture should look like. I think @pserwylo has it pretty well sketched out in #592. But I have no strong feelings about the notification stuff for 0.100, so I'm happy to shape this MR accordingly, provided its only a little work.
See merge request !278
I wrestled with this a bunch, it seems quite difficult to make the Cancel
button on the notification responsive. This collection of minor changes
made it more reliable, but its still kind of flaky. I think the problem
might be related to the fact that it is creating a whole new Notification
instance, with the accompanying Intent and PendingIntent instances, for
every single download progress update.
closes#652https://gitlab.com/fdroid/fdroidclient/issues/652
If an APK was queued to download but had not started downloading yet, it
was not able to be fully canceled because ACTION_INTERRUPTED was not sent.
That meant that the UI never got updated, even though the APK was removed
from the queue.
#652https://gitlab.com/fdroid/fdroidclient/issues/652
This logic is pretty basic for now, it'll have to be expanded a lot to
support the different UX between priv and non-priv installs. For example,
priv updates will be able to happen entirely in the background. Those will
then require leaving a notification to tell the user that the app was
updated so nothing can transparently install updates without the user
knowing. When the user is an active part of each install, like the
non-priv experience requires, then keeping the "app installed" notification
feels like just extra noise.
