Skip to content

Commit

Permalink
Fix issue about multi-engine loading assets throws exception after en…
Browse files Browse the repository at this point in the history
…gines restart (flutter#34473)
  • Loading branch information
ColdPaleLight committed Jul 8, 2022
1 parent f34a228 commit e6b17e0
Show file tree
Hide file tree
Showing 8 changed files with 130 additions and 58 deletions.
1 change: 1 addition & 0 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -1356,6 +1356,7 @@ FILE: ../../../flutter/shell/platform/android/android_surface_software.cc
FILE: ../../../flutter/shell/platform/android/android_surface_software.h
FILE: ../../../flutter/shell/platform/android/apk_asset_provider.cc
FILE: ../../../flutter/shell/platform/android/apk_asset_provider.h
FILE: ../../../flutter/shell/platform/android/apk_asset_provider_unittests.cc
FILE: ../../../flutter/shell/platform/android/context/android_context.cc
FILE: ../../../flutter/shell/platform/android/context/android_context.h
FILE: ../../../flutter/shell/platform/android/external_view_embedder/external_view_embedder.cc
Expand Down
1 change: 1 addition & 0 deletions shell/platform/android/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ executable("flutter_shell_native_unittests") {
sources = [
"android_context_gl_unittests.cc",
"android_shell_holder_unittests.cc",
"apk_asset_provider_unittests.cc",
"flutter_shell_native_unittests.cc",
]
public_configs = [ "//flutter:config" ]
Expand Down
15 changes: 6 additions & 9 deletions shell/platform/android/android_shell_holder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -259,8 +259,7 @@ std::unique_ptr<AndroidShellHolder> AndroidShellHolder::Spawn(

// TODO(xster): could be worth tracing this to investigate whether
// the IsolateConfiguration could be cached somewhere.
auto config = BuildRunConfiguration(asset_manager_, entrypoint, libraryUrl,
entrypoint_args);
auto config = BuildRunConfiguration(entrypoint, libraryUrl, entrypoint_args);
if (!config) {
// If the RunConfiguration was null, the kernel blob wasn't readable.
// Fail the whole thing.
Expand All @@ -277,17 +276,16 @@ std::unique_ptr<AndroidShellHolder> AndroidShellHolder::Spawn(
}

void AndroidShellHolder::Launch(
std::shared_ptr<AssetManager> asset_manager,
std::unique_ptr<APKAssetProvider> apk_asset_provider,
const std::string& entrypoint,
const std::string& libraryUrl,
const std::vector<std::string>& entrypoint_args) {
if (!IsValid()) {
return;
}

asset_manager_ = asset_manager;
auto config = BuildRunConfiguration(asset_manager, entrypoint, libraryUrl,
entrypoint_args);
apk_asset_provider_ = std::move(apk_asset_provider);
auto config = BuildRunConfiguration(entrypoint, libraryUrl, entrypoint_args);
if (!config) {
return;
}
Expand All @@ -314,7 +312,6 @@ void AndroidShellHolder::NotifyLowMemoryWarning() {
}

std::optional<RunConfiguration> AndroidShellHolder::BuildRunConfiguration(
std::shared_ptr<flutter::AssetManager> asset_manager,
const std::string& entrypoint,
const std::string& libraryUrl,
const std::vector<std::string>& entrypoint_args) const {
Expand All @@ -333,8 +330,8 @@ std::optional<RunConfiguration> AndroidShellHolder::BuildRunConfiguration(
IsolateConfiguration::CreateForKernel(std::move(kernel_blob));
}

RunConfiguration config(std::move(isolate_configuration),
std::move(asset_manager));
RunConfiguration config(std::move(isolate_configuration));
config.AddAssetResolver(apk_asset_provider_->Clone());

{
if (!entrypoint.empty() && !libraryUrl.empty()) {
Expand Down
8 changes: 3 additions & 5 deletions shell/platform/android/android_shell_holder.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "flutter/shell/common/run_configuration.h"
#include "flutter/shell/common/shell.h"
#include "flutter/shell/common/thread_host.h"
#include "flutter/shell/platform/android/apk_asset_provider.h"
#include "flutter/shell/platform/android/jni/platform_view_android_jni.h"
#include "flutter/shell/platform/android/platform_message_handler_android.h"
#include "flutter/shell/platform/android/platform_view_android.h"
Expand Down Expand Up @@ -83,7 +84,7 @@ class AndroidShellHolder {
const std::string& initial_route,
const std::vector<std::string>& entrypoint_args) const;

void Launch(std::shared_ptr<AssetManager> asset_manager,
void Launch(std::unique_ptr<APKAssetProvider> apk_asset_provider,
const std::string& entrypoint,
const std::string& libraryUrl,
const std::vector<std::string>& entrypoint_args);
Expand All @@ -95,8 +96,6 @@ class AndroidShellHolder {
Rasterizer::Screenshot Screenshot(Rasterizer::ScreenshotType type,
bool base64_encode);

void UpdateAssetManager(fml::RefPtr<flutter::AssetManager> asset_manager);

void NotifyLowMemoryWarning();

const std::shared_ptr<PlatformMessageHandler>& GetPlatformMessageHandler()
Expand All @@ -112,7 +111,7 @@ class AndroidShellHolder {
std::unique_ptr<Shell> shell_;
bool is_valid_ = false;
uint64_t next_pointer_flow_id_ = 0;
std::shared_ptr<AssetManager> asset_manager_;
std::unique_ptr<APKAssetProvider> apk_asset_provider_;

//----------------------------------------------------------------------------
/// @brief Constructor with its components injected.
Expand All @@ -132,7 +131,6 @@ class AndroidShellHolder {
const fml::WeakPtr<PlatformViewAndroid>& platform_view);
static void ThreadDestructCallback(void* value);
std::optional<RunConfiguration> BuildRunConfiguration(
std::shared_ptr<flutter::AssetManager> asset_manager,
const std::string& entrypoint,
const std::string& libraryUrl,
const std::vector<std::string>& entrypoint_args) const;
Expand Down
95 changes: 64 additions & 31 deletions shell/platform/android/apk_asset_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,29 +13,6 @@

namespace flutter {

APKAssetProvider::APKAssetProvider(JNIEnv* env,
jobject jassetManager,
std::string directory)
: java_asset_manager_(env, jassetManager),
directory_(std::move(directory)) {
assetManager_ = AAssetManager_fromJava(env, jassetManager);
}

APKAssetProvider::~APKAssetProvider() = default;

bool APKAssetProvider::IsValid() const {
return true;
}

bool APKAssetProvider::IsValidAfterAssetManagerChange() const {
return true;
}

// |AssetResolver|
AssetResolver::AssetResolverType APKAssetProvider::GetType() const {
return AssetResolver::AssetResolverType::kApkAssetProvider;
}

class APKAssetMapping : public fml::Mapping {
public:
explicit APKAssetMapping(AAsset* asset) : asset_(asset) {}
Expand All @@ -56,17 +33,73 @@ class APKAssetMapping : public fml::Mapping {
FML_DISALLOW_COPY_AND_ASSIGN(APKAssetMapping);
};

class APKAssetProviderImpl : public APKAssetProviderInternal {
public:
explicit APKAssetProviderImpl(JNIEnv* env,
jobject jassetManager,
std::string directory)
: java_asset_manager_(env, jassetManager),
directory_(std::move(directory)) {
asset_manager_ = AAssetManager_fromJava(env, jassetManager);
}

~APKAssetProviderImpl() = default;

std::unique_ptr<fml::Mapping> GetAsMapping(
const std::string& asset_name) const override {
std::stringstream ss;
ss << directory_.c_str() << "/" << asset_name;
AAsset* asset = AAssetManager_open(asset_manager_, ss.str().c_str(),
AASSET_MODE_BUFFER);
if (!asset) {
return nullptr;
}

return std::make_unique<APKAssetMapping>(asset);
};

private:
fml::jni::ScopedJavaGlobalRef<jobject> java_asset_manager_;
AAssetManager* asset_manager_;
const std::string directory_;

FML_DISALLOW_COPY_AND_ASSIGN(APKAssetProviderImpl);
};

APKAssetProvider::APKAssetProvider(JNIEnv* env,
jobject assetManager,
std::string directory)
: impl_(std::make_shared<APKAssetProviderImpl>(env,
assetManager,
std::move(directory))) {}

APKAssetProvider::APKAssetProvider(
std::shared_ptr<APKAssetProviderInternal> impl)
: impl_(impl) {}

// |AssetResolver|
bool APKAssetProvider::IsValid() const {
return true;
}

// |AssetResolver|
bool APKAssetProvider::IsValidAfterAssetManagerChange() const {
return true;
}

// |AssetResolver|
AssetResolver::AssetResolverType APKAssetProvider::GetType() const {
return AssetResolver::AssetResolverType::kApkAssetProvider;
}

// |AssetResolver|
std::unique_ptr<fml::Mapping> APKAssetProvider::GetAsMapping(
const std::string& asset_name) const {
std::stringstream ss;
ss << directory_.c_str() << "/" << asset_name;
AAsset* asset =
AAssetManager_open(assetManager_, ss.str().c_str(), AASSET_MODE_BUFFER);
if (!asset) {
return nullptr;
}
return impl_->GetAsMapping(asset_name);
}

return std::make_unique<APKAssetMapping>(asset);
std::unique_ptr<APKAssetProvider> APKAssetProvider::Clone() const {
return std::make_unique<APKAssetProvider>(impl_);
}

} // namespace flutter
28 changes: 24 additions & 4 deletions shell/platform/android/apk_asset_provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,37 @@

namespace flutter {

class APKAssetProviderInternal {
public:
virtual std::unique_ptr<fml::Mapping> GetAsMapping(
const std::string& asset_name) const = 0;

protected:
virtual ~APKAssetProviderInternal() = default;
};

class APKAssetProvider final : public AssetResolver {
public:
explicit APKAssetProvider(JNIEnv* env,
jobject assetManager,
std::string directory);
~APKAssetProvider() override;

explicit APKAssetProvider(std::shared_ptr<APKAssetProviderInternal> impl);

~APKAssetProvider() = default;

// Returns a new 'std::unique_ptr<APKAssetProvider>' with the same 'impl_' as
// this provider.
std::unique_ptr<APKAssetProvider> Clone() const;

// Obtain a raw pointer to the APKAssetProviderInternal.
//
// This method is intended for use in tests. Callers must not
// delete the returned pointer.
APKAssetProviderInternal* GetImpl() const { return impl_.get(); }

private:
fml::jni::ScopedJavaGlobalRef<jobject> java_asset_manager_;
AAssetManager* assetManager_;
const std::string directory_;
std::shared_ptr<APKAssetProviderInternal> impl_;

// |flutter::AssetResolver|
bool IsValid() const override;
Expand Down
25 changes: 25 additions & 0 deletions shell/platform/android/apk_asset_provider_unittests.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
#include "flutter/shell/platform/android/apk_asset_provider.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"

namespace flutter {
namespace testing {
class MockAPKAssetProviderImpl : public APKAssetProviderInternal {
public:
MOCK_CONST_METHOD1(
GetAsMapping,
std::unique_ptr<fml::Mapping>(const std::string& asset_name));
};

TEST(APKAssetProvider, Clone) {
auto first_provider = std::make_unique<APKAssetProvider>(
std::make_shared<MockAPKAssetProviderImpl>());
auto second_provider = std::make_unique<APKAssetProvider>(
std::make_shared<MockAPKAssetProviderImpl>());
auto third_provider = first_provider->Clone();

ASSERT_NE(first_provider->GetImpl(), second_provider->GetImpl());
ASSERT_EQ(first_provider->GetImpl(), third_provider->GetImpl());
}
} // namespace testing
} // namespace flutter
15 changes: 6 additions & 9 deletions shell/platform/android/platform_view_android_jni_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -241,20 +241,17 @@ static void RunBundleAndSnapshotFromLibrary(JNIEnv* env,
jstring jLibraryUrl,
jobject jAssetManager,
jobject jEntrypointArgs) {
auto asset_manager = std::make_shared<flutter::AssetManager>();

asset_manager->PushBack(std::make_unique<flutter::APKAssetProvider>(
env, // jni environment
jAssetManager, // asset manager
fml::jni::JavaStringToString(env, jBundlePath)) // apk asset dir
auto apk_asset_provider = std::make_unique<flutter::APKAssetProvider>(
env, // jni environment
jAssetManager, // asset manager
fml::jni::JavaStringToString(env, jBundlePath) // apk asset dir
);

auto entrypoint = fml::jni::JavaStringToString(env, jEntrypoint);
auto libraryUrl = fml::jni::JavaStringToString(env, jLibraryUrl);
auto entrypoint_args = fml::jni::StringListToVector(env, jEntrypointArgs);

ANDROID_SHELL_HOLDER->Launch(asset_manager, entrypoint, libraryUrl,
entrypoint_args);
ANDROID_SHELL_HOLDER->Launch(std::move(apk_asset_provider), entrypoint,
libraryUrl, entrypoint_args);
}

static jobject LookupCallbackInformation(JNIEnv* env,
Expand Down

0 comments on commit e6b17e0

Please sign in to comment.