Merge "Don't try to recreate IncrementalFileStorages on re-commit." into rvc-dev am: eb191392e6

Change-Id: Ie7eca6534835d3f63ed037341b49c2f0c1aa0a57

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

@@ -92,10 +92,7 @@ public final class IncrementalFileStorages {
             }
         }
 
-        if (!result.mDefaultStorage.startLoading()) {
+        result.startLoading();
-            // TODO(b/146080380): add incremental-specific error code
-            throw new IOException("Failed to start loading data for Incremental installation.");
-        }
 
         return result;
     }
@@ -143,6 +140,15 @@ public final class IncrementalFileStorages {
         }
     }
 
+    /**
+     * Starts or re-starts loading of data.
+     */
+    public void startLoading() throws IOException {
+        if (!mDefaultStorage.startLoading()) {
+            throw new IOException("Failed to start loading data for Incremental installation.");
+        }
+    }
+
     /**
      * Resets the states and unbinds storage instances for an installation session.
      * TODO(b/136132412): make sure unnecessary binds are removed but useful storages are kept

services/core/java/com/android/server/pm/PackageInstallerSession.java

@@ -395,7 +395,6 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
 
     private boolean mDataLoaderFinished = false;
 
-    // TODO(b/146080380): merge file list with Callback installation.
     private IncrementalFileStorages mIncrementalFileStorages;
 
     private static final FileFilter sAddedApkFilter = new FileFilter() {
@@ -2698,6 +2697,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
 
     /**
      * Makes sure files are present in staging location.
+     * @return if the image is ready for installation
      */
     @GuardedBy("mLock")
     private boolean prepareDataLoaderLocked()
@@ -2709,6 +2709,17 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
             return true;
         }
 
+        // Retrying commit.
+        if (mIncrementalFileStorages != null) {
+            try {
+                mIncrementalFileStorages.startLoading();
+            } catch (IOException e) {
+                throw new PackageManagerException(INSTALL_FAILED_MEDIA_UNAVAILABLE, e.getMessage(),
+                        e.getCause());
+            }
+            return false;
+        }
+
         final List<InstallationFileParcel> addedFiles = new ArrayList<>();
         final List<String> removedFiles = new ArrayList<>();
 

services/incremental/IncrementalService.cpp

@@ -748,7 +748,7 @@ int IncrementalService::unbind(StorageId storage, std::string_view target) {
         return -EINVAL;
     }
 
-    LOG(INFO) << "Removing bind point " << target;
+    LOG(INFO) << "Removing bind point " << target << " for storage " << storage;
 
     // Here we should only look up by the exact target, not by a subdirectory of any existing mount,
     // otherwise there's a chance to unmount something completely unrelated
@@ -1807,6 +1807,8 @@ bool IncrementalService::DataLoaderStub::fsmStep() {
         targetStatus = mTargetStatus;
     }
 
+    LOG(DEBUG) << "fsmStep: " << mId << ": " << currentStatus << " -> " << targetStatus;
+
     if (currentStatus == targetStatus) {
         return true;
     }
@@ -1868,8 +1870,8 @@ binder::Status IncrementalService::DataLoaderStub::onStatusChanged(MountId mount
         listener = mListener;
 
         if (mCurrentStatus == IDataLoaderStatusListener::DATA_LOADER_UNAVAILABLE) {
-            // For unavailable, reset target status.
+            // For unavailable, unbind from DataLoader to ensure proper re-commit.
-            setTargetStatusLocked(IDataLoaderStatusListener::DATA_LOADER_UNAVAILABLE);
+            setTargetStatusLocked(IDataLoaderStatusListener::DATA_LOADER_DESTROYED);
         }
     }
 

services/incremental/test/IncrementalServiceTest.cpp

@@ -677,10 +677,10 @@ TEST_F(IncrementalServiceTest, testStartDataLoaderRecreateOnPendingReads) {
     mDataLoaderManager->bindToDataLoaderSuccess();
     mDataLoaderManager->getDataLoaderSuccess();
     EXPECT_CALL(*mDataLoaderManager, bindToDataLoader(_, _, _, _)).Times(2);
-    EXPECT_CALL(*mDataLoaderManager, unbindFromDataLoader(_)).Times(1);
+    EXPECT_CALL(*mDataLoaderManager, unbindFromDataLoader(_)).Times(2);
     EXPECT_CALL(*mDataLoader, create(_, _, _, _)).Times(2);
     EXPECT_CALL(*mDataLoader, start(_)).Times(0);
-    EXPECT_CALL(*mDataLoader, destroy(_)).Times(1);
+    EXPECT_CALL(*mDataLoader, destroy(_)).Times(2);
     EXPECT_CALL(*mVold, unmountIncFs(_)).Times(2);
     EXPECT_CALL(*mLooper, addFd(MockIncFs::kPendingReadsFd, _, _, _, _)).Times(1);
     EXPECT_CALL(*mLooper, removeFd(MockIncFs::kPendingReadsFd)).Times(1);