Make sure to query package manager for PackageInfo when required.

A previous commit accidentally pushed the code which queries the
`PackageManager` to a different method, but then still used the
`packageInfo` which was supposed to be populated by that code.

This change rectifies this, and in the process also clarifies/documents under
what circumstances the `PackageManager` needs to be queried, rather than
relying on the incoming intent.

Fixes #686.

app/src/main/java/org/fdroid/fdroid/data/InstalledAppProviderService.java

@@ -9,6 +9,7 @@ import android.content.pm.PackageManager;
 import android.content.pm.Signature;
 import android.net.Uri;
 import android.os.Process;
+import android.support.annotation.Nullable;
 
 import org.fdroid.fdroid.Hasher;
 import org.fdroid.fdroid.Utils;
@@ -32,7 +33,7 @@ import rx.subjects.PublishSubject;
  * Since {@link android.content.ContentProvider#insert(Uri, ContentValues)} does not check
  * for duplicate records, it is entirely the job of this service to ensure that it is not
  * inserting duplicate versions of the same installed APK. On that note,
- * {@link #insertAppIntoDb(Context, String, PackageInfo)} and
+ * {@link #insertAppIntoDb(Context, PackageInfo, String, String)} and
  * {@link #deleteAppFromDb(Context, String)} are both static methods to enable easy testing
  * of this stuff.
  */
@@ -156,10 +157,12 @@ public class InstalledAppProviderService extends IntentService {
             String packageName = intent.getData().getSchemeSpecificPart();
             final String action = intent.getAction();
             if (ACTION_INSERT.equals(action)) {
-                PackageInfo packageInfo = intent.getParcelableExtra(EXTRA_PACKAGE_INFO);
+                PackageInfo packageInfo = getPackageInfo(intent, packageName);
-                String hashType = "sha256";
+                if (packageInfo != null) {
-                String hash = Utils.getBinaryHash(new File(packageInfo.applicationInfo.publicSourceDir), hashType);
+                    String hashType = "sha256";
-                insertAppIntoDb(this, packageName, packageInfo, hashType, hash);
+                    String hash = Utils.getBinaryHash(new File(packageInfo.applicationInfo.publicSourceDir), hashType);
+                    insertAppIntoDb(this, packageInfo, hashType, hash);
+                }
             } else if (ACTION_DELETE.equals(action)) {
                 deleteAppFromDb(this, packageName);
             }
@@ -167,23 +170,38 @@ public class InstalledAppProviderService extends IntentService {
         }
     }
 
+    /**
+     * This class will either have received an intent from the {@link InstalledAppProviderService}
+     * itself, while iterating over installed apps, or from a {@link Intent#ACTION_PACKAGE_ADDED}
+     * broadcast. In the first case, it will already have a {@link PackageInfo} for us. However if
+     * it is from the later case, we'll need to query the {@link PackageManager} ourselves to get
+     * this info.
+     *
+     * Can still return null, as there is potentially race conditions to do with uninstalling apps
+     * such that querying the {@link PackageManager} for a given package may throw an exception.
+     */
+    @Nullable
+    private PackageInfo getPackageInfo(Intent intent, String packageName) {
+        PackageInfo packageInfo = intent.getParcelableExtra(EXTRA_PACKAGE_INFO);
+        if (packageInfo != null) {
+            return packageInfo;
+        }
+
+        try {
+            return getPackageManager().getPackageInfo(packageName, PackageManager.GET_SIGNATURES);
+        } catch (PackageManager.NameNotFoundException e) {
+            e.printStackTrace();
+            return null;
+        }
+    }
+
     /**
      * @param hash Although the has could be calculated within this function, it is helpful to inject
      *             the hash so as to be able to use this method during testing. Otherwise, the
      *             hashing method will try to hash a non-existent .apk file and try to insert NULL
      *             into the database when under test.
      */
-    static void insertAppIntoDb(Context context, String packageName, PackageInfo packageInfo, String hashType, String hash) {
+    static void insertAppIntoDb(Context context, PackageInfo packageInfo, String hashType, String hash) {
-        if (packageInfo == null) {
-            try {
-                packageInfo = context.getPackageManager().getPackageInfo(packageName,
-                        PackageManager.GET_SIGNATURES);
-            } catch (PackageManager.NameNotFoundException e) {
-                e.printStackTrace();
-                return;
-            }
-        }
-
         Uri uri = InstalledAppProvider.getContentUri();
         ContentValues contentValues = new ContentValues();
         contentValues.put(InstalledAppProvider.DataColumns.PACKAGE_NAME, packageInfo.packageName);

app/src/test/java/org/fdroid/fdroid/data/InstalledAppTestUtils.java

@@ -21,7 +21,7 @@ public class InstalledAppTestUtils {
         info.applicationInfo.publicSourceDir = "/tmp/mock-location";
         String hashType = "sha256";
         String hash = "00112233445566778899aabbccddeeff";
-        InstalledAppProviderService.insertAppIntoDb(context, packageName, info, hashType, hash);
+        InstalledAppProviderService.insertAppIntoDb(context, info, hashType, hash);
     }
 
 }