diff --git a/engine/src/flutter/fml/file.cc b/engine/src/flutter/fml/file.cc index dc31a8cfd86..3d9e4397ee6 100644 --- a/engine/src/flutter/fml/file.cc +++ b/engine/src/flutter/fml/file.cc @@ -92,4 +92,29 @@ fml::UniqueFD OpenDirectoryReadOnly(const fml::UniqueFD& base_directory, 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/engine/src/flutter/fml/file.h b/engine/src/flutter/fml/file.h index bc82622036f..0b9ac97e23d 100644 --- a/engine/src/flutter/fml/file.h +++ b/engine/src/flutter/fml/file.h @@ -124,6 +124,19 @@ bool VisitFiles(const fml::UniqueFD& directory, const FileVisitor& visitor); 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/engine/src/flutter/shell/common/persistent_cache.cc b/engine/src/flutter/shell/common/persistent_cache.cc index bd1e26f1a50..993e8c73f26 100644 --- a/engine/src/flutter/shell/common/persistent_cache.cc +++ b/engine/src/flutter/shell/common/persistent_cache.cc @@ -79,6 +79,28 @@ void PersistentCache::SetCacheDirectoryPath(std::string path) { } 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 MakeCacheDirectory( const std::string& global_cache_base_path, bool read_only, @@ -92,8 +114,9 @@ static std::shared_ptr MakeCacheDirectory( } if (cache_base_dir.is_valid()) { + FreeOldCacheDirectory(cache_base_dir); std::vector components = { - "flutter_engine", GetFlutterEngineVersion(), "skia", GetSkiaVersion()}; + kEngineComponent, GetFlutterEngineVersion(), "skia", GetSkiaVersion()}; if (cache_sksl) { components.push_back(PersistentCache::kSkSLSubdirName); } diff --git a/engine/src/flutter/shell/common/persistent_cache_unittests.cc b/engine/src/flutter/shell/common/persistent_cache_unittests.cc index 8a9a477fbd6..8dc4a1e2a30 100644 --- a/engine/src/flutter/shell/common/persistent_cache_unittests.cc +++ b/engine/src/flutter/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 @@ TEST_F(ShellTest, CanLoadSkSLsFromAsset) { 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/engine/src/flutter/shell/common/shell_unittests.cc b/engine/src/flutter/shell/common/shell_unittests.cc index e96ab7c441b..589d593cd0f 100644 --- a/engine/src/flutter/shell/common/shell_unittests.cc +++ b/engine/src/flutter/shell/common/shell_unittests.cc @@ -1218,20 +1218,7 @@ TEST_F(ShellTest, OnServiceProtocolGetSkSLsWorks) { << 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/engine/src/flutter/testing/fuchsia/run_tests.sh b/engine/src/flutter/testing/fuchsia/run_tests.sh index 6a4f02e3963..05ffac4bd21 100755 --- a/engine/src/flutter/testing/fuchsia/run_tests.sh +++ b/engine/src/flutter/testing/fuchsia/run_tests.sh @@ -130,13 +130,13 @@ echo "$(date) DONE:flow_tests ---------------------------------------" # 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