Avoid reloading font collection for spawned engines with compatible asset managers (#50897)

@jason-simmons @jiahaog fyi

I need to figure out a test for this still but it seems to work. Hot restart is kind of a mess though, because `Engine::Restart` nulls out the asset manager it ends up reloading the font collection for every engine after a restart. But we separately need to fix hot restart, it's not very usable on the example app 

Fixes flutter/flutter#143701

Author: dnfield

assets/asset_manager.cc

@@ -111,4 +111,21 @@ AssetResolver::AssetResolverType AssetManager::GetType() const {
   return AssetResolverType::kAssetManager;
 }
 
+bool AssetManager::operator==(const AssetResolver& other) const {
+  const AssetManager* other_manager = other.as_asset_manager();
+  if (!other_manager) {
+    return false;
+  }
+  if (resolvers_.size() != other_manager->resolvers_.size()) {
+    return false;
+  }
+
+  for (size_t i = 0; i < resolvers_.size(); i++) {
+    if (*resolvers_[i] != *other_manager->resolvers_[i]) {
+      return false;
+    }
+  }
+  return true;
+}
+
 }  // namespace flutter

assets/asset_manager.h

@@ -88,6 +88,12 @@ class AssetManager final : public AssetResolver {
       const std::string& asset_pattern,
       const std::optional<std::string>& subdir) const override;
 
+  // |AssetResolver|
+  bool operator==(const AssetResolver& other) const override;
+
+  // |AssetResolver|
+  const AssetManager* as_asset_manager() const override { return this; }
+
  private:
   std::deque<std::unique_ptr<AssetResolver>> resolvers_;
 

assets/asset_resolver.h

@@ -14,6 +14,10 @@
 
 namespace flutter {
 
+class AssetManager;
+class APKAssetProvider;
+class DirectoryAssetBundle;
+
 class AssetResolver {
  public:
   AssetResolver() = default;
@@ -29,6 +33,14 @@ class AssetResolver {
     kDirectoryAssetBundle
   };
 
+  virtual const AssetManager* as_asset_manager() const { return nullptr; }
+  virtual const APKAssetProvider* as_apk_asset_provider() const {
+    return nullptr;
+  }
+  virtual const DirectoryAssetBundle* as_directory_asset_bundle() const {
+    return nullptr;
+  }
+
   virtual bool IsValid() const = 0;
 
   //----------------------------------------------------------------------------
@@ -81,6 +93,12 @@ class AssetResolver {
     return {};
   };
 
+  virtual bool operator==(const AssetResolver& other) const = 0;
+
+  bool operator!=(const AssetResolver& other) const {
+    return !operator==(other);
+  }
+
  private:
   FML_DISALLOW_COPY_AND_ASSIGN(AssetResolver);
 };

assets/directory_asset_bundle.cc

@@ -110,4 +110,14 @@ std::vector<std::unique_ptr<fml::Mapping>> DirectoryAssetBundle::GetAsMappings(
   return mappings;
 }
 
+bool DirectoryAssetBundle::operator==(const AssetResolver& other) const {
+  auto other_bundle = other.as_directory_asset_bundle();
+  if (!other_bundle) {
+    return false;
+  }
+  return is_valid_after_asset_manager_change_ ==
+             other_bundle->is_valid_after_asset_manager_change_ &&
+         descriptor_.get() == other_bundle->descriptor_.get();
+}
+
 }  // namespace flutter

assets/directory_asset_bundle.h

@@ -43,6 +43,14 @@ class DirectoryAssetBundle : public AssetResolver {
       const std::string& asset_pattern,
       const std::optional<std::string>& subdir) const override;
 
+  // |AssetResolver|
+  bool operator==(const AssetResolver& other) const override;
+
+  // |AssetResolver|
+  const DirectoryAssetBundle* as_directory_asset_bundle() const override {
+    return this;
+  }
+
   FML_DISALLOW_COPY_AND_ASSIGN(DirectoryAssetBundle);
 };
 

lib/ui/text/font_collection.h

@@ -20,13 +20,15 @@ class FontCollection {
  public:
   FontCollection();
 
-  ~FontCollection();
+  virtual ~FontCollection();
 
   std::shared_ptr<txt::FontCollection> GetFontCollection() const;
 
   void SetupDefaultFontManager(uint32_t font_initialization_data);
 
-  void RegisterFonts(const std::shared_ptr<AssetManager>& asset_manager);
+  // Virtual for testing.
+  virtual void RegisterFonts(
+      const std::shared_ptr<AssetManager>& asset_manager);
 
   void RegisterTestFonts();
 

shell/common/engine.cc

@@ -146,6 +146,7 @@ std::unique_ptr<Engine> Engine::Spawn(
       /*image_generator_registry=*/result->GetImageGeneratorRegistry(),
       /*snapshot_delegate=*/std::move(snapshot_delegate));
   result->initial_route_ = initial_route;
+  result->asset_manager_ = asset_manager_;
   return result;
 }
 
