From 3785aecc2672f2a96759d76ba7d3480e27b54130 Mon Sep 17 00:00:00 2001
From: Hans-Christoph Steiner <hans@eds.org>
Date: Tue, 24 Apr 2018 13:41:37 +0200
Subject: [PATCH 01/11] UpdateService: use data URI rather than custom extra

Just proper Android style.
---
 app/src/main/java/org/fdroid/fdroid/UpdateService.java | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/src/main/java/org/fdroid/fdroid/UpdateService.java b/app/src/main/java/org/fdroid/fdroid/UpdateService.java
index 563bfd405..2b06fb2b6 100644
--- a/app/src/main/java/org/fdroid/fdroid/UpdateService.java
+++ b/app/src/main/java/org/fdroid/fdroid/UpdateService.java
@@ -30,6 +30,7 @@ import android.content.Context;
 import android.content.Intent;
 import android.content.IntentFilter;
 import android.content.SharedPreferences;
+import android.net.Uri;
 import android.os.AsyncTask;
 import android.os.Build;
 import android.os.Handler;
@@ -68,7 +69,6 @@ public class UpdateService extends IntentService {
     public static final String EXTRA_MESSAGE = "msg";
     public static final String EXTRA_REPO_ERRORS = "repoErrors";
     public static final String EXTRA_STATUS_CODE = "status";
-    public static final String EXTRA_ADDRESS = "address";
     public static final String EXTRA_MANUAL_UPDATE = "manualUpdate";
     public static final String EXTRA_FORCED_UPDATE = "forcedUpdate";
     public static final String EXTRA_PROGRESS = "progress";
@@ -104,7 +104,7 @@ public class UpdateService extends IntentService {
         Intent intent = new Intent(context, UpdateService.class);
         intent.putExtra(EXTRA_MANUAL_UPDATE, true);
         if (!TextUtils.isEmpty(address)) {
-            intent.putExtra(EXTRA_ADDRESS, address);
+            intent.setData(Uri.parse(address));
         }
         context.startService(intent);
     }
@@ -376,7 +376,7 @@ public class UpdateService extends IntentService {
         boolean forcedUpdate = false;
         String address = null;
         if (intent != null) {
-            address = intent.getStringExtra(EXTRA_ADDRESS); // TODO switch to Intent.setData()
+            address = intent.getDataString();
             manualUpdate = intent.getBooleanExtra(EXTRA_MANUAL_UPDATE, false);
             forcedUpdate = intent.getBooleanExtra(EXTRA_FORCED_UPDATE, false);
         }

From fe260d931c7e005d98dd42f631fd535eba00e771 Mon Sep 17 00:00:00 2001
From: Hans-Christoph Steiner <hans@eds.org>
Date: Tue, 24 Apr 2018 16:53:05 +0200
Subject: [PATCH 02/11] use JobIntentService for CleanCache, DeleteCache,
 Installer, InstalledApp

This should ensure that these jobs complete before sleep, and also, they
should work more reliably with the new Android 8.0 background restrictions.
https://developer.android.com/reference/android/support/v4/app/JobIntentService.html #1426
---
 app/src/main/AndroidManifest.xml              |  5 +++
 .../org/fdroid/fdroid/CleanCacheService.java  | 17 ++++-----
 .../org/fdroid/fdroid/DeleteCacheService.java | 18 ++++------
 .../data/InstalledAppProviderService.java     | 24 ++++++-------
 .../fdroid/installer/InstallerService.java    | 35 +++++++++----------
 5 files changed, 44 insertions(+), 55 deletions(-)

diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml
index b6b1fdaff..0bf7d64ca 100644
--- a/app/src/main/AndroidManifest.xml
+++ b/app/src/main/AndroidManifest.xml
@@ -46,6 +46,7 @@
     <uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE"/>
     <uses-permission android:name="android.permission.WRITE_SETTINGS"/>
     <uses-permission android:name="android.permission.NFC"/>
+    <uses-permission android:name="android.permission.WAKE_LOCK"/>
 
     <!-- Indicate that F-Droid may request root access (introduced by Koush's Superuser app)
          This permission is deprecated, but necessary for some old superuser
@@ -261,12 +262,15 @@
                 android:exported="false"/>
         <service
                 android:name=".installer.InstallerService"
+                android:permission="android.permission.BIND_JOB_SERVICE"
                 android:exported="false"/>
         <service
                 android:name=".CleanCacheService"
+                android:permission="android.permission.BIND_JOB_SERVICE"
                 android:exported="false"/>
         <service
                 android:name=".DeleteCacheService"
+                android:permission="android.permission.BIND_JOB_SERVICE"
                 android:exported="false"/>
         <service
                 android:name=".net.WifiStateChangeService"
@@ -286,6 +290,7 @@
                 android:exported="false"/>
         <service
                 android:name=".data.InstalledAppProviderService"
+                android:permission="android.permission.BIND_JOB_SERVICE"
                 android:exported="false"/>
         <service
                 android:name=".AppUpdateStatusService"
diff --git a/app/src/main/java/org/fdroid/fdroid/CleanCacheService.java b/app/src/main/java/org/fdroid/fdroid/CleanCacheService.java
index da887eb00..8833b983f 100644
--- a/app/src/main/java/org/fdroid/fdroid/CleanCacheService.java
+++ b/app/src/main/java/org/fdroid/fdroid/CleanCacheService.java
@@ -1,13 +1,14 @@
 package org.fdroid.fdroid;
 
 import android.app.AlarmManager;
-import android.app.IntentService;
 import android.app.PendingIntent;
 import android.content.Context;
 import android.content.Intent;
 import android.os.Build;
 import android.os.Process;
 import android.os.SystemClock;
+import android.support.annotation.NonNull;
+import android.support.v4.app.JobIntentService;
 import org.apache.commons.io.FileUtils;
 import org.fdroid.fdroid.installer.ApkCache;
 
@@ -27,7 +28,7 @@ import java.util.concurrent.TimeUnit;
  * and newer.  On older Android, last modified time from {@link File#lastModified()}
  * is used.
  */
-public class CleanCacheService extends IntentService {
+public class CleanCacheService extends JobIntentService {
 
     /**
      * Schedule or cancel this service to update the app index, according to the
@@ -35,6 +36,7 @@ public class CleanCacheService extends IntentService {
      * is changed, or c) on startup, in case we get upgraded.
      */
     public static void schedule(Context context) {
+        // TODO use JobScheduler
         long keepTime = Preferences.get().getKeepCacheTime();
         long interval = TimeUnit.DAYS.toMillis(1);
         if (keepTime < interval) {
@@ -51,18 +53,11 @@ public class CleanCacheService extends IntentService {
     }
 
     public static void start(Context context) {
-        context.startService(new Intent(context, CleanCacheService.class));
-    }
-
-    public CleanCacheService() {
-        super("CleanCacheService");
+        enqueueWork(context, CleanCacheService.class, 0x982374, new Intent(context, CleanCacheService.class));
     }
 
     @Override
-    protected void onHandleIntent(Intent intent) {
-        if (intent == null) {
-            return;
-        }
+    protected void onHandleWork(@NonNull Intent intent) {
         Process.setThreadPriority(Process.THREAD_PRIORITY_LOWEST);
         deleteExpiredApksFromCache();
         deleteStrayIndexFiles();
diff --git a/app/src/main/java/org/fdroid/fdroid/DeleteCacheService.java b/app/src/main/java/org/fdroid/fdroid/DeleteCacheService.java
index 8832d1b1a..b526cc2ed 100644
--- a/app/src/main/java/org/fdroid/fdroid/DeleteCacheService.java
+++ b/app/src/main/java/org/fdroid/fdroid/DeleteCacheService.java
@@ -1,9 +1,10 @@
 package org.fdroid.fdroid;
 
-import android.app.IntentService;
 import android.content.Context;
 import android.content.Intent;
 import android.os.Process;
+import android.support.annotation.NonNull;
+import android.support.v4.app.JobIntentService;
 import android.support.v4.content.ContextCompat;
 import android.util.Log;
 import org.apache.commons.io.FileUtils;
@@ -11,25 +12,18 @@ import org.apache.commons.io.FileUtils;
 import java.io.File;
 
 /**
- * An {@link IntentService} subclass for deleting the full cache for this app.
+ * An {@link JobIntentService} subclass for deleting the full cache for this app.
  */
-public class DeleteCacheService extends IntentService {
+public class DeleteCacheService extends JobIntentService {
     public static final String TAG = "DeleteCacheService";
 
-    public DeleteCacheService() {
-        super("DeleteCacheService");
-    }
-
     public static void deleteAll(Context context) {
         Intent intent = new Intent(context, DeleteCacheService.class);
-        context.startService(intent);
+        enqueueWork(context, DeleteCacheService.class, 0x523432, intent);
     }
 
     @Override
-    protected void onHandleIntent(Intent intent) {
-        if (intent == null) {
-            return;
-        }
+    protected void onHandleWork(@NonNull Intent intent) {
         Process.setThreadPriority(Process.THREAD_PRIORITY_LOWEST);
         Log.w(TAG, "Deleting all cached contents!");
         try {
diff --git a/app/src/main/java/org/fdroid/fdroid/data/InstalledAppProviderService.java b/app/src/main/java/org/fdroid/fdroid/data/InstalledAppProviderService.java
index 9548ca057..0084c5e13 100644
--- a/app/src/main/java/org/fdroid/fdroid/data/InstalledAppProviderService.java
+++ b/app/src/main/java/org/fdroid/fdroid/data/InstalledAppProviderService.java
@@ -1,6 +1,5 @@
 package org.fdroid.fdroid.data;
 
-import android.app.IntentService;
 import android.content.ContentValues;
 import android.content.Context;
 import android.content.Intent;
@@ -8,7 +7,9 @@ import android.content.pm.PackageInfo;
 import android.content.pm.PackageManager;
 import android.net.Uri;
 import android.os.Process;
+import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
+import android.support.v4.app.JobIntentService;
 import android.util.Log;
 import org.acra.ACRA;
 import org.fdroid.fdroid.AppUpdateStatusManager;
@@ -40,7 +41,7 @@ import java.util.concurrent.TimeUnit;
  * of this stuff.
  */
 @SuppressWarnings("LineLength")
-public class InstalledAppProviderService extends IntentService {
+public class InstalledAppProviderService extends JobIntentService {
     private static final String TAG = "InstalledAppProviderSer";
 
     private static final String ACTION_INSERT = "org.fdroid.fdroid.data.action.INSERT";
@@ -56,10 +57,6 @@ public class InstalledAppProviderService extends IntentService {
      */
     private PublishSubject<String> packageChangeNotifier;
 
-    public InstalledAppProviderService() {
-        super("InstalledAppProviderService");
-    }
-
     @Override
     public void onCreate() {
         super.onCreate();
@@ -121,7 +118,7 @@ public class InstalledAppProviderService extends IntentService {
         intent.setAction(ACTION_INSERT);
         intent.setData(uri);
         intent.putExtra(EXTRA_PACKAGE_INFO, packageInfo);
-        context.startService(intent);
+        enqueueWork(context, intent);
     }
 
     /**
@@ -138,7 +135,11 @@ public class InstalledAppProviderService extends IntentService {
         Intent intent = new Intent(context, InstalledAppProviderService.class);
         intent.setAction(ACTION_DELETE);
         intent.setData(uri);
-        context.startService(intent);
+        enqueueWork(context, intent);
+    }
+
+    private static void enqueueWork(Context context, Intent intent) {
+        enqueueWork(context, InstalledAppProviderService.class, 0x192834, intent);
     }
 
     /**
@@ -153,7 +154,7 @@ public class InstalledAppProviderService extends IntentService {
      * The installed app cache could get out of sync, e.g. if F-Droid crashed/ or
      * ran out of battery half way through responding to {@link Intent#ACTION_PACKAGE_ADDED}.
      * This method returns immediately, and will continue to work in an
-     * {@link IntentService}.  It doesn't really matter where we put this in the
+     * {@link JobIntentService}.  It doesn't really matter where we put this in the
      * bootstrap process, because it runs in its own thread, at the lowest priority:
      * {@link Process#THREAD_PRIORITY_LOWEST}.
      * <p>
@@ -218,11 +219,8 @@ public class InstalledAppProviderService extends IntentService {
     }
 
     @Override
-    protected void onHandleIntent(Intent intent) {
+    protected void onHandleWork(@NonNull Intent intent) {
         Process.setThreadPriority(Process.THREAD_PRIORITY_LOWEST);
-        if (intent == null) {
-            return;
-        }
 
         String packageName = intent.getData().getSchemeSpecificPart();
         final String action = intent.getAction();
diff --git a/app/src/main/java/org/fdroid/fdroid/installer/InstallerService.java b/app/src/main/java/org/fdroid/fdroid/installer/InstallerService.java
index da2736b05..140665c58 100644
--- a/app/src/main/java/org/fdroid/fdroid/installer/InstallerService.java
+++ b/app/src/main/java/org/fdroid/fdroid/installer/InstallerService.java
@@ -20,11 +20,11 @@
 
 package org.fdroid.fdroid.installer;
 
-import android.app.IntentService;
 import android.content.Context;
 import android.content.Intent;
 import android.net.Uri;
-
+import android.support.annotation.NonNull;
+import android.support.v4.app.JobIntentService;
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.filefilter.WildcardFileFilter;
 import org.fdroid.fdroid.Utils;
@@ -37,11 +37,13 @@ import java.io.FileFilter;
  * This service handles the install process of apk files and
  * uninstall process of apps.
  * <p>
- * This service is based on an IntentService because:
- * - no parallel installs/uninstalls should be allowed,
- * i.e., runs sequentially
- * - no cancel operation is needed. Cancelling an installation
- * would be the same as starting uninstall afterwards
+ * This service is based on an JobIntentService because:
+ * <ul>
+ * <li>no parallel installs/uninstalls should be allowed,
+ * i.e., runs sequentially</li>
+ * <li>no cancel operation is needed. Cancelling an installation
+ * would be the same as starting uninstall afterwards</li>
+ * </ul>
  * <p>
  * The download URL is only used as the unique ID that represents this
  * particular apk throughout the whole install process in
@@ -52,23 +54,15 @@ import java.io.FileFilter;
  * <a href="https://developer.android.com/google/play/expansion-files.html">
  * APK Expansion Files</a> spec.
  */
-public class InstallerService extends IntentService {
+public class InstallerService extends JobIntentService {
     public static final String TAG = "InstallerService";
 
     private static final String ACTION_INSTALL = "org.fdroid.fdroid.installer.InstallerService.action.INSTALL";
     private static final String ACTION_UNINSTALL = "org.fdroid.fdroid.installer.InstallerService.action.UNINSTALL";
 
-    public InstallerService() {
-        super("InstallerService");
-    }
-
     @Override
-    protected void onHandleIntent(Intent intent) {
+    protected void onHandleWork(@NonNull Intent intent) {
         final Apk apk = intent.getParcelableExtra(Installer.EXTRA_APK);
-        if (apk == null) {
-            Utils.debugLog(TAG, "ignoring intent with null EXTRA_APK: " + intent);
-            return;
-        }
         Installer installer = InstallerFactory.create(this, apk);
 
         if (ACTION_INSTALL.equals(intent.getAction())) {
@@ -117,7 +111,7 @@ public class InstallerService extends IntentService {
         intent.setData(localApkUri);
         intent.putExtra(Installer.EXTRA_DOWNLOAD_URI, downloadUri);
         intent.putExtra(Installer.EXTRA_APK, apk);
-        context.startService(intent);
+        enqueueWork(context, intent);
     }
 
     /**
@@ -130,7 +124,10 @@ public class InstallerService extends IntentService {
         Intent intent = new Intent(context, InstallerService.class);
         intent.setAction(ACTION_UNINSTALL);
         intent.putExtra(Installer.EXTRA_APK, apk);
-        context.startService(intent);
+        enqueueWork(context, intent);
     }
 
+    private static void enqueueWork(Context context, @NonNull Intent intent) {
+        enqueueWork(context, InstallerService.class, 0x872394, intent);
+    }
 }

From 07ada4e6b23698c3cadf9a5dd24cc8d1930f0360 Mon Sep 17 00:00:00 2001
From: Hans-Christoph Steiner <hans@eds.org>
Date: Tue, 24 Apr 2018 21:09:36 +0200
Subject: [PATCH 03/11] switch UpdateService to a JobIntentService

closes #1426
---
 app/src/main/AndroidManifest.xml              |  5 ++-
 .../org/fdroid/fdroid/UpdateJobService.java   | 27 +++-----------
 .../java/org/fdroid/fdroid/UpdateService.java | 37 ++++++++++---------
 .../fdroid/net/WifiStateChangeService.java    |  2 +-
 4 files changed, 30 insertions(+), 41 deletions(-)

diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml
index 0bf7d64ca..f8c1972ae 100644
--- a/app/src/main/AndroidManifest.xml
+++ b/app/src/main/AndroidManifest.xml
@@ -252,7 +252,10 @@
             </intent-filter>
         </receiver>
 
-        <service android:name=".UpdateService"/>
+        <service
+                android:name=".UpdateService"
+                android:permission="android.permission.BIND_JOB_SERVICE"
+                android:exported="false"/>
         <service
                 android:name=".UpdateJobService"
                 android:exported="false"
diff --git a/app/src/main/java/org/fdroid/fdroid/UpdateJobService.java b/app/src/main/java/org/fdroid/fdroid/UpdateJobService.java
index 73561ce19..6911584b4 100644
--- a/app/src/main/java/org/fdroid/fdroid/UpdateJobService.java
+++ b/app/src/main/java/org/fdroid/fdroid/UpdateJobService.java
@@ -3,15 +3,10 @@ package org.fdroid.fdroid;
 import android.annotation.TargetApi;
 import android.app.job.JobParameters;
 import android.app.job.JobService;
-import android.content.Intent;
 
 /**
  * Interface between the new {@link android.app.job.JobScheduler} API and
- * our old {@link UpdateService}, which is based on {@link android.app.IntentService}.
- * This does not do things the way it should, e.g. stopping the job on
- * {@link #onStopJob(JobParameters)} and properly reporting
- * {@link #jobFinished(JobParameters, boolean)}, but this at least provides
- * the nice early triggering when there is good power/wifi available.
+ * {@link UpdateService}, which is based on {@link android.support.v4.app.JobIntentService}.
  *
  * @see <a href="https://developer.android.com/about/versions/android-5.0.html#Power">Project Volta: Scheduling jobs</a>
  */
@@ -19,25 +14,13 @@ import android.content.Intent;
 public class UpdateJobService extends JobService {
     @Override
     public boolean onStartJob(final JobParameters params) {
-        new Thread() {
-            @Override
-            public void run() {
-                // faking the actually run time
-                try {
-                    startService(new Intent(UpdateJobService.this, UpdateService.class));
-                    Thread.sleep(2000);
-                } catch (InterruptedException e) {
-                    // ignored
-                } finally {
-                    jobFinished(params, false);
-                }
-            }
-        }.start();
-        return true;
+        UpdateService.updateNow(this);
+        return false;
     }
 
     @Override
     public boolean onStopJob(JobParameters params) {
-        return false;
+        // TODO this should gracefully stop UpdateService
+        return true;
     }
 }
diff --git a/app/src/main/java/org/fdroid/fdroid/UpdateService.java b/app/src/main/java/org/fdroid/fdroid/UpdateService.java
index 2b06fb2b6..7e431430e 100644
--- a/app/src/main/java/org/fdroid/fdroid/UpdateService.java
+++ b/app/src/main/java/org/fdroid/fdroid/UpdateService.java
@@ -37,6 +37,8 @@ import android.os.Handler;
 import android.os.Looper;
 import android.os.Process;
 import android.os.SystemClock;
+import android.support.annotation.NonNull;
+import android.support.v4.app.JobIntentService;
 import android.support.v4.app.NotificationCompat;
 import android.support.v4.content.LocalBroadcastManager;
 import android.support.v7.preference.PreferenceManager;
@@ -60,7 +62,7 @@ import java.lang.ref.WeakReference;
 import java.util.ArrayList;
 import java.util.List;
 
-public class UpdateService extends IntentService {
+public class UpdateService extends JobIntentService {
 
     private static final String TAG = "UpdateService";
 
@@ -81,6 +83,7 @@ public class UpdateService extends IntentService {
     public static final int STATUS_INFO = 5;
 
     private static final String STATE_LAST_UPDATED = "lastUpdateCheck";
+    private static final int JOB_ID = 0xfedcba;
 
     private static final int NOTIFY_ID_UPDATING = 0;
 
@@ -92,10 +95,6 @@ public class UpdateService extends IntentService {
 
     private static boolean updating;
 
-    public UpdateService() {
-        super("UpdateService");
-    }
-
     public static void updateNow(Context context) {
         updateRepoNow(context, null);
     }
@@ -106,7 +105,7 @@ public class UpdateService extends IntentService {
         if (!TextUtils.isEmpty(address)) {
             intent.setData(Uri.parse(address));
         }
-        context.startService(intent);
+        enqueueWork(context, intent);
     }
 
     /**
@@ -117,7 +116,16 @@ public class UpdateService extends IntentService {
     public static void forceUpdateRepo(Context context) {
         Intent intent = new Intent(context, UpdateService.class);
         intent.putExtra(EXTRA_FORCED_UPDATE, true);
-        context.startService(intent);
+        enqueueWork(context, intent);
+    }
+
+    /**
+     * Add work to the queue for processing now
+     *
+     * @see JobIntentService#enqueueWork(Context, Class, int, Intent)
+     */
+    private static void enqueueWork(Context context, @NonNull Intent intent) {
+        enqueueWork(context, UpdateService.class, JOB_ID, intent);
     }
 
     /**
@@ -150,7 +158,7 @@ public class UpdateService extends IntentService {
             Utils.debugLog(TAG, "Using android-21 JobScheduler for updates");
             JobScheduler jobScheduler = (JobScheduler) context.getSystemService(Context.JOB_SCHEDULER_SERVICE);
             ComponentName componentName = new ComponentName(context, UpdateJobService.class);
-            JobInfo.Builder builder = new JobInfo.Builder(0xfedcba, componentName)
+            JobInfo.Builder builder = new JobInfo.Builder(JOB_ID, componentName)
                     .setRequiresDeviceIdle(true)
                     .setPeriodic(interval);
             if (Build.VERSION.SDK_INT >= 26) {
@@ -368,18 +376,13 @@ public class UpdateService extends IntentService {
     }
 
     @Override
-    protected void onHandleIntent(Intent intent) {
+    protected void onHandleWork(@NonNull Intent intent) {
         Process.setThreadPriority(Process.THREAD_PRIORITY_LOWEST);
 
         final long startTime = System.currentTimeMillis();
-        boolean manualUpdate = false;
-        boolean forcedUpdate = false;
-        String address = null;
-        if (intent != null) {
-            address = intent.getDataString();
-            manualUpdate = intent.getBooleanExtra(EXTRA_MANUAL_UPDATE, false);
-            forcedUpdate = intent.getBooleanExtra(EXTRA_FORCED_UPDATE, false);
-        }
+        boolean manualUpdate = intent.getBooleanExtra(EXTRA_MANUAL_UPDATE, false);
+        boolean forcedUpdate = intent.getBooleanExtra(EXTRA_FORCED_UPDATE, false);
+        String address = intent.getDataString();
 
         try {
             final Preferences fdroidPrefs = Preferences.get();
diff --git a/app/src/main/java/org/fdroid/fdroid/net/WifiStateChangeService.java b/app/src/main/java/org/fdroid/fdroid/net/WifiStateChangeService.java
index cdc99524f..023524a9a 100644
--- a/app/src/main/java/org/fdroid/fdroid/net/WifiStateChangeService.java
+++ b/app/src/main/java/org/fdroid/fdroid/net/WifiStateChangeService.java
@@ -47,7 +47,7 @@ import java.util.Locale;
  * This also schedules an update to encourage updates happening on
  * unmetered networks like typical WiFi rather than networks that can
  * cost money or have caps.  The logic for checking the state of the
- * internet connection is in {@link org.fdroid.fdroid.UpdateService#onHandleIntent(Intent)}
+ * internet connection is in {@link org.fdroid.fdroid.UpdateService#onHandleWork(Intent)}
  * <p>
  * Some devices send multiple copies of given events, like a Moto G often
  * sends three {@code CONNECTED} events.  So they have to be debounced to

From 34ac4654387fa0016676843716062d34e38c60f6 Mon Sep 17 00:00:00 2001
From: Hans-Christoph Steiner <hans@eds.org>
Date: Tue, 24 Apr 2018 17:09:33 +0200
Subject: [PATCH 04/11] convert ConnectivityMonitorService to JobIntentService
 #1426

---
 app/src/main/AndroidManifest.xml              |  1 +
 .../net/ConnectivityMonitorService.java       | 21 ++++++++++---------
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml
index f8c1972ae..54e773b96 100644
--- a/app/src/main/AndroidManifest.xml
+++ b/app/src/main/AndroidManifest.xml
@@ -280,6 +280,7 @@
                 android:exported="false"/>
         <service
                 android:name=".net.ConnectivityMonitorService"
+                android:permission="android.permission.BIND_JOB_SERVICE"
                 android:exported="false"/>
         <service android:name=".localrepo.SwapService"/>
         <service
diff --git a/app/src/main/java/org/fdroid/fdroid/net/ConnectivityMonitorService.java b/app/src/main/java/org/fdroid/fdroid/net/ConnectivityMonitorService.java
index 13df8ee6f..8d943a191 100644
--- a/app/src/main/java/org/fdroid/fdroid/net/ConnectivityMonitorService.java
+++ b/app/src/main/java/org/fdroid/fdroid/net/ConnectivityMonitorService.java
@@ -1,6 +1,5 @@
 package org.fdroid.fdroid.net;
 
-import android.app.IntentService;
 import android.content.BroadcastReceiver;
 import android.content.Context;
 import android.content.Intent;
@@ -8,16 +7,18 @@ import android.content.IntentFilter;
 import android.net.ConnectivityManager;
 import android.net.NetworkInfo;
 import android.os.Build;
+import android.support.annotation.NonNull;
+import android.support.v4.app.JobIntentService;
 import com.nostra13.universalimageloader.core.ImageLoader;
 import org.fdroid.fdroid.FDroidApp;
 import org.fdroid.fdroid.Preferences;
 
 /**
- * An {@link IntentService} subclass for tracking whether there is metered or
+ * An {@link JobIntentService} subclass for tracking whether there is metered or
  * unmetered internet available, based on
  * {@link android.net.ConnectivityManager#CONNECTIVITY_ACTION}
  */
-public class ConnectivityMonitorService extends IntentService {
+public class ConnectivityMonitorService extends JobIntentService {
     public static final String TAG = "ConnectivityMonitorServ";
 
     public static final int FLAG_NET_UNAVAILABLE = 0;
@@ -33,10 +34,10 @@ public class ConnectivityMonitorService extends IntentService {
         }
     };
 
-    public ConnectivityMonitorService() {
-        super("ConnectivityMonitorService");
-    }
-
+    /**
+     * Register the {@link BroadcastReceiver} which also starts this
+     * {@code Service} since it is a sticky broadcast.
+     */
     public static void registerAndStart(Context context) {
         context.registerReceiver(CONNECTIVITY_RECEIVER, new IntentFilter(ConnectivityManager.CONNECTIVITY_ACTION));
     }
@@ -44,7 +45,7 @@ public class ConnectivityMonitorService extends IntentService {
     public static void start(Context context) {
         Intent intent = new Intent(context, ConnectivityMonitorService.class);
         intent.setAction(ACTION_START);
-        context.startService(intent);
+        enqueueWork(context, ConnectivityMonitorService.class, 0x982ae7b, intent);
     }
 
     /**
@@ -77,8 +78,8 @@ public class ConnectivityMonitorService extends IntentService {
     }
 
     @Override
-    protected void onHandleIntent(Intent intent) {
-        if (intent != null && ACTION_START.equals(intent.getAction())) {
+    protected void onHandleWork(@NonNull Intent intent) {
+        if (ACTION_START.equals(intent.getAction())) {
             FDroidApp.networkState = getNetworkState(this);
             ImageLoader.getInstance().denyNetworkDownloads(!Preferences.get().isBackgroundDownloadAllowed());
         }

From e6fb837af0859360d25f11b807b8b52eeca16983 Mon Sep 17 00:00:00 2001
From: Hans-Christoph Steiner <hans@eds.org>
Date: Tue, 24 Apr 2018 21:08:22 +0200
Subject: [PATCH 05/11] use JobSchedule to better schedule CleanCacheService

 #588
 #1426
---
 app/src/main/AndroidManifest.xml              |  4 +++
 .../fdroid/fdroid/CleanCacheJobService.java   | 22 ++++++++++++
 .../org/fdroid/fdroid/CleanCacheService.java  | 36 ++++++++++++++-----
 3 files changed, 54 insertions(+), 8 deletions(-)
 create mode 100644 app/src/main/java/org/fdroid/fdroid/CleanCacheJobService.java

diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml
index 54e773b96..168915be6 100644
--- a/app/src/main/AndroidManifest.xml
+++ b/app/src/main/AndroidManifest.xml
@@ -271,6 +271,10 @@
                 android:name=".CleanCacheService"
                 android:permission="android.permission.BIND_JOB_SERVICE"
                 android:exported="false"/>
+        <service
+                android:name=".CleanCacheJobService"
+                android:permission="android.permission.BIND_JOB_SERVICE"
+                android:exported="false"/>
         <service
                 android:name=".DeleteCacheService"
                 android:permission="android.permission.BIND_JOB_SERVICE"
diff --git a/app/src/main/java/org/fdroid/fdroid/CleanCacheJobService.java b/app/src/main/java/org/fdroid/fdroid/CleanCacheJobService.java
new file mode 100644
index 000000000..bbf3d78f8
--- /dev/null
+++ b/app/src/main/java/org/fdroid/fdroid/CleanCacheJobService.java
@@ -0,0 +1,22 @@
+package org.fdroid.fdroid;
+
+import android.annotation.TargetApi;
+import android.app.job.JobParameters;
+import android.app.job.JobService;
+
+/**
+ * Shim to run {@link CleanCacheService} with {@link android.app.job.JobScheduler}
+ */
+@TargetApi(21)
+public class CleanCacheJobService extends JobService {
+    @Override
+    public boolean onStartJob(JobParameters jobParameters) {
+        CleanCacheService.start(this);
+        return false;
+    }
+
+    @Override
+    public boolean onStopJob(JobParameters jobParameters) {
+        return true;
+    }
+}
diff --git a/app/src/main/java/org/fdroid/fdroid/CleanCacheService.java b/app/src/main/java/org/fdroid/fdroid/CleanCacheService.java
index 8833b983f..ebff5c465 100644
--- a/app/src/main/java/org/fdroid/fdroid/CleanCacheService.java
+++ b/app/src/main/java/org/fdroid/fdroid/CleanCacheService.java
@@ -2,6 +2,9 @@ package org.fdroid.fdroid;
 
 import android.app.AlarmManager;
 import android.app.PendingIntent;
+import android.app.job.JobInfo;
+import android.app.job.JobScheduler;
+import android.content.ComponentName;
 import android.content.Context;
 import android.content.Intent;
 import android.os.Build;
@@ -29,6 +32,9 @@ import java.util.concurrent.TimeUnit;
  * is used.
  */
 public class CleanCacheService extends JobIntentService {
+    public static final String TAG = "CleanCacheService";
+
+    private static final int JOB_ID = 0x982374;
 
     /**
      * Schedule or cancel this service to update the app index, according to the
@@ -36,24 +42,38 @@ public class CleanCacheService extends JobIntentService {
      * is changed, or c) on startup, in case we get upgraded.
      */
     public static void schedule(Context context) {
-        // TODO use JobScheduler
         long keepTime = Preferences.get().getKeepCacheTime();
         long interval = TimeUnit.DAYS.toMillis(1);
         if (keepTime < interval) {
             interval = keepTime;
         }
 
-        Intent intent = new Intent(context, CleanCacheService.class);
-        PendingIntent pending = PendingIntent.getService(context, 0, intent, 0);
+        if (Build.VERSION.SDK_INT < 21) {
+            Intent intent = new Intent(context, CleanCacheService.class);
+            PendingIntent pending = PendingIntent.getService(context, 0, intent, 0);
 
-        AlarmManager alarm = (AlarmManager) context.getSystemService(Context.ALARM_SERVICE);
-        alarm.cancel(pending);
-        alarm.setInexactRepeating(AlarmManager.ELAPSED_REALTIME,
-                SystemClock.elapsedRealtime() + 5000, interval, pending);
+            AlarmManager alarm = (AlarmManager) context.getSystemService(Context.ALARM_SERVICE);
+            alarm.cancel(pending);
+            alarm.setInexactRepeating(AlarmManager.ELAPSED_REALTIME,
+                    SystemClock.elapsedRealtime() + 5000, interval, pending);
+        } else {
+            Utils.debugLog(TAG, "Using android-21 JobScheduler for updates");
+            JobScheduler jobScheduler = (JobScheduler) context.getSystemService(Context.JOB_SCHEDULER_SERVICE);
+            ComponentName componentName = new ComponentName(context, CleanCacheJobService.class);
+            JobInfo.Builder builder = new JobInfo.Builder(JOB_ID, componentName)
+                    .setRequiresDeviceIdle(true)
+                    .setRequiresCharging(true)
+                    .setPeriodic(interval);
+            if (Build.VERSION.SDK_INT >= 26) {
+                builder.setRequiresBatteryNotLow(true);
+            }
+            jobScheduler.schedule(builder.build());
+
+        }
     }
 
     public static void start(Context context) {
-        enqueueWork(context, CleanCacheService.class, 0x982374, new Intent(context, CleanCacheService.class));
+        enqueueWork(context, CleanCacheService.class, JOB_ID, new Intent(context, CleanCacheService.class));
     }
 
     @Override

From 7b66ad4920f0078a131f4630e572b0e583868d45 Mon Sep 17 00:00:00 2001
From: Hans-Christoph Steiner <hans@eds.org>
Date: Wed, 25 Apr 2018 11:12:18 +0200
Subject: [PATCH 06/11] don't crash if installerPackageName is null #1436

This will instead lead users with Privileged Extension to the F-Droid
uninstall screen which will probably fail.  But that's better than crashing
in my opinion.

Introduced in c095a85c3dd3c505951bebb52e4ae010c69cc9f9
---
 .../main/java/org/fdroid/fdroid/installer/Installer.java   | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/app/src/main/java/org/fdroid/fdroid/installer/Installer.java b/app/src/main/java/org/fdroid/fdroid/installer/Installer.java
index bb0735b4b..7d39269ba 100644
--- a/app/src/main/java/org/fdroid/fdroid/installer/Installer.java
+++ b/app/src/main/java/org/fdroid/fdroid/installer/Installer.java
@@ -144,9 +144,10 @@ public abstract class Installer {
         }
 
         PackageManager pm = context.getPackageManager();
-        if (Build.VERSION.SDK_INT >= 24 && (
-                pm.getInstallerPackageName(apk.packageName).equals("com.android.packageinstaller")
-                        || pm.getInstallerPackageName(apk.packageName).equals("com.google.android.packageinstaller"))) {
+        String installerPackageName = pm.getInstallerPackageName(apk.packageName);
+        if (Build.VERSION.SDK_INT >= 24 &&
+                ("com.android.packageinstaller".equals(installerPackageName)
+                        || "com.google.android.packageinstaller".equals(installerPackageName))) {
             Utils.debugLog(TAG, "Falling back to default installer for uninstall");
             Intent intent = new Intent(context, DefaultInstallerActivity.class);
             intent.setAction(DefaultInstallerActivity.ACTION_UNINSTALL_PACKAGE);

From 80734891a645f46391bef86ed2859d8040ffbe01 Mon Sep 17 00:00:00 2001
From: Hans-Christoph Steiner <hans@eds.org>
Date: Wed, 25 Apr 2018 11:13:47 +0200
Subject: [PATCH 07/11] Installer instances require an Apk instance, mark
 @NonNull #1436

---
 .../java/org/fdroid/fdroid/installer/DefaultInstaller.java | 3 ++-
 .../org/fdroid/fdroid/installer/ExtensionInstaller.java    | 3 ++-
 .../java/org/fdroid/fdroid/installer/FileInstaller.java    | 3 ++-
 .../main/java/org/fdroid/fdroid/installer/Installer.java   | 3 ++-
 .../java/org/fdroid/fdroid/installer/InstallerFactory.java | 7 +++----
 .../org/fdroid/fdroid/installer/PrivilegedInstaller.java   | 3 ++-
 6 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/app/src/main/java/org/fdroid/fdroid/installer/DefaultInstaller.java b/app/src/main/java/org/fdroid/fdroid/installer/DefaultInstaller.java
index 222256377..b806d5724 100644
--- a/app/src/main/java/org/fdroid/fdroid/installer/DefaultInstaller.java
+++ b/app/src/main/java/org/fdroid/fdroid/installer/DefaultInstaller.java
@@ -24,6 +24,7 @@ import android.app.PendingIntent;
 import android.content.Context;
 import android.content.Intent;
 import android.net.Uri;
+import android.support.annotation.NonNull;
 import org.fdroid.fdroid.data.Apk;
 
 /**
@@ -37,7 +38,7 @@ public class DefaultInstaller extends Installer {
 
     public static final String TAG = "DefaultInstaller";
 
-    DefaultInstaller(Context context, Apk apk) {
+    DefaultInstaller(Context context, @NonNull Apk apk) {
         super(context, apk);
     }
 
diff --git a/app/src/main/java/org/fdroid/fdroid/installer/ExtensionInstaller.java b/app/src/main/java/org/fdroid/fdroid/installer/ExtensionInstaller.java
index 8b710e00a..647ed89af 100644
--- a/app/src/main/java/org/fdroid/fdroid/installer/ExtensionInstaller.java
+++ b/app/src/main/java/org/fdroid/fdroid/installer/ExtensionInstaller.java
@@ -24,6 +24,7 @@ import android.app.PendingIntent;
 import android.content.Context;
 import android.content.Intent;
 import android.net.Uri;
+import android.support.annotation.NonNull;
 import org.fdroid.fdroid.BuildConfig;
 import org.fdroid.fdroid.data.Apk;
 import org.fdroid.fdroid.privileged.install.InstallExtensionDialogActivity;
@@ -44,7 +45,7 @@ import java.io.File;
 @Deprecated
 public class ExtensionInstaller extends Installer {
 
-    ExtensionInstaller(Context context, Apk apk) {
+    ExtensionInstaller(Context context, @NonNull Apk apk) {
         super(context, apk);
     }
 
diff --git a/app/src/main/java/org/fdroid/fdroid/installer/FileInstaller.java b/app/src/main/java/org/fdroid/fdroid/installer/FileInstaller.java
index 9356159aa..aa7d41d48 100644
--- a/app/src/main/java/org/fdroid/fdroid/installer/FileInstaller.java
+++ b/app/src/main/java/org/fdroid/fdroid/installer/FileInstaller.java
@@ -24,11 +24,12 @@ import android.content.Context;
 import android.content.Intent;
 import android.net.Uri;
 
+import android.support.annotation.NonNull;
 import org.fdroid.fdroid.data.Apk;
 
 public class FileInstaller extends Installer {
 
-    public FileInstaller(Context context, Apk apk) {
+    public FileInstaller(Context context, @NonNull Apk apk) {
         super(context, apk);
     }
 
diff --git a/app/src/main/java/org/fdroid/fdroid/installer/Installer.java b/app/src/main/java/org/fdroid/fdroid/installer/Installer.java
index 7d39269ba..38268e983 100644
--- a/app/src/main/java/org/fdroid/fdroid/installer/Installer.java
+++ b/app/src/main/java/org/fdroid/fdroid/installer/Installer.java
@@ -28,6 +28,7 @@ import android.content.pm.PackageManager;
 import android.net.Uri;
 import android.os.Build;
 import android.os.PatternMatcher;
+import android.support.annotation.NonNull;
 import android.support.v4.content.LocalBroadcastManager;
 import android.text.TextUtils;
 import org.fdroid.fdroid.Utils;
@@ -77,7 +78,7 @@ public abstract class Installer {
      * @param apk must be included so that all the phases of the install process
      *            can get all the data about the app, even after F-Droid was killed
      */
-    Installer(Context context, Apk apk) {
+    Installer(Context context, @NonNull Apk apk) {
         this.context = context;
         this.apk = apk;
     }
diff --git a/app/src/main/java/org/fdroid/fdroid/installer/InstallerFactory.java b/app/src/main/java/org/fdroid/fdroid/installer/InstallerFactory.java
index 072b1de74..12acd9079 100644
--- a/app/src/main/java/org/fdroid/fdroid/installer/InstallerFactory.java
+++ b/app/src/main/java/org/fdroid/fdroid/installer/InstallerFactory.java
@@ -21,8 +21,8 @@
 package org.fdroid.fdroid.installer;
 
 import android.content.Context;
+import android.support.annotation.NonNull;
 import android.text.TextUtils;
-
 import org.fdroid.fdroid.Utils;
 import org.fdroid.fdroid.data.Apk;
 
@@ -39,12 +39,11 @@ public class InstallerFactory {
      * @param apk     to be installed, always required.
      * @return instance of an Installer
      */
-    public static Installer create(Context context, Apk apk) {
-        if (apk == null || TextUtils.isEmpty(apk.packageName)) {
+    public static Installer create(Context context, @NonNull Apk apk) {
+        if (TextUtils.isEmpty(apk.packageName)) {
             throw new IllegalArgumentException("Apk.packageName must not be empty: " + apk);
         }
 
-
         Installer installer;
         if (!apk.isApk()) {
             Utils.debugLog(TAG, "Using FileInstaller for non-apk file");
diff --git a/app/src/main/java/org/fdroid/fdroid/installer/PrivilegedInstaller.java b/app/src/main/java/org/fdroid/fdroid/installer/PrivilegedInstaller.java
index 0bc880887..fee625af3 100644
--- a/app/src/main/java/org/fdroid/fdroid/installer/PrivilegedInstaller.java
+++ b/app/src/main/java/org/fdroid/fdroid/installer/PrivilegedInstaller.java
@@ -29,6 +29,7 @@ import android.content.pm.PackageManager;
 import android.net.Uri;
 import android.os.IBinder;
 import android.os.RemoteException;
+import android.support.annotation.NonNull;
 import android.util.Log;
 import org.fdroid.fdroid.Preferences;
 import org.fdroid.fdroid.R;
@@ -256,7 +257,7 @@ public class PrivilegedInstaller extends Installer {
                         "device owner has marked the package as uninstallable.");
     }
 
-    public PrivilegedInstaller(Context context, Apk apk) {
+    public PrivilegedInstaller(Context context, @NonNull Apk apk) {
         super(context, apk);
     }
 

From 94b70608d06cfd43dd328abbd5a8e31e08e94435 Mon Sep 17 00:00:00 2001
From: Hans-Christoph Steiner <hans@eds.org>
Date: Wed, 25 Apr 2018 11:23:39 +0200
Subject: [PATCH 08/11] format ApkProvider using Android Studio Ctrl-Alt-L

---
 .../org/fdroid/fdroid/data/ApkProvider.java   | 78 +++++++++++--------
 1 file changed, 44 insertions(+), 34 deletions(-)

diff --git a/app/src/main/java/org/fdroid/fdroid/data/ApkProvider.java b/app/src/main/java/org/fdroid/fdroid/data/ApkProvider.java
index f132c6c4d..73a8b7da1 100644
--- a/app/src/main/java/org/fdroid/fdroid/data/ApkProvider.java
+++ b/app/src/main/java/org/fdroid/fdroid/data/ApkProvider.java
@@ -26,7 +26,6 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
-@SuppressWarnings("LineLength")
 public class ApkProvider extends FDroidProvider {
 
     private static final String TAG = "ApkProvider";
@@ -41,7 +40,8 @@ public class ApkProvider extends FDroidProvider {
 
     public static final class Helper {
 
-        private Helper() { }
+        private Helper() {
+        }
 
         public static void update(Context context, Apk apk) {
             ContentResolver resolver = context.getContentResolver();
@@ -83,19 +83,21 @@ public class ApkProvider extends FDroidProvider {
         /**
          * Find an app which is closest to the version code suggested by the server, with some caveates:
          * <ul>
-         *     <li>If installed, limit to apks signed by the same signer as the installed apk.</li>
-         *     <li>Otherwise, limit to apks signed by the "preferred" signer (see {@link App#preferredSigner}).</li>
+         * <li>If installed, limit to apks signed by the same signer as the installed apk.</li>
+         * <li>Otherwise, limit to apks signed by the "preferred" signer (see {@link App#preferredSigner}).</li>
          * </ul>
          */
         public static Apk findSuggestedApk(Context context, App app) {
-            return findApkFromAnyRepo(context, app.packageName, app.suggestedVersionCode, app.getMostAppropriateSignature());
+            return findApkFromAnyRepo(context, app.packageName, app.suggestedVersionCode,
+                    app.getMostAppropriateSignature());
         }
 
         public static Apk findApkFromAnyRepo(Context context, String packageName, int versionCode) {
             return findApkFromAnyRepo(context, packageName, versionCode, null, Cols.ALL);
         }
 
-        public static Apk findApkFromAnyRepo(Context context, String packageName, int versionCode, String signature) {
+        public static Apk findApkFromAnyRepo(Context context, String packageName, int versionCode,
+                                             String signature) {
             return findApkFromAnyRepo(context, packageName, versionCode, signature, Cols.ALL);
         }
 
@@ -185,7 +187,7 @@ public class ApkProvider extends FDroidProvider {
     protected static final String PATH_REPO_APP = "repo-app";
     private static final String PATH_APKS = "apks";
     private static final String PATH_APP = "app";
-    private static final String PATH_REPO      = "repo";
+    private static final String PATH_REPO = "repo";
     private static final String PATH_APK_ROW_ID = "apk-rowId";
 
     private static final UriMatcher MATCHER = new UriMatcher(-1);
@@ -226,27 +228,27 @@ public class ApkProvider extends FDroidProvider {
 
     public static Uri getAppUri(String packageName) {
         return getContentUri()
-            .buildUpon()
-            .appendPath(PATH_APP)
-            .appendPath(packageName)
-            .build();
+                .buildUpon()
+                .appendPath(PATH_APP)
+                .appendPath(packageName)
+                .build();
     }
 
     public static Uri getRepoUri(long repoId) {
         return getContentUri()
-            .buildUpon()
-            .appendPath(PATH_REPO)
-            .appendPath(Long.toString(repoId))
-            .build();
+                .buildUpon()
+                .appendPath(PATH_REPO)
+                .appendPath(Long.toString(repoId))
+                .build();
     }
 
     public static Uri getRepoUri(long repoId, String packageName) {
         return getContentUri()
-            .buildUpon()
-            .appendPath(PATH_REPO_APP)
-            .appendPath(Long.toString(repoId))
-            .appendPath(packageName)
-            .build();
+                .buildUpon()
+                .appendPath(PATH_REPO_APP)
+                .appendPath(Long.toString(repoId))
+                .appendPath(packageName)
+                .build();
     }
 
     public static Uri getApkFromAnyRepoUri(Apk apk) {
@@ -296,10 +298,13 @@ public class ApkProvider extends FDroidProvider {
         private boolean antiFeaturesRequested;
 
         /**
-         * If the query includes anti features, then we group by apk id. This is because joining onto the anti-features
-         * table will result in multiple result rows for each apk (potentially), so we will GROUP_CONCAT each of the
-         * anti features into a single comma separated list for each apk. If we are _not_ including anti features, then
-         * don't group by apk, because when doing a COUNT(*) this will result in the wrong result.
+         * If the query includes anti features, then we group by apk id. This
+         * is because joining onto the anti-features table will result in
+         * multiple result rows for each apk (potentially), so we will
+         * {@code GROUP_CONCAT} each of the anti-features into a single comma-
+         * separated list for each apk. If we are _not_ including anti-
+         * features, then don't group by apk, because when doing a COUNT(*)
+         * this will result in the wrong result.
          */
         @Override
         protected String groupBy() {
@@ -313,8 +318,8 @@ public class ApkProvider extends FDroidProvider {
             final String pkg = PackageTable.NAME;
 
             return apk + " AS apk " +
-                " LEFT JOIN " + app + " AS app ON (app." + AppMetadataTable.Cols.ROW_ID + " = apk." + Cols.APP_ID + ")" +
-                " LEFT JOIN " + pkg + " AS pkg ON (pkg." + PackageTable.Cols.ROW_ID + " = app." + AppMetadataTable.Cols.PACKAGE_ID + ")";
+                    " LEFT JOIN " + app + " AS app ON (app." + AppMetadataTable.Cols.ROW_ID + " = apk." + Cols.APP_ID + ")" + // NOPMD NOCHECKSTYLE LineLength
+                    " LEFT JOIN " + pkg + " AS pkg ON (pkg." + PackageTable.Cols.ROW_ID + " = app." + AppMetadataTable.Cols.PACKAGE_ID + ")"; // NOPMD NOCHECKSTYLE LineLength
         }
 
         @Override
@@ -357,9 +362,11 @@ public class ApkProvider extends FDroidProvider {
                     "apk." + Cols.ROW_ID + " = " + apkAntiFeature + "." + ApkAntiFeatureJoinTable.Cols.APK_ID);
 
             leftJoin(AntiFeatureTable.NAME, antiFeature,
-                    apkAntiFeature + "." + ApkAntiFeatureJoinTable.Cols.ANTI_FEATURE_ID + " = " + antiFeature + "." + AntiFeatureTable.Cols.ROW_ID);
+                    apkAntiFeature + "." + ApkAntiFeatureJoinTable.Cols.ANTI_FEATURE_ID + " = "
+                            + antiFeature + "." + AntiFeatureTable.Cols.ROW_ID);
 
-            appendField("group_concat(" + antiFeature + "." + AntiFeatureTable.Cols.NAME + ") as " + Cols.AntiFeatures.ANTI_FEATURES);
+            appendField("group_concat(" + antiFeature + "." + AntiFeatureTable.Cols.NAME + ") as "
+                    + Cols.AntiFeatures.ANTI_FEATURES);
         }
     }
 
@@ -378,11 +385,11 @@ public class ApkProvider extends FDroidProvider {
 
         String selection =
                 alias + Cols.VERSION_CODE + " = ? AND " +
-                alias + Cols.APP_ID + " IN (" + getMetadataIdFromPackageNameQuery() + ")";
+                        alias + Cols.APP_ID + " IN (" + getMetadataIdFromPackageNameQuery() + ")";
 
         List<String> pathSegments = uri.getPathSegments();
         List<String> args = new ArrayList<>(3);
-        args.add(pathSegments.get(1)); // First (0th) path segment is the word "apk" and we are not interested in it.
+        args.add(pathSegments.get(1)); // 0th path segment is the word "apk" and we are not interested in it.
         args.add(pathSegments.get(2));
 
         if (pathSegments.size() >= 4) {
@@ -438,8 +445,8 @@ public class ApkProvider extends FDroidProvider {
         final String[] apkDetails = apkKeys.split(",");
         if (apkDetails.length > MAX_APKS_TO_QUERY) {
             throw new IllegalArgumentException(
-                "Cannot query more than " + MAX_APKS_TO_QUERY + ". " +
-                "You tried to query " + apkDetails.length);
+                    "Cannot query more than " + MAX_APKS_TO_QUERY + ". " +
+                            "You tried to query " + apkDetails.length);
         }
         String alias = includeAlias ? "apk." : "";
         final String[] args = new String[apkDetails.length * 2];
@@ -569,7 +576,9 @@ public class ApkProvider extends FDroidProvider {
     }
 
     protected void ensureAntiFeatures(String[] antiFeatures, long apkId) {
-        db().delete(getApkAntiFeatureJoinTableName(), ApkAntiFeatureJoinTable.Cols.APK_ID + " = ?", new String[] {Long.toString(apkId)});
+        db().delete(getApkAntiFeatureJoinTableName(),
+                ApkAntiFeatureJoinTable.Cols.APK_ID + " = ?",
+                new String[]{Long.toString(apkId)});
         if (antiFeatures != null) {
             Set<String> antiFeatureSet = new HashSet<>();
             for (String antiFeatureName : antiFeatures) {
@@ -594,7 +603,8 @@ public class ApkProvider extends FDroidProvider {
 
     protected long ensureAntiFeature(String antiFeatureName) {
         long antiFeatureId = 0;
-        Cursor cursor = db().query(AntiFeatureTable.NAME, new String[] {AntiFeatureTable.Cols.ROW_ID}, AntiFeatureTable.Cols.NAME + " = ?", new String[]{antiFeatureName}, null, null, null);
+        Cursor cursor = db().query(AntiFeatureTable.NAME, new String[]{AntiFeatureTable.Cols.ROW_ID},
+                AntiFeatureTable.Cols.NAME + " = ?", new String[]{antiFeatureName}, null, null, null);
         if (cursor != null) {
             if (cursor.getCount() > 0) {
                 cursor.moveToFirst();

From 3a5ecc5e8ec6c820dbfdb788dc06f7dbb0699c18 Mon Sep 17 00:00:00 2001
From: Hans-Christoph Steiner <hans@eds.org>
Date: Wed, 25 Apr 2018 17:10:03 +0200
Subject: [PATCH 09/11] do not crash when getting hash of APK that disappears

This changes the logic of Utils.getBinaryHash() to return null on failure
rather than only throwing exceptions.  That makes it easier to handle these
failures where Utils.getBinaryHash() is called.

 #1305
 #855
---
 .../main/java/org/fdroid/fdroid/Utils.java    |  3 +-
 .../main/java/org/fdroid/fdroid/data/Apk.java |  3 +-
 .../org/fdroid/fdroid/data/ApkProvider.java   |  5 +-
 .../main/java/org/fdroid/fdroid/data/App.java | 47 ++++++++++---------
 .../localrepo/CacheSwapAppsService.java       |  6 ++-
 .../fdroid/localrepo/LocalRepoManager.java    |  4 +-
 .../fdroid/fdroid/localrepo/SwapService.java  |  2 +-
 7 files changed, 40 insertions(+), 30 deletions(-)

diff --git a/app/src/main/java/org/fdroid/fdroid/Utils.java b/app/src/main/java/org/fdroid/fdroid/Utils.java
index 73944d1fa..0f5c938f4 100644
--- a/app/src/main/java/org/fdroid/fdroid/Utils.java
+++ b/app/src/main/java/org/fdroid/fdroid/Utils.java
@@ -492,6 +492,7 @@ public final class Utils {
      * probably warranted. See https://www.gitlab.com/fdroid/fdroidclient/issues/855
      * for more detail.
      */
+    @Nullable
     public static String getBinaryHash(File apk, String algo) {
         FileInputStream fis = null;
         try {
@@ -514,12 +515,12 @@ public final class Utils {
             } else if (message.contains(" ENOENT ")) {
                 Utils.debugLog(TAG, apk + " vanished: " + message);
             }
-            throw new IllegalArgumentException(e);
         } catch (NoSuchAlgorithmException e) {
             throw new IllegalArgumentException(e);
         } finally {
             closeQuietly(fis);
         }
+        return null;
     }
 
     /**
diff --git a/app/src/main/java/org/fdroid/fdroid/data/Apk.java b/app/src/main/java/org/fdroid/fdroid/data/Apk.java
index 0c14c1fd5..2618edd94 100644
--- a/app/src/main/java/org/fdroid/fdroid/data/Apk.java
+++ b/app/src/main/java/org/fdroid/fdroid/data/Apk.java
@@ -70,6 +70,7 @@ public class Apk extends ValueObject implements Comparable<Apk>, Parcelable {
     public String versionName;
     public int versionCode;
     public int size; // Size in bytes - 0 means we don't know!
+    @NonNull
     public String hash; // checksum of the APK, in lowercase hex
     public String hashType;
     public int minSdkVersion = SDK_VERSION_MIN_VALUE; // 0 if unknown
@@ -358,7 +359,7 @@ public class Apk extends ValueObject implements Comparable<Apk>, Parcelable {
 
     @Override
     @TargetApi(19)
-    public int compareTo(Apk apk) {
+    public int compareTo(@NonNull Apk apk) {
         if (Build.VERSION.SDK_INT < 19) {
             return Integer.valueOf(versionCode).compareTo(apk.versionCode);
         }
diff --git a/app/src/main/java/org/fdroid/fdroid/data/ApkProvider.java b/app/src/main/java/org/fdroid/fdroid/data/ApkProvider.java
index 73a8b7da1..3a1ea2cf1 100644
--- a/app/src/main/java/org/fdroid/fdroid/data/ApkProvider.java
+++ b/app/src/main/java/org/fdroid/fdroid/data/ApkProvider.java
@@ -9,7 +9,6 @@ import android.net.Uri;
 import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
 import android.util.Log;
-
 import org.fdroid.fdroid.Utils;
 import org.fdroid.fdroid.data.Schema.AntiFeatureTable;
 import org.fdroid.fdroid.data.Schema.ApkAntiFeatureJoinTable;
@@ -20,6 +19,7 @@ import org.fdroid.fdroid.data.Schema.PackageTable;
 import org.fdroid.fdroid.data.Schema.RepoTable;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -164,6 +164,9 @@ public class ApkProvider extends FDroidProvider {
 
         @NonNull
         public static List<Apk> findApksByHash(Context context, String apkHash) {
+            if (apkHash == null) {
+                return Collections.emptyList();
+            }
             ContentResolver resolver = context.getContentResolver();
             final Uri uri = getContentUri();
             String selection = " apk." + Cols.HASH + " = ? ";
diff --git a/app/src/main/java/org/fdroid/fdroid/data/App.java b/app/src/main/java/org/fdroid/fdroid/data/App.java
index c89b92185..33b6b5ee4 100644
--- a/app/src/main/java/org/fdroid/fdroid/data/App.java
+++ b/app/src/main/java/org/fdroid/fdroid/data/App.java
@@ -369,15 +369,33 @@ public class App extends ValueObject implements Comparable<App>, Parcelable {
 
     /**
      * Instantiate from a locally installed package.
+     * <p>
+     * Initializes an {@link App} instances from an APK file. Since the file
+     * could in the cache, and files can disappear from the cache at any time,
+     * this needs to be quite defensive ensuring that {@code apkFile} still
+     * exists.
      */
-    public App(Context context, PackageManager pm, String packageName)
+    @Nullable
+    public static App getInstance(Context context, PackageManager pm, String packageName)
             throws CertificateEncodingException, IOException, PackageManager.NameNotFoundException {
-
+        App app = new App();
         PackageInfo packageInfo = pm.getPackageInfo(packageName, PackageManager.GET_PERMISSIONS);
-        setFromPackageInfo(pm, packageInfo);
-        this.installedApk = new Apk();
         SanitizedFile apkFile = SanitizedFile.knownSanitized(packageInfo.applicationInfo.publicSourceDir);
-        initApkFromApkFile(context, this.installedApk, packageInfo, apkFile);
+        if (apkFile.canRead()) {
+            String hashType = "SHA-256";
+            String hash = Utils.getBinaryHash(apkFile, hashType);
+            if (TextUtils.isEmpty(hash)) {
+                return null;
+            }
+            app.installedApk.hashType = hashType;
+            app.installedApk.hash = hash;
+            app.installedApk.sig = Utils.getPackageSig(packageInfo);
+        }
+
+        app.setFromPackageInfo(pm, packageInfo);
+        app.installedApk = new Apk();
+        app.initInstalledApk(context, app.installedApk, packageInfo, apkFile);
+        return app;
     }
 
     /**
@@ -704,22 +722,6 @@ public class App extends ValueObject implements Comparable<App>, Parcelable {
         this.compatible = true;
     }
 
-    /**
-     * Initializes an {@link App} instances from an APK file. Since the file
-     * could in the cache, and files can disappear from the cache at any time,
-     * this needs to be quite defensive ensuring that {@code apkFile} still
-     * exists.
-     */
-    private void initApkFromApkFile(Context context, Apk apk, PackageInfo packageInfo, SanitizedFile apkFile)
-            throws IOException, CertificateEncodingException {
-        if (apkFile.canRead()) {
-            apk.hashType = "sha256";
-            apk.hash = Utils.getBinaryHash(apkFile, apk.hashType);
-            apk.sig = Utils.getPackageSig(packageInfo);
-        }
-        initInstalledApk(context, apk, packageInfo, apkFile);
-    }
-
     public static void initInstalledObbFiles(Apk apk) {
         File obbdir = getObbDir(apk.packageName);
         FileFilter filter = new RegexFileFilter("(main|patch)\\.[0-9-][0-9]*\\." + apk.packageName + "\\.obb");
@@ -743,6 +745,7 @@ public class App extends ValueObject implements Comparable<App>, Parcelable {
         }
     }
 
+    @SuppressWarnings("EmptyForIteratorPad")
     private void initInstalledApk(Context context, Apk apk, PackageInfo packageInfo, SanitizedFile apkFile)
             throws IOException, CertificateEncodingException {
         apk.compatible = true;
@@ -775,7 +778,7 @@ public class App extends ValueObject implements Comparable<App>, Parcelable {
         JarFile apkJar = new JarFile(apkFile);
         HashSet<String> abis = new HashSet<>(3);
         Pattern pattern = Pattern.compile("^lib/([a-z0-9-]+)/.*");
-        for (Enumeration<JarEntry> jarEntries = apkJar.entries(); jarEntries.hasMoreElements();) {
+        for (Enumeration<JarEntry> jarEntries = apkJar.entries(); jarEntries.hasMoreElements(); ) {
             JarEntry jarEntry = jarEntries.nextElement();
             Matcher matcher = pattern.matcher(jarEntry.getName());
             if (matcher.matches()) {
diff --git a/app/src/main/java/org/fdroid/fdroid/localrepo/CacheSwapAppsService.java b/app/src/main/java/org/fdroid/fdroid/localrepo/CacheSwapAppsService.java
index 885c8bee2..f6452c1eb 100644
--- a/app/src/main/java/org/fdroid/fdroid/localrepo/CacheSwapAppsService.java
+++ b/app/src/main/java/org/fdroid/fdroid/localrepo/CacheSwapAppsService.java
@@ -74,8 +74,10 @@ public class CacheSwapAppsService extends IntentService {
         try {
             PackageManager pm = getPackageManager();
             String packageName = intent.getData().getSchemeSpecificPart();
-            App app = new App(this, pm, packageName);
-            SwapService.putAppInCache(packageName, app);
+            App app = App.getInstance(this, pm, packageName);
+            if (app != null) {
+                SwapService.putAppInCache(packageName, app);
+            }
         } catch (CertificateEncodingException | IOException | PackageManager.NameNotFoundException e) {
             e.printStackTrace();
         }
diff --git a/app/src/main/java/org/fdroid/fdroid/localrepo/LocalRepoManager.java b/app/src/main/java/org/fdroid/fdroid/localrepo/LocalRepoManager.java
index 6c0071054..7ea5905cc 100644
--- a/app/src/main/java/org/fdroid/fdroid/localrepo/LocalRepoManager.java
+++ b/app/src/main/java/org/fdroid/fdroid/localrepo/LocalRepoManager.java
@@ -259,9 +259,9 @@ public final class LocalRepoManager {
         try {
             app = SwapService.getAppFromCache(packageName);
             if (app == null) {
-                app = new App(context.getApplicationContext(), pm, packageName);
+                app = App.getInstance(context.getApplicationContext(), pm, packageName);
             }
-            if (!app.isValid()) {
+            if (app == null || !app.isValid()) {
                 return;
             }
         } catch (PackageManager.NameNotFoundException | CertificateEncodingException | IOException e) {
diff --git a/app/src/main/java/org/fdroid/fdroid/localrepo/SwapService.java b/app/src/main/java/org/fdroid/fdroid/localrepo/SwapService.java
index 3a16ea723..826de3736 100644
--- a/app/src/main/java/org/fdroid/fdroid/localrepo/SwapService.java
+++ b/app/src/main/java/org/fdroid/fdroid/localrepo/SwapService.java
@@ -94,7 +94,7 @@ public class SwapService extends Service {
         return INSTALLED_APPS.get(packageName);
     }
 
-    static void putAppInCache(String packageName, App app) {
+    static void putAppInCache(String packageName, @NonNull App app) {
         INSTALLED_APPS.put(packageName, app);
     }
 

From e76aa0390d9db4d98a7c46026db796b8f10388ec Mon Sep 17 00:00:00 2001
From: Hans-Christoph Steiner <hans@eds.org>
Date: Wed, 25 Apr 2018 17:28:43 +0200
Subject: [PATCH 10/11] make Anti-Features list in App Details clickable

This should provide some more info about each anti-feature.
---
 .../views/AppDetailsRecyclerViewAdapter.java  | 33 ++++++++++++-------
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/app/src/main/java/org/fdroid/fdroid/views/AppDetailsRecyclerViewAdapter.java b/app/src/main/java/org/fdroid/fdroid/views/AppDetailsRecyclerViewAdapter.java
index 12ffd19bc..dd5bf1df3 100644
--- a/app/src/main/java/org/fdroid/fdroid/views/AppDetailsRecyclerViewAdapter.java
+++ b/app/src/main/java/org/fdroid/fdroid/views/AppDetailsRecyclerViewAdapter.java
@@ -450,19 +450,24 @@ public class AppDetailsRecyclerViewAdapter
                     }
                 }
             });
-            if (app.antiFeatures != null) {
+            if (app.antiFeatures != null && app.antiFeatures.length > 0) {
                 StringBuilder sb = new StringBuilder();
+                sb.append("<ul>");
                 for (String af : app.antiFeatures) {
                     String afdesc = descAntiFeature(af);
-                    sb.append("\t• ").append(afdesc).append('\n');
-                }
-                if (sb.length() > 0) {
-                    sb.setLength(sb.length() - 1);
-                    antiFeaturesView.setText(sb.toString());
-                } else {
-                    antiFeaturesView.setVisibility(View.GONE);
+                    sb.append("<li><a href=\"https://f-droid.org/wiki/page/Antifeature:")
+                            .append(af)
+                            .append("\">")
+                            .append(afdesc)
+                            .append("</a></li>");
                 }
+                sb.append("</ul>");
+                antiFeaturesView.setText(Html.fromHtml(sb.toString()));
+                antiFeaturesView.setMovementMethod(LinkMovementMethod.getInstance());
+            } else {
+                antiFeaturesView.setVisibility(View.GONE);
             }
+
             updateAntiFeaturesWarning();
             buttonSecondaryView.setText(R.string.menu_uninstall);
             buttonSecondaryView.setVisibility(app.isUninstallable(context) ? View.VISIBLE : View.INVISIBLE);
@@ -664,7 +669,8 @@ public class AppDetailsRecyclerViewAdapter
             contentView = (LinearLayout) view.findViewById(R.id.ll_content);
         }
 
-        protected abstract @DrawableRes int getIcon();
+        @DrawableRes
+        protected abstract int getIcon();
 
         /**
          * Depending on whether we are expanded or not, update the icon which indicates whether the
@@ -694,7 +700,8 @@ public class AppDetailsRecyclerViewAdapter
             updateExpandableItem(showVersions);
         }
 
-        protected @DrawableRes int getIcon() {
+        @DrawableRes
+        protected int getIcon() {
             return R.drawable.ic_access_time_24dp_grey600;
         }
     }
@@ -794,7 +801,8 @@ public class AppDetailsRecyclerViewAdapter
             contentView.addView(perms.getPermissionsView(AppSecurityPermissions.WHICH_ALL));
         }
 
-        protected @DrawableRes int getIcon() {
+        @DrawableRes
+        protected int getIcon() {
             return R.drawable.ic_lock_24dp_grey600;
         }
     }
@@ -867,7 +875,8 @@ public class AppDetailsRecyclerViewAdapter
             }
         }
 
-        protected @DrawableRes int getIcon() {
+        @DrawableRes
+        protected int getIcon() {
             return R.drawable.ic_website;
         }
     }

From fb8cbd04c332d0c10909588f10561bd0e13177ed Mon Sep 17 00:00:00 2001
From: Hans-Christoph Steiner <hans@eds.org>
Date: Wed, 25 Apr 2018 17:29:08 +0200
Subject: [PATCH 11/11] add text description for DisabledAlgorithm and
 KnownVuln anti-features

---
 .../fdroid/fdroid/views/AppDetailsRecyclerViewAdapter.java    | 4 ++++
 app/src/main/res/values/strings.xml                           | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/app/src/main/java/org/fdroid/fdroid/views/AppDetailsRecyclerViewAdapter.java b/app/src/main/java/org/fdroid/fdroid/views/AppDetailsRecyclerViewAdapter.java
index dd5bf1df3..9a82a6947 100644
--- a/app/src/main/java/org/fdroid/fdroid/views/AppDetailsRecyclerViewAdapter.java
+++ b/app/src/main/java/org/fdroid/fdroid/views/AppDetailsRecyclerViewAdapter.java
@@ -543,6 +543,10 @@ public class AppDetailsRecyclerViewAdapter
                     return itemView.getContext().getString(R.string.antiupstreamnonfreelist);
                 case "NonFreeAssets":
                     return itemView.getContext().getString(R.string.antinonfreeassetslist);
+                case "DisabledAlgorithm":
+                    return itemView.getContext().getString(R.string.antidisabledalgorithmlist);
+                case "KnownVuln":
+                    return itemView.getContext().getString(R.string.antiknownvulnlist);
                 default:
                     return af;
             }
diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml
index f8181bc7f..a27bfa507 100644
--- a/app/src/main/res/values/strings.xml
+++ b/app/src/main/res/values/strings.xml
@@ -203,6 +203,8 @@ This often occurs with apps installed via Google Play or other sources, if they
     <string name="antinonfreedeplist">This app depends on other non-free apps</string>
     <string name="antiupstreamnonfreelist">The upstream source code is not entirely Free</string>
     <string name="antinonfreeassetslist">This app contains non-free assets</string>
+    <string name="antidisabledalgorithmlist">This app has a weak security signature</string>
+    <string name="antiknownvulnlist">This app contains a known security vulnerability</string>
 
     <string name="display">Display</string>