From 41afa65947d5993c1912438e586bbb5ebe46b90d Mon Sep 17 00:00:00 2001
From: Jeff Sharkey <jsharkey@android.com>
Date: Tue, 1 May 2018 13:09:28 -0600
Subject: [PATCH] Avoid redundant getAllocatableBytes() calls.

Shuffle around the ordering of resolveInstallVolume() so that we
determine the fitsOnInternal value while doing a drive-by of all
possible target volumes.  This way we only call getAllocatableBytes()
internal storage once.

Bug: 70985520
Test: atest android.content.pm.PackageHelperTests
Change-Id: I8273c239af4a13b1db17f320e15ee0ef2dc89186
---
 .../internal/content/PackageHelper.java       | 58 +++++++++----------
 1 file changed, 26 insertions(+), 32 deletions(-)

diff --git a/core/java/com/android/internal/content/PackageHelper.java b/core/java/com/android/internal/content/PackageHelper.java
index 8a456d1c6f581..e2b8f7dca6808 100644
--- a/core/java/com/android/internal/content/PackageHelper.java
+++ b/core/java/com/android/internal/content/PackageHelper.java
@@ -94,14 +94,6 @@ public class PackageHelper {
         abstract public boolean getAllow3rdPartyOnInternalConfig(Context context);
         abstract public ApplicationInfo getExistingAppInfo(Context context, String packageName);
         abstract public File getDataDirectory();
-
-        public boolean fitsOnInternalStorage(Context context, SessionParams params)
-                throws IOException {
-            StorageManager storage = getStorageManager(context);
-            final UUID target = storage.getUuidForPath(getDataDirectory());
-            return (params.sizeBytes <= storage.getAllocatableBytes(target,
-                    translateAllocateFlags(params.installFlags)));
-        }
     }
 
     private synchronized static TestableInterface getDefaultTestableInterface() {
@@ -175,6 +167,7 @@ public class PackageHelper {
     @VisibleForTesting
     public static String resolveInstallVolume(Context context, SessionParams params,
             TestableInterface testInterface) throws IOException {
+        final StorageManager storageManager = testInterface.getStorageManager(context);
         final boolean forceAllowOnExternal = testInterface.getForceAllowOnExternalSetting(context);
         final boolean allow3rdPartyOnInternal =
                 testInterface.getAllow3rdPartyOnInternalConfig(context);
@@ -183,9 +176,31 @@ public class PackageHelper {
         ApplicationInfo existingInfo = testInterface.getExistingAppInfo(context,
                 params.appPackageName);
 
-        final boolean fitsOnInternal = testInterface.fitsOnInternalStorage(context, params);
-        final StorageManager storageManager =
-                testInterface.getStorageManager(context);
+        // Figure out best candidate volume, and also if we fit on internal
+        final ArraySet<String> allCandidates = new ArraySet<>();
+        boolean fitsOnInternal = false;
+        VolumeInfo bestCandidate = null;
+        long bestCandidateAvailBytes = Long.MIN_VALUE;
+        for (VolumeInfo vol : storageManager.getVolumes()) {
+            if (vol.type == VolumeInfo.TYPE_PRIVATE && vol.isMountedWritable()) {
+                final boolean isInternalStorage = ID_PRIVATE_INTERNAL.equals(vol.id);
+                final UUID target = storageManager.getUuidForPath(new File(vol.path));
+                final long availBytes = storageManager.getAllocatableBytes(target,
+                        translateAllocateFlags(params.installFlags));
+                if (isInternalStorage) {
+                    fitsOnInternal = (params.sizeBytes <= availBytes);
+                }
+                if (!isInternalStorage || allow3rdPartyOnInternal) {
+                    if (availBytes >= params.sizeBytes) {
+                        allCandidates.add(vol.fsUuid);
+                    }
+                    if (availBytes >= bestCandidateAvailBytes) {
+                        bestCandidate = vol;
+                        bestCandidateAvailBytes = availBytes;
+                    }
+                }
+            }
+        }
 
         // System apps always forced to internal storage
         if (existingInfo != null && existingInfo.isSystemApp()) {
@@ -198,27 +213,6 @@ public class PackageHelper {
             }
         }
 
-        // Now deal with non-system apps.
-        final ArraySet<String> allCandidates = new ArraySet<>();
-        VolumeInfo bestCandidate = null;
-        long bestCandidateAvailBytes = Long.MIN_VALUE;
-        for (VolumeInfo vol : storageManager.getVolumes()) {
-            boolean isInternalStorage = ID_PRIVATE_INTERNAL.equals(vol.id);
-            if (vol.type == VolumeInfo.TYPE_PRIVATE && vol.isMountedWritable()
-                    && (!isInternalStorage || allow3rdPartyOnInternal)) {
-                final UUID target = storageManager.getUuidForPath(new File(vol.path));
-                final long availBytes = storageManager.getAllocatableBytes(target,
-                        translateAllocateFlags(params.installFlags));
-                if (availBytes >= params.sizeBytes) {
-                    allCandidates.add(vol.fsUuid);
-                }
-                if (availBytes >= bestCandidateAvailBytes) {
-                    bestCandidate = vol;
-                    bestCandidateAvailBytes = availBytes;
-                }
-            }
-        }
-
         // If app expresses strong desire for internal storage, honor it
         if (!forceAllowOnExternal
                 && params.installLocation == PackageInfo.INSTALL_LOCATION_INTERNAL_ONLY) {