Fix the progress getting for mapped files

Mapped files don't support querying their loading progress,
so we should simply skip them - they are already a part of
some other file, and will get accounted for loading when
that file's progress get queried

+ a bunch of small improvements

Bug: 180535478
Test: atest service.incremental_test, adb install --incremental
  with mapped native libs

Change-Id: Ifc8a402144f2f3669a0419124fb0f35d7002190a
(cherry picked from commit 7731ebd1d8187c92a992d1f53c4114a6c40f7563)

4 files changed, 37 insertions, 27 deletions

diff --git a/services/incremental/IncrementalService.cpp b/services/incremental/IncrementalService.cpp
index 24623b2dacfc..ce6e6ab1e29c 100644
--- a/services/incremental/IncrementalService.cpp
+++ b/services/incremental/IncrementalService.cpp
@@ -1936,25 +1936,33 @@ IncrementalService::LoadingProgress IncrementalService::getLoadingProgress(
 }
 
 IncrementalService::LoadingProgress IncrementalService::getLoadingProgressFromPath(
-        const IncFsMount& ifs, std::string_view storagePath, bool stopOnFirstIncomplete) const {
-    ssize_t totalBlocks = 0, filledBlocks = 0;
-    const auto filePaths = mFs->listFilesRecursive(storagePath);
-    for (const auto& filePath : filePaths) {
+        const IncFsMount& ifs, std::string_view storagePath,
+        const bool stopOnFirstIncomplete) const {
+    ssize_t totalBlocks = 0, filledBlocks = 0, error = 0;
+    mFs->listFilesRecursive(storagePath, [&, this](auto filePath) {
         const auto [filledBlocksCount, totalBlocksCount] =
                 mIncFs->countFilledBlocks(ifs.control, filePath);
+        if (filledBlocksCount == -EOPNOTSUPP || filledBlocksCount == -ENOTSUP ||
+            filledBlocksCount == -ENOENT) {
+            // a kind of a file that's not really being loaded, e.g. a mapped range
+            // an older IncFS used to return ENOENT in this case, so handle it the same way
+            return true;
+        }
         if (filledBlocksCount < 0) {
             LOG(ERROR) << "getLoadingProgress failed to get filled blocks count for: " << filePath
                        << " errno: " << filledBlocksCount;
-            return {filledBlocksCount, filledBlocksCount};
+            error = filledBlocksCount;
+            return false;
         }
         totalBlocks += totalBlocksCount;
         filledBlocks += filledBlocksCount;
         if (stopOnFirstIncomplete && filledBlocks < totalBlocks) {
-            break;
+            return false;
         }
-    }
+        return true;
+    });
 
-    return {filledBlocks, totalBlocks};
+    return error ? LoadingProgress{error, error} : LoadingProgress{filledBlocks, totalBlocks};
 }
 
 bool IncrementalService::updateLoadingProgress(
diff --git a/services/incremental/ServiceWrappers.cpp b/services/incremental/ServiceWrappers.cpp
index 36bda49d278c..d61328942e5c 100644
--- a/services/incremental/ServiceWrappers.cpp
+++ b/services/incremental/ServiceWrappers.cpp
@@ -287,11 +287,10 @@ private:
             auto it = mJobs.begin();
             // Always acquire begin(). We can't use it after unlock as mTimedJobs can change.
             for (; it != mJobs.end() && it->when <= now; it = mJobs.begin()) {
-                auto job = std::move(it->what);
-                mJobs.erase(it);
+                auto jobNode = mJobs.extract(it);
 
                 lock.unlock();
-                job();
+                jobNode.value().what();
                 lock.lock();
             }
             nextJobTs = it != mJobs.end() ? it->when : kInfinityTs;
@@ -313,20 +312,20 @@ private:
     std::thread mThread;
 };
 
-class RealFsWrapper : public FsWrapper {
+class RealFsWrapper final : public FsWrapper {
 public:
     RealFsWrapper() = default;
     ~RealFsWrapper() = default;
 
-    std::vector<std::string> listFilesRecursive(std::string_view directoryPath) const final {
-        std::vector<std::string> files;
+    void listFilesRecursive(std::string_view directoryPath, FileCallback onFile) const final {
         for (const auto& entry : std::filesystem::recursive_directory_iterator(directoryPath)) {
             if (!entry.is_regular_file()) {
                 continue;
             }
-            files.push_back(entry.path().c_str());
+            if (!onFile(entry.path().native())) {
+                break;
+            }
         }
-        return files;
     }
 };
 
diff --git a/services/incremental/ServiceWrappers.h b/services/incremental/ServiceWrappers.h
index 46c0fc677261..245bb3105be5 100644
--- a/services/incremental/ServiceWrappers.h
+++ b/services/incremental/ServiceWrappers.h
@@ -16,6 +16,7 @@
 
 #pragma once
 
+#include <android-base/function_ref.h>
 #include <android-base/unique_fd.h>
 #include <android/content/pm/DataLoaderParamsParcel.h>
 #include <android/content/pm/FileSystemControlParcel.h>
@@ -79,9 +80,9 @@ public:
     using WaitResult = incfs::WaitResult;
     using Features = incfs::Features;
 
-    using ExistingMountCallback =
-            std::function<void(std::string_view root, std::string_view backingDir,
-                               std::span<std::pair<std::string_view, std::string_view>> binds)>;
+    using ExistingMountCallback = android::base::function_ref<
+            void(std::string_view root, std::string_view backingDir,
+                 std::span<std::pair<std::string_view, std::string_view>> binds)>;
 
     virtual ~IncFsWrapper() = default;
     virtual Features features() const = 0;
@@ -152,7 +153,9 @@ public:
 class FsWrapper {
 public:
     virtual ~FsWrapper() = default;
-    virtual std::vector<std::string> listFilesRecursive(std::string_view directoryPath) const = 0;
+
+    using FileCallback = android::base::function_ref<bool(std::string_view)>;
+    virtual void listFilesRecursive(std::string_view directoryPath, FileCallback onFile) const = 0;
 };
 
 class ServiceManagerWrapper {
diff --git a/services/incremental/test/IncrementalServiceTest.cpp b/services/incremental/test/IncrementalServiceTest.cpp
index 154a55fd8bd1..b00a84fcd003 100644
--- a/services/incremental/test/IncrementalServiceTest.cpp
+++ b/services/incremental/test/IncrementalServiceTest.cpp
@@ -543,16 +543,16 @@ public:
 
 class MockFsWrapper : public FsWrapper {
 public:
-    MOCK_CONST_METHOD1(listFilesRecursive, std::vector<std::string>(std::string_view));
-    void hasNoFile() {
-        ON_CALL(*this, listFilesRecursive(_)).WillByDefault(Return(std::vector<std::string>()));
-    }
+    MOCK_CONST_METHOD2(listFilesRecursive, void(std::string_view, FileCallback));
+    void hasNoFile() { ON_CALL(*this, listFilesRecursive(_, _)).WillByDefault(Return()); }
     void hasFiles() {
-        ON_CALL(*this, listFilesRecursive(_))
+        ON_CALL(*this, listFilesRecursive(_, _))
                 .WillByDefault(Invoke(this, &MockFsWrapper::fakeFiles));
     }
-    std::vector<std::string> fakeFiles(std::string_view directoryPath) {
-        return {"base.apk", "split.apk", "lib/a.so"};
+    void fakeFiles(std::string_view directoryPath, FileCallback onFile) {
+        for (auto file : {"base.apk", "split.apk", "lib/a.so"}) {
+            if (!onFile(file)) break;
+        }
     }
 };