Automatically remove old persistent cache dir (#18244)
Fixes https://github.com/flutter/flutter/issues/53445
diff --git a/fml/file.cc b/fml/file.cc
index dc31a8c..3d9e439 100644
--- a/fml/file.cc
+++ b/fml/file.cc
@@ -92,4 +92,29 @@
return OpenDirectory(base_directory, path, false, FilePermission::kRead);
}
+bool RemoveFilesInDirectory(const fml::UniqueFD& directory) {
+ fml::FileVisitor recursive_cleanup = [&recursive_cleanup](
+ const fml::UniqueFD& directory,
+ const std::string& filename) {
+ bool removed;
+ if (fml::IsDirectory(directory, filename.c_str())) {
+ fml::UniqueFD sub_dir =
+ OpenDirectoryReadOnly(directory, filename.c_str());
+ removed = VisitFiles(sub_dir, recursive_cleanup) &&
+ fml::UnlinkDirectory(directory, filename.c_str());
+ } else {
+ removed = fml::UnlinkFile(directory, filename.c_str());
+ }
+ return removed;
+ };
+ return VisitFiles(directory, recursive_cleanup);
+}
+
+bool RemoveDirectoryRecursively(const fml::UniqueFD& parent,
+ const char* directory_name) {
+ auto dir = fml::OpenDirectory(parent, directory_name, false,
+ fml::FilePermission::kReadWrite);
+ return RemoveFilesInDirectory(dir) && UnlinkDirectory(parent, directory_name);
+}
+
} // namespace fml
diff --git a/fml/file.h b/fml/file.h
index bc82622..0b9ac97 100644
--- a/fml/file.h
+++ b/fml/file.h
@@ -124,6 +124,19 @@
bool VisitFilesRecursively(const fml::UniqueFD& directory,
const FileVisitor& visitor);
+/// Helper method to recursively remove files and subdirectories inside the
+/// directory. The directory itself will not be removed.
+///
+/// Return true if and only if all files have been successfully removed.
+bool RemoveFilesInDirectory(const fml::UniqueFD& directory);
+
+/// Helper method to recursively remove files and subdirectories inside the
+/// directory. The directory itself will also be removed.
+///
+/// Return true if and only if all files have been successfully removed.
+bool RemoveDirectoryRecursively(const fml::UniqueFD& parent,
+ const char* directory_name);
+
class ScopedTemporaryDirectory {
public:
ScopedTemporaryDirectory();
diff --git a/shell/common/persistent_cache.cc b/shell/common/persistent_cache.cc
index bd1e26f..993e8c7 100644
--- a/shell/common/persistent_cache.cc
+++ b/shell/common/persistent_cache.cc
@@ -79,6 +79,28 @@
}
namespace {
+
+constexpr char kEngineComponent[] = "flutter_engine";
+
+static void FreeOldCacheDirectory(const fml::UniqueFD& cache_base_dir) {
+ fml::UniqueFD engine_dir =
+ fml::OpenDirectoryReadOnly(cache_base_dir, kEngineComponent);
+ if (!engine_dir.is_valid()) {
+ return;
+ }
+ fml::VisitFiles(engine_dir, [](const fml::UniqueFD& directory,
+ const std::string& filename) {
+ if (filename != GetFlutterEngineVersion()) {
+ auto dir = fml::OpenDirectory(directory, filename.c_str(), false,
+ fml::FilePermission::kReadWrite);
+ if (dir.is_valid()) {
+ fml::RemoveDirectoryRecursively(directory, filename.c_str());
+ }
+ }
+ return true;
+ });
+}
+
static std::shared_ptr<fml::UniqueFD> MakeCacheDirectory(
const std::string& global_cache_base_path,
bool read_only,
@@ -92,8 +114,9 @@
}
if (cache_base_dir.is_valid()) {
+ FreeOldCacheDirectory(cache_base_dir);
std::vector<std::string> components = {
- "flutter_engine", GetFlutterEngineVersion(), "skia", GetSkiaVersion()};
+ kEngineComponent, GetFlutterEngineVersion(), "skia", GetSkiaVersion()};
if (cache_sksl) {
components.push_back(PersistentCache::kSkSLSubdirName);
}
diff --git a/shell/common/persistent_cache_unittests.cc b/shell/common/persistent_cache_unittests.cc
index 8a9a477..8dc4a1e 100644
--- a/shell/common/persistent_cache_unittests.cc
+++ b/shell/common/persistent_cache_unittests.cc
@@ -16,6 +16,7 @@
#include "flutter/shell/common/persistent_cache.h"
#include "flutter/shell/common/shell_test.h"
#include "flutter/shell/common/switches.h"
+#include "flutter/shell/version/version.h"
#include "flutter/testing/testing.h"
#include "include/core/SkPicture.h"
@@ -204,5 +205,35 @@
fml::UnlinkFile(asset_dir.fd(), PersistentCache::kAssetFileName);
}
+TEST_F(ShellTest, CanRemoveOldPersistentCache) {
+ fml::ScopedTemporaryDirectory base_dir;
+ ASSERT_TRUE(base_dir.fd().is_valid());
+
+ fml::CreateDirectory(base_dir.fd(),
+ {"flutter_engine", GetFlutterEngineVersion(), "skia"},
+ fml::FilePermission::kReadWrite);
+
+ constexpr char kOldEngineVersion[] = "old";
+ auto old_created = fml::CreateDirectory(
+ base_dir.fd(), {"flutter_engine", kOldEngineVersion, "skia"},
+ fml::FilePermission::kReadWrite);
+ ASSERT_TRUE(old_created.is_valid());
+
+ PersistentCache::SetCacheDirectoryPath(base_dir.path());
+ PersistentCache::ResetCacheForProcess();
+
+ auto engine_dir = fml::OpenDirectoryReadOnly(base_dir.fd(), "flutter_engine");
+ auto current_dir =
+ fml::OpenDirectoryReadOnly(engine_dir, GetFlutterEngineVersion());
+ auto old_dir = fml::OpenDirectoryReadOnly(engine_dir, kOldEngineVersion);
+
+ ASSERT_TRUE(engine_dir.is_valid());
+ ASSERT_TRUE(current_dir.is_valid());
+ ASSERT_FALSE(old_dir.is_valid());
+
+ // Cleanup
+ fml::RemoveFilesInDirectory(base_dir.fd());
+}
+
} // namespace testing
} // namespace flutter
diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc
index e96ab7c..589d593 100644
--- a/shell/common/shell_unittests.cc
+++ b/shell/common/shell_unittests.cc
@@ -1218,20 +1218,7 @@
<< expected_json1 << " or " << expected_json2;
// Cleanup files
- fml::FileVisitor recursive_cleanup = [&recursive_cleanup](
- const fml::UniqueFD& directory,
- const std::string& filename) {
- if (fml::IsDirectory(directory, filename.c_str())) {
- fml::UniqueFD sub_dir =
- OpenDirectoryReadOnly(directory, filename.c_str());
- VisitFiles(sub_dir, recursive_cleanup);
- fml::UnlinkDirectory(directory, filename.c_str());
- } else {
- fml::UnlinkFile(directory, filename.c_str());
- }
- return true;
- };
- VisitFiles(temp_dir.fd(), recursive_cleanup);
+ fml::RemoveFilesInDirectory(temp_dir.fd());
}
} // namespace testing
diff --git a/testing/fuchsia/run_tests.sh b/testing/fuchsia/run_tests.sh
index 6a4f02e..05ffac4 100755
--- a/testing/fuchsia/run_tests.sh
+++ b/testing/fuchsia/run_tests.sh
@@ -130,13 +130,13 @@
# TODO(https://github.com/flutter/flutter/issues/53399): Re-enable
-# OnServiceProtocolGetSkSLsWorks and CanLoadSkSLsFromAsset once they pass on
-# Fuchsia.
+# OnServiceProtocolGetSkSLsWorks, CanLoadSkSLsFromAsset, and
+# CanRemoveOldPersistentCache once they pass on Fuchsia.
# echo "$(date) START:shell_tests -------------------------------------"
# ./fuchsia_ctl -d $device_name test \
# -f shell_tests-0.far \
# -t shell_tests \
-# -a "--gtest_filter=-ShellTest.CacheSkSLWorks:ShellTest.SetResourceCacheSize*:ShellTest.OnServiceProtocolGetSkSLsWorks:ShellTest.CanLoadSkSLsFromAsset" \
+# -a "--gtest_filter=-ShellTest.CacheSkSLWorks:ShellTest.SetResourceCacheSize*:ShellTest.OnServiceProtocolGetSkSLsWorks:ShellTest.CanLoadSkSLsFromAsset:ShellTest.CanRemoveOldPersistentCache" \
# --identity-file $pkey \
# --timeout-seconds $test_timeout_seconds \
# --packages-directory packages
