Merge changes I8942a2db,Ibcef3f40,I6e765563 into rvc-dev

* changes:
  [incfs] Cleanup storage creation and don't use temp directory
  [incfs] Notify IncrementalService about system startup later
  [incfs] Fix bad path::join() path preparation

core/java/android/os/incremental/IncrementalFileStorages.java

@@ -38,16 +38,10 @@ import android.content.pm.DataLoaderParams;
 import android.content.pm.IDataLoaderStatusListener;
 import android.content.pm.InstallationFileParcel;
 import android.text.TextUtils;
-import android.util.Slog;
 
 import java.io.File;
 import java.io.IOException;
-import java.nio.file.Files;
-import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.util.List;
-import java.util.Objects;
-import java.util.Random;
 
 /**
  * This class manages storage instances used during a package installation session.
@@ -56,13 +50,9 @@ import java.util.Random;
 public final class IncrementalFileStorages {
     private static final String TAG = "IncrementalFileStorages";
 
-    private static final String TMP_DIR_ROOT = "/data/incremental/tmp";
+    private @NonNull final IncrementalManager mIncrementalManager;
-    private static final Random TMP_DIR_RANDOM = new Random();
+    private @NonNull final File mStageDir;
-
     private @Nullable IncrementalStorage mDefaultStorage;
-    private @Nullable String mDefaultDir;
-    private @NonNull IncrementalManager mIncrementalManager;
-    private @NonNull File mStageDir;
 
     /**
      * Set up files and directories used in an installation session. Only used by Incremental.
@@ -85,72 +75,63 @@ public final class IncrementalFileStorages {
             throw new IOException("Failed to obtain incrementalManager.");
         }
 
-        IncrementalFileStorages result = null;
+        final IncrementalFileStorages result =
-        try {
+                new IncrementalFileStorages(stageDir, incrementalManager, dataLoaderParams,
-            result = new IncrementalFileStorages(stageDir, incrementalManager, dataLoaderParams,
+                                            dataLoaderStatusListener);
-                    dataLoaderStatusListener);
+        for (InstallationFileParcel file : addedFiles) {
-
+            if (file.location == LOCATION_DATA_APP) {
-            if (!addedFiles.isEmpty()) {
+                try {
-                result.mDefaultStorage.bind(stageDir.getAbsolutePath());
+                    result.addApkFile(file);
-            }
+                } catch (IOException e) {
-
+                    // TODO(b/146080380): add incremental-specific error code
-            for (InstallationFileParcel file : addedFiles) {
+                    throw new IOException(
-                if (file.location == LOCATION_DATA_APP) {
+                            "Failed to add file to IncFS: " + file.name + ", reason: ", e);
-                    try {
-                        result.addApkFile(file);
-                    } catch (IOException e) {
-                        // TODO(b/146080380): add incremental-specific error code
-                        throw new IOException(
-                                "Failed to add file to IncFS: " + file.name + ", reason: ", e);
-                    }
-                } else {
-                    throw new IOException("Unknown file location: " + file.location);
                 }
+            } else {
+                throw new IOException("Unknown file location: " + file.location);
             }
-
-            // TODO(b/146080380): remove 5 secs wait in startLoading
-            if (!result.mDefaultStorage.startLoading()) {
-                // TODO(b/146080380): add incremental-specific error code
-                throw new IOException("Failed to start loading data for Incremental installation.");
-            }
-
-            return result;
-        } catch (IOException e) {
-            Slog.e(TAG, "Failed to initialize Incremental file storages. Cleaning up...", e);
-            if (result != null) {
-                result.cleanUp();
-            }
-            throw e;
         }
+
+        if (!result.mDefaultStorage.startLoading()) {
+            // TODO(b/146080380): add incremental-specific error code
+            throw new IOException("Failed to start loading data for Incremental installation.");
+        }
+
+        return result;
     }
 
     private IncrementalFileStorages(@NonNull File stageDir,
             @NonNull IncrementalManager incrementalManager,
             @NonNull DataLoaderParams dataLoaderParams,
             @Nullable IDataLoaderStatusListener dataLoaderStatusListener) throws IOException {
-        mStageDir = stageDir;
+        try {
-        mIncrementalManager = incrementalManager;
+            mStageDir = stageDir;
-        if (dataLoaderParams.getComponentName().getPackageName().equals("local")) {
+            mIncrementalManager = incrementalManager;
-            final String incrementalPath = dataLoaderParams.getArguments();
+            if (dataLoaderParams.getComponentName().getPackageName().equals("local")) {
-            mDefaultDir = incrementalPath;
+                final String incrementalPath = dataLoaderParams.getArguments();
-            if (TextUtils.isEmpty(mDefaultDir)) {
+                if (TextUtils.isEmpty(incrementalPath)) {
-                throw new IOException("Failed to create storage: incrementalPath is empty");
+                    throw new IOException("Failed to create storage: incrementalPath is empty");
+                }
+                mDefaultStorage = mIncrementalManager.openStorage(incrementalPath);
+                if (mDefaultStorage == null) {
+                    throw new IOException(
+                            "Couldn't open incremental storage at " + incrementalPath);
+                }
+                mDefaultStorage.bind(stageDir.getAbsolutePath());
+            } else {
+                mDefaultStorage = mIncrementalManager.createStorage(stageDir.getAbsolutePath(),
+                        dataLoaderParams,
+                        dataLoaderStatusListener,
+                        IncrementalManager.CREATE_MODE_CREATE
+                                | IncrementalManager.CREATE_MODE_TEMPORARY_BIND, false);
+                if (mDefaultStorage == null) {
+                    throw new IOException(
+                            "Couldn't create incremental storage at " + stageDir);
+                }
             }
-            mDefaultStorage = mIncrementalManager.openStorage(incrementalPath);
+        } catch (IOException e) {
-        } else {
+            cleanUp();
-            mDefaultDir = getTempDir();
+            throw e;
-            if (mDefaultDir == null) {
-                throw new IOException("Failed to create storage: tempDir is empty");
-            }
-            mDefaultStorage = mIncrementalManager.createStorage(mDefaultDir,
-                    dataLoaderParams,
-                    dataLoaderStatusListener,
-                    IncrementalManager.CREATE_MODE_CREATE
-                            | IncrementalManager.CREATE_MODE_TEMPORARY_BIND, false);
-        }
-
-        if (mDefaultStorage == null) {
-            throw new IOException("Failed to create storage");
         }
     }
 
@@ -167,27 +148,14 @@ public final class IncrementalFileStorages {
      * TODO(b/136132412): make sure unnecessary binds are removed but useful storages are kept
      */
     public void cleanUp() {
-        Objects.requireNonNull(mDefaultStorage);
+        if (mDefaultStorage == null) {
+            return;
+        }
 
         try {
-            mDefaultStorage.unBind(mDefaultDir);
             mDefaultStorage.unBind(mStageDir.getAbsolutePath());
         } catch (IOException ignored) {
         }
-
-        mDefaultDir = null;
         mDefaultStorage = null;
     }