@@ -174,7 +175,8 @@ fml::WeakPtr<ImageGeneratorRegistry> Engine::GetImageGeneratorRegistry() {
 
 bool Engine::UpdateAssetManager(
     const std::shared_ptr<AssetManager>& new_asset_manager) {
-  if (asset_manager_ == new_asset_manager) {
+  if (asset_manager_ && new_asset_manager &&
+      *asset_manager_ == *new_asset_manager) {
     return false;
   }
 

shell/common/engine_unittests.cc

@@ -13,10 +13,14 @@
 #include "flutter/shell/common/thread_host.h"
 #include "flutter/testing/fixture_test.h"
 #include "flutter/testing/testing.h"
+#include "fml/mapping.h"
 #include "gmock/gmock.h"
+#include "lib/ui/text/font_collection.h"
 #include "rapidjson/document.h"
 #include "rapidjson/stringbuffer.h"
 #include "rapidjson/writer.h"
+#include "runtime/isolate_configuration.h"
+#include "shell/common/run_configuration.h"
 
 namespace flutter {
 
@@ -37,6 +41,41 @@ void PostSync(const fml::RefPtr<fml::TaskRunner>& task_runner,
   latch.Wait();
 }
 
+class FontManifestAssetResolver : public AssetResolver {
+ public:
+  FontManifestAssetResolver() {}
+
+  bool IsValid() const override { return true; }
+
+  bool IsValidAfterAssetManagerChange() const override { return true; }
+
+  AssetResolver::AssetResolverType GetType() const override {
+    return AssetResolver::AssetResolverType::kApkAssetProvider;
+  }
+
+  mutable size_t mapping_call_count = 0u;
+  std::unique_ptr<fml::Mapping> GetAsMapping(
+      const std::string& asset_name) const override {
+    mapping_call_count++;
+    if (asset_name == "FontManifest.json") {
+      return std::make_unique<fml::DataMapping>("[{},{},{}]");
+    }
+    return nullptr;
+  }
+
+  std::vector<std::unique_ptr<fml::Mapping>> GetAsMappings(
+      const std::string& asset_pattern,
+      const std::optional<std::string>& subdir) const override {
+    return {};
+  };
+
+  bool operator==(const AssetResolver& other) const override {
+    auto mapping = GetAsMapping("FontManifest.json");
+    return memcmp(other.GetAsMapping("FontManifest.json")->GetMapping(),
+                  mapping->GetMapping(), mapping->GetSize()) == 0;
+  }
+};
+
 class MockDelegate : public Engine::Delegate {
  public:
   MOCK_METHOD(void,
@@ -181,6 +220,14 @@ class MockPlatformMessageHandler : public PlatformMessageHandler {
               (override));
 };
 
+class MockFontCollection : public FontCollection {
+ public:
+  MOCK_METHOD(void,
+              RegisterFonts,
+              (const std::shared_ptr<AssetManager>& asset_manager),
+              (override));
+};
+
 std::unique_ptr<PlatformMessage> MakePlatformMessage(
     const std::string& channel,
     const std::map<std::string, std::string>& values,
@@ -697,6 +744,89 @@ TEST_F(EngineTest, AnimatorSubmitWarmUpImplicitView) {
   draw_latch.Wait();
 }
 
+TEST_F(EngineTest, SpawnedEngineInheritsAssetManager) {
+  PostUITaskSync([this] {
+    MockRuntimeDelegate client;
+    auto mock_runtime_controller =
+        std::make_unique<MockRuntimeController>(client, task_runners_);
+    auto vm_ref = DartVMRef::Create(settings_);
+    EXPECT_CALL(*mock_runtime_controller, GetDartVM())
+        .WillRepeatedly(::testing::Return(vm_ref.get()));
+
+    // auto mock_font_collection = std::make_shared<MockFontCollection>();
+    // EXPECT_CALL(*mock_font_collection, RegisterFonts(::testing::_))
+    //     .WillOnce(::testing::Return());
+    auto engine = std::make_unique<Engine>(
+        /*delegate=*/delegate_,
+        /*dispatcher_maker=*/dispatcher_maker_,
+        /*image_decoder_task_runner=*/image_decoder_task_runner_,
+        /*task_runners=*/task_runners_,
+        /*settings=*/settings_,
+        /*animator=*/std::move(animator_),
+        /*io_manager=*/io_manager_,
+        /*font_collection=*/std::make_shared<FontCollection>(),
+        /*runtime_controller=*/std::move(mock_runtime_controller),
+        /*gpu_disabled_switch=*/std::make_shared<fml::SyncSwitch>());
+
+    EXPECT_EQ(engine->GetAssetManager(), nullptr);
+
+    auto asset_manager = std::make_shared<AssetManager>();
+    asset_manager->PushBack(std::make_unique<FontManifestAssetResolver>());
+    engine->UpdateAssetManager(asset_manager);
+    EXPECT_EQ(engine->GetAssetManager(), asset_manager);
+
+    auto spawn =
+        engine->Spawn(delegate_, dispatcher_maker_, settings_, nullptr,
+                      std::string(), io_manager_, snapshot_delegate_, nullptr);
+    EXPECT_TRUE(spawn != nullptr);
+    EXPECT_EQ(engine->GetAssetManager(), spawn->GetAssetManager());
+  });
+}
+
+TEST_F(EngineTest, UpdateAssetManagerWithEqualManagers) {
+  PostUITaskSync([this] {
+    MockRuntimeDelegate client;
+    auto mock_runtime_controller =
+        std::make_unique<MockRuntimeController>(client, task_runners_);
+    auto vm_ref = DartVMRef::Create(settings_);
+    EXPECT_CALL(*mock_runtime_controller, GetDartVM())
+        .WillRepeatedly(::testing::Return(vm_ref.get()));
+
+    auto mock_font_collection = std::make_shared<MockFontCollection>();
+    EXPECT_CALL(*mock_font_collection, RegisterFonts(::testing::_))
+        .WillOnce(::testing::Return());
+    auto engine = std::make_unique<Engine>(
+        /*delegate=*/delegate_,
+        /*dispatcher_maker=*/dispatcher_maker_,
+        /*image_decoder_task_runner=*/image_decoder_task_runner_,
+        /*task_runners=*/task_runners_,
+        /*settings=*/settings_,
+        /*animator=*/std::move(animator_),
+        /*io_manager=*/io_manager_,
+        /*font_collection=*/mock_font_collection,
+        /*runtime_controller=*/std::move(mock_runtime_controller),
+        /*gpu_disabled_switch=*/std::make_shared<fml::SyncSwitch>());
+
+    EXPECT_EQ(engine->GetAssetManager(), nullptr);
+
+    auto asset_manager = std::make_shared<AssetManager>();
+    asset_manager->PushBack(std::make_unique<FontManifestAssetResolver>());
+
+    auto asset_manager_2 = std::make_shared<AssetManager>();
+    asset_manager_2->PushBack(std::make_unique<FontManifestAssetResolver>());
+
+    EXPECT_NE(asset_manager, asset_manager_2);
+    EXPECT_TRUE(*asset_manager == *asset_manager_2);
+
+    engine->UpdateAssetManager(asset_manager);
+    EXPECT_EQ(engine->GetAssetManager(), asset_manager);
+
+    engine->UpdateAssetManager(asset_manager_2);
+    // Didn't change because they're equivalent.
+    EXPECT_EQ(engine->GetAssetManager(), asset_manager);
+  });
+}
+
 // The warm up frame should work if only some of the registered views are
 // included.
 //

shell/common/shell_unittests.cc

@@ -3,7 +3,6 @@
 // found in the LICENSE file.
 
 #include <strstream>
-#include "impeller/core/runtime_types.h"
 #define FML_USED_ON_EMBEDDER
 
 #include <algorithm>
@@ -19,6 +18,7 @@
 #include <EGL/egl.h>
 #endif  // SHELL_ENABLE_GL
 
+#include "assets/asset_resolver.h"
 #include "assets/directory_asset_bundle.h"
 #include "common/graphics/persistent_cache.h"
 #include "flutter/flow/layers/backdrop_filter_layer.h"
@@ -47,6 +47,7 @@
 #include "flutter/testing/mock_canvas.h"
 #include "flutter/testing/testing.h"
 #include "gmock/gmock.h"
+#include "impeller/core/runtime_types.h"
 #include "third_party/rapidjson/include/rapidjson/writer.h"
 #include "third_party/skia/include/codec/SkCodecAnimation.h"
 #include "third_party/tonic/converter/dart_converter.h"
@@ -246,6 +247,10 @@ class TestAssetResolver : public AssetResolver {
     return {};
   };
 
+  bool operator==(const AssetResolver& other) const override {
+    return this == &other;
+  }
+
  private:
   bool valid_;
   AssetResolver::AssetResolverType type_;
@@ -283,6 +288,10 @@ class ThreadCheckingAssetResolver : public AssetResolver {
 
   mutable std::vector<std::string> mapping_requests;
 
+  bool operator==(const AssetResolver& other) const override {
+    return this == &other;
+  }
+
  private:
   std::shared_ptr<fml::ConcurrentMessageLoop> concurrent_loop_;
 };

shell/platform/android/apk_asset_provider.cc

@@ -10,6 +10,7 @@
 #include <sstream>
 #include <utility>
 
+#include "assets/asset_resolver.h"
 #include "flutter/fml/logging.h"
 
 namespace flutter {
@@ -103,4 +104,12 @@ std::unique_ptr<APKAssetProvider> APKAssetProvider::Clone() const {
   return std::make_unique<APKAssetProvider>(impl_);
 }
 
+bool APKAssetProvider::operator==(const AssetResolver& other) const {
+  auto other_provider = other.as_apk_asset_provider();
+  if (!other_provider) {
+    return false;
+  }
+  return impl_ == other_provider->impl_;
+}
+
 }  // namespace flutter