Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@liyuqian
Copy link
Contributor

@liyuqian liyuqian commented Apr 25, 2020

Previously, #17601 set the asset path if embedder APIs, RunConfigurations::InferFromSettings, or relevant service protocols are called. However, it turns out that Android uses none of them, so we need to add this. (The old PR #17601 was only tested with iOS devices and unit tests.)

Copy link
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@liyuqian
Copy link
Contributor Author

@chinmaygarde : do you know how to best test this? It seems that "platform_view_android_jni" isn't test anywhere in our unit tests.

@liyuqian liyuqian force-pushed the fix_android_sksl_asset_path branch from 02834a7 to 721fb03 Compare April 25, 2020 06:59
@liyuqian liyuqian changed the title Set the SkSL asset path on Android Set SkSL asset manager in RunConfiguration ctor Apr 25, 2020
@liyuqian
Copy link
Contributor Author

@chinmaygarde : never mind. I just realized that there's no asset path in Android so we'll have to use the AssetManager directly. It's a breeze as we just moved from loading files in a path to loading a single json file (#17861). Now the unit test covers the case of Android construction of RunConfiguration. There's no way that one can start a shell without a RunConfiguration, right?

@liyuqian liyuqian force-pushed the fix_android_sksl_asset_path branch 3 times, most recently from 863f1d0 to 967b0e0 Compare April 25, 2020 18:58
liyuqian referenced this pull request in jonahwilliams/flutter Apr 27, 2020
Android doesn't use RunConfiguration::InferFromSettings, and there's no
path for assets since it's using AAssetManager_open. So we will directly
use the AssetManager and ensure its initialization during
RunConfiguration construction.
@liyuqian liyuqian force-pushed the fix_android_sksl_asset_path branch from 967b0e0 to aecb8a3 Compare April 27, 2020 23:04
@liyuqian liyuqian added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Apr 28, 2020
@fluttergithubbot fluttergithubbot merged commit 50ae2b9 into flutter:master Apr 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yes platform-android waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants