Set SkSL asset manager in RunConfiguration ctor (flutter#17948)

shell/common/BUILD.gn

@@ -204,6 +204,7 @@ if (enable_unittests) {
     deps = [
       ":shell_unittests_fixtures",
       ":shell_unittests_gpu_configuration",
+      "//flutter/assets",
       "//flutter/common",
       "//flutter/flow",
       "//flutter/fml/dart",

shell/common/persistent_cache.cc

@@ -23,7 +23,8 @@
 namespace flutter {
 
 std::string PersistentCache::cache_base_path_;
-std::string PersistentCache::asset_path_;
+
+std::shared_ptr<AssetManager> PersistentCache::asset_manager_;
 
 std::mutex PersistentCache::instance_mutex_;
 std::unique_ptr<PersistentCache> PersistentCache::gPersistentCache;
@@ -149,16 +150,14 @@ std::vector<PersistentCache::SkSLCache> PersistentCache::LoadSkSLs() {
     fml::VisitFiles(*sksl_cache_directory_, visitor);
   }
 
-  fml::UniqueFD root_asset_dir = fml::OpenDirectory(asset_path_.c_str(), false,
-                                                    fml::FilePermission::kRead);
-  fml::UniqueFD sksl_asset_dir =
-      fml::OpenDirectoryReadOnly(root_asset_dir, kSkSLSubdirName);
-  auto sksl_asset_file = fml::OpenFileReadOnly(sksl_asset_dir, kAssetFileName);
-  if (!sksl_asset_file.is_valid()) {
-    FML_LOG(INFO) << "No sksl asset file found.";
+  std::unique_ptr<fml::Mapping> mapping = nullptr;
+  if (asset_manager_ != nullptr) {
+    mapping = asset_manager_->GetAsMapping(kAssetFileName);
+  }
+  if (mapping == nullptr) {
+    FML_LOG(INFO) << "No sksl asset found.";
   } else {
     FML_LOG(INFO) << "Found sksl asset. Loading SkSLs from it...";
-    auto mapping = std::make_unique<fml::FileMapping>(sksl_asset_file);
     rapidjson::Document json_doc;
     rapidjson::ParseResult parse_result =
         json_doc.Parse(reinterpret_cast<const char*>(mapping->GetMapping()),
@@ -334,9 +333,9 @@ fml::RefPtr<fml::TaskRunner> PersistentCache::GetWorkerTaskRunner() const {
   return worker;
 }
 
-void PersistentCache::UpdateAssetPath(const std::string& path) {
-  FML_LOG(INFO) << "PersistentCache::UpdateAssetPath: " << path;
-  asset_path_ = path;
+void PersistentCache::SetAssetManager(std::shared_ptr<AssetManager> value) {
+  TRACE_EVENT_INSTANT0("flutter", "PersistentCache::SetAssetManager");
+  asset_manager_ = value;
 }
 
 }  // namespace flutter

shell/common/persistent_cache.h

@@ -9,6 +9,7 @@
 #include <mutex>
 #include <set>
 
+#include "flutter/assets/asset_manager.h"
 #include "flutter/fml/macros.h"
 #include "flutter/fml/task_runner.h"
 #include "flutter/fml/unique_fd.h"
@@ -65,8 +66,9 @@ class PersistentCache : public GrContextOptions::PersistentCache {
   /// Load all the SkSL shader caches in the right directory.
   std::vector<SkSLCache> LoadSkSLs();
 
-  /// Update the asset path from which PersistentCache can load SkLSs.
-  static void UpdateAssetPath(const std::string& path);
+  /// Set the asset manager from which PersistentCache can load SkLSs. A nullptr
+  /// can be provided to clear the asset manager.
+  static void SetAssetManager(std::shared_ptr<AssetManager> value);
 
   static bool cache_sksl() { return cache_sksl_; }
   static void SetCacheSkSL(bool value);
@@ -77,7 +79,8 @@ class PersistentCache : public GrContextOptions::PersistentCache {
 
  private:
   static std::string cache_base_path_;
-  static std::string asset_path_;
+
+  static std::shared_ptr<AssetManager> asset_manager_;
 
   static std::mutex instance_mutex_;
   static std::unique_ptr<PersistentCache> gPersistentCache;

shell/common/persistent_cache_unittests.cc

@@ -4,6 +4,7 @@
 
 #include <memory>
 
+#include "flutter/assets/directory_asset_bundle.h"
 #include "flutter/flow/layers/container_layer.h"
 #include "flutter/flow/layers/layer.h"
 #include "flutter/flow/layers/physical_shape_layer.h"
@@ -130,12 +131,12 @@ static void CheckTextSkData(sk_sp<SkData> data, const std::string& expected) {
   ASSERT_EQ(data_string, expected);
 }
 
-void ResetAssetPath() {
-  PersistentCache::UpdateAssetPath("some_path_that_does_not_exist");
+static void ResetAssetManager() {
+  PersistentCache::SetAssetManager(nullptr);
   ASSERT_EQ(PersistentCache::GetCacheForProcess()->LoadSkSLs().size(), 0u);
 }
 
-void CheckTwoSkSLsAreLoaded() {
+static void CheckTwoSkSLsAreLoaded() {
   auto shaders = PersistentCache::GetCacheForProcess()->LoadSkSLs();
   ASSERT_EQ(shaders.size(), 2u);
 }
@@ -145,11 +146,6 @@ TEST_F(ShellTest, CanLoadSkSLsFromAsset) {
   fml::LogSettings warning_only = {fml::LOG_WARNING};
   fml::ScopedSetLogSettings scoped_set_log_settings(warning_only);
 
-  // Create an empty shell to test its service protocol handlers.
-  auto empty_settings = CreateSettingsForFixture();
-  auto empty_config = RunConfiguration::InferFromSettings(empty_settings);
-  std::unique_ptr<Shell> empty_shell = CreateShell(empty_settings);
-
   // The SkSL key is Base32 encoded. "IE" is the encoding of "A" and "II" is the
   // encoding of "B".
   //
@@ -165,41 +161,29 @@ TEST_F(ShellTest, CanLoadSkSLsFromAsset) {
 
   // Temp dir for the asset.
   fml::ScopedTemporaryDirectory asset_dir;
-  fml::UniqueFD sksl_asset_dir =
-      fml::OpenDirectory(asset_dir.fd(), PersistentCache::kSkSLSubdirName, true,
-                         fml::FilePermission::kReadWrite);
 
   auto data = std::make_unique<fml::DataMapping>(
       std::vector<uint8_t>{kTestJson.begin(), kTestJson.end()});
-  fml::WriteAtomically(sksl_asset_dir, PersistentCache::kAssetFileName, *data);
+  fml::WriteAtomically(asset_dir.fd(), PersistentCache::kAssetFileName, *data);
 
-  // 1st, test that RunConfiguration::InferFromSettings sets the path.
-  ResetAssetPath();
+  // 1st, test that RunConfiguration::InferFromSettings sets the asset manager.
+  ResetAssetManager();
   auto settings = CreateSettingsForFixture();
   settings.assets_path = asset_dir.path();
   RunConfiguration::InferFromSettings(settings);
   CheckTwoSkSLsAreLoaded();
 
-  // 2nd, test that Shell::OnServiceProtocolSetAssetBundlePath sets the path.
-  ResetAssetPath();
-  ServiceProtocol::Handler::ServiceProtocolMap params;
-  rapidjson::Document document;
-  params["assetDirectory"] = asset_dir.path();
-  OnServiceProtocol(
-      empty_shell.get(), ShellTest::ServiceProtocolEnum::kSetAssetBundlePath,
-      empty_shell->GetTaskRunners().GetUITaskRunner(), params, document);
-  CheckTwoSkSLsAreLoaded();
-
-  // 3rd, test that Shell::OnServiceProtocolRunInView sets the path.
-  ResetAssetPath();
-  params["assetDirectory"] = asset_dir.path();
-  params["mainScript"] = "no_such_script.dart";
-  OnServiceProtocol(
-      empty_shell.get(), ShellTest::ServiceProtocolEnum::kSetAssetBundlePath,
-      empty_shell->GetTaskRunners().GetUITaskRunner(), params, document);
+  // 2nd, test that the RunConfiguration constructor sets the asset manager.
+  // (Android is directly calling that constructor without InferFromSettings.)
+  ResetAssetManager();
+  auto asset_manager = std::make_shared<AssetManager>();
+  RunConfiguration config(nullptr, asset_manager);
+  asset_manager->PushBack(
+      std::make_unique<DirectoryAssetBundle>(fml::OpenDirectory(
+          asset_dir.path().c_str(), false, fml::FilePermission::kRead)));
   CheckTwoSkSLsAreLoaded();
 
-  // 4th, test the content of the SkSLs in the asset.
+  // 3rd, test the content of the SkSLs in the asset.
   {
     auto shaders = PersistentCache::GetCacheForProcess()->LoadSkSLs();
     ASSERT_EQ(shaders.size(), 2u);
@@ -217,9 +201,7 @@ TEST_F(ShellTest, CanLoadSkSLsFromAsset) {
   }
 
   // Cleanup.
-  DestroyShell(std::move(empty_shell));
-  fml::UnlinkFile(sksl_asset_dir, PersistentCache::kAssetFileName);
-  fml::UnlinkDirectory(asset_dir.fd(), PersistentCache::kSkSLSubdirName);
+  fml::UnlinkFile(asset_dir.fd(), PersistentCache::kAssetFileName);
 }
 
 }  // namespace testing

shell/common/run_configuration.cc

@@ -27,7 +27,6 @@ RunConfiguration RunConfiguration::InferFromSettings(
   asset_manager->PushBack(
       std::make_unique<DirectoryAssetBundle>(fml::OpenDirectory(
           settings.assets_path.c_str(), false, fml::FilePermission::kRead)));
-  PersistentCache::UpdateAssetPath(settings.assets_path);
 
   return {IsolateConfiguration::InferFromSettings(settings, asset_manager,
                                                   io_worker),
@@ -37,13 +36,17 @@ RunConfiguration RunConfiguration::InferFromSettings(
 RunConfiguration::RunConfiguration(
     std::unique_ptr<IsolateConfiguration> configuration)
     : RunConfiguration(std::move(configuration),
-                       std::make_shared<AssetManager>()) {}
+                       std::make_shared<AssetManager>()) {
+  PersistentCache::SetAssetManager(asset_manager_);
+}
 
 RunConfiguration::RunConfiguration(
     std::unique_ptr<IsolateConfiguration> configuration,
     std::shared_ptr<AssetManager> asset_manager)
     : isolate_configuration_(std::move(configuration)),
-      asset_manager_(std::move(asset_manager)) {}
+      asset_manager_(std::move(asset_manager)) {
+  PersistentCache::SetAssetManager(asset_manager_);
+}
 
 RunConfiguration::RunConfiguration(RunConfiguration&&) = default;
 

shell/common/shell.cc

@@ -1312,7 +1312,6 @@ bool Shell::OnServiceProtocolRunInView(
   configuration.AddAssetResolver(
       std::make_unique<DirectoryAssetBundle>(fml::OpenDirectory(
           asset_directory_path.c_str(), false, fml::FilePermission::kRead)));
-  PersistentCache::UpdateAssetPath(asset_directory_path);
 
   auto& allocator = response.GetAllocator();
   response.SetObject();
@@ -1407,7 +1406,6 @@ bool Shell::OnServiceProtocolSetAssetBundlePath(
   asset_manager->PushFront(std::make_unique<DirectoryAssetBundle>(
       fml::OpenDirectory(params.at("assetDirectory").data(), false,
                          fml::FilePermission::kRead)));
-  PersistentCache::UpdateAssetPath(params.at("assetDirectory").data());
 
   if (engine_->UpdateAssetManager(std::move(asset_manager))) {
     response.AddMember("type", "Success", allocator);