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)
diff --git a/services/incremental/IncrementalService.cpp b/services/incremental/IncrementalService.cpp
index 24623b2..ce6e6ab 100644
--- a/services/incremental/IncrementalService.cpp
+++ b/services/incremental/IncrementalService.cpp
@@ -1936,25 +1936,33 @@
}
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 36bda49..d613289 100644
--- a/services/incremental/ServiceWrappers.cpp
+++ b/services/incremental/ServiceWrappers.cpp
@@ -287,11 +287,10 @@
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 @@
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 46c0fc6..245bb31 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 @@
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 @@
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 154a55f..b00a84f 100644
--- a/services/incremental/test/IncrementalServiceTest.cpp
+++ b/services/incremental/test/IncrementalServiceTest.cpp
@@ -543,16 +543,16 @@
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;
+ }
}
};