Avoid reloading font collection for spawned engines with compatible asset managers (flutter/engine#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 https://github.com/flutter/flutter/issues/143701
This commit is contained in:
Dan Field
2024-02-24 06:58:37 -08:00
committed by GitHub
parent 283f15aa36
commit f235a65897
12 changed files with 225 additions and 5 deletions

View File

@@ -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

View File

@@ -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_;

View File

@@ -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);
};

View File

@@ -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

View File

@@ -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);
};

View File

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

View File

@@ -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;
}

View File

@@ -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.
//

View File

@@ -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_;
};

View File

@@ -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

View File

@@ -43,6 +43,8 @@ class APKAssetProvider final : public AssetResolver {
// delete the returned pointer.
APKAssetProviderInternal* GetImpl() const { return impl_.get(); }
bool operator==(const AssetResolver& other) const override;
private:
std::shared_ptr<APKAssetProviderInternal> impl_;
@@ -59,6 +61,11 @@ class APKAssetProvider final : public AssetResolver {
std::unique_ptr<fml::Mapping> GetAsMapping(
const std::string& asset_name) const override;
// |AssetResolver|
const APKAssetProvider* as_apk_asset_provider() const override {
return this;
}
FML_DISALLOW_COPY_AND_ASSIGN(APKAssetProvider);
};

View File

@@ -12,7 +12,7 @@ class MockAPKAssetProviderImpl : public APKAssetProviderInternal {
(const, override));
};
TEST(APKAssetProvider, Clone) {
TEST(APKAssetProvider, CloneAndEquals) {
auto first_provider = std::make_unique<APKAssetProvider>(
std::make_shared<MockAPKAssetProviderImpl>());
auto second_provider = std::make_unique<APKAssetProvider>(
@@ -21,6 +21,8 @@ TEST(APKAssetProvider, Clone) {
ASSERT_NE(first_provider->GetImpl(), second_provider->GetImpl());
ASSERT_EQ(first_provider->GetImpl(), third_provider->GetImpl());
ASSERT_FALSE(*first_provider == *second_provider);
ASSERT_TRUE(*first_provider == *third_provider);
}
} // namespace testing
} // namespace flutter