-
-    private static String getTempDir() {
-        final Path tmpDir = Paths.get(TMP_DIR_ROOT,
-                String.valueOf(TMP_DIR_RANDOM.nextInt(Integer.MAX_VALUE - 1)));
-        try {
-            Files.createDirectories(tmpDir);
-        } catch (Exception ex) {
-            Slog.e(TAG, "Failed to create dir", ex);
-            return null;
-        }
-        return tmpDir.toAbsolutePath().toString();
-    }
 }

services/incremental/path.cpp

@@ -44,7 +44,7 @@ bool PathLess::operator()(std::string_view l, std::string_view r) const {
                                         PathCharsLess());
 }
 
-static void preparePathComponent(std::string_view path, bool trimFront) {
+static void preparePathComponent(std::string_view& path, bool trimFront) {
     if (trimFront) {
         while (!path.empty() && path.front() == '/') {
             path.remove_prefix(1);

services/java/com/android/server/SystemServer.java

@@ -2174,12 +2174,6 @@ public final class SystemServer {
         mPackageManagerService.systemReady();
         t.traceEnd();
 
-        if (mIncrementalServiceHandle != 0) {
-            t.traceBegin("MakeIncrementalServiceReady");
-            setIncrementalServiceSystemReady(mIncrementalServiceHandle);
-            t.traceEnd();
-        }
-
         t.traceBegin("MakeDisplayManagerServiceReady");
         try {
             // TODO: use boot phase and communicate these flags some other way
@@ -2449,6 +2443,12 @@ public final class SystemServer {
                 reportWtf("Notifying incident daemon running", e);
             }
             t.traceEnd();
+
+            if (mIncrementalServiceHandle != 0) {
+                t.traceBegin("MakeIncrementalServiceReady");
+                setIncrementalServiceSystemReady(mIncrementalServiceHandle);
+                t.traceEnd();
+            }
         }, t);
 
         t.traceEnd(); // startOtherServices