Automatically remove old persistent cache dir (flutter/engine#18244)

Fixes https://github.com/flutter/flutter/issues/53445

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

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();

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<fml::UniqueFD> MakeCacheDirectory(
     const std::string& global_cache_base_path,
     bool read_only,
@@ -92,8 +114,9 @@ static std::shared_ptr<fml::UniqueFD> MakeCacheDirectory(
   }
 
   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);
     }

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

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

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