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

Conversation

sbaranov
Copy link
Contributor

This replicates similar logic found in ApkAssetProvider that locates assets by their short name in a sudbirectory inside of an archive file, instead of only at the root of archive.

This replicates similar logic found in ApkAssetProvider that locates assets by their short name in a sudbirectory inside of an archive file, instead of only at the root of archive.
@sbaranov sbaranov changed the title Add support for loading flutter assets from dynamic patch. Support loading flutter assets from dynamic patch Dec 26, 2018
@sbaranov sbaranov requested a review from liyuqian December 26, 2018 23:50
@@ -253,7 +253,7 @@ static void RunBundleAndSnapshotFromLibrary(JNIEnv* env,
const auto file_ext_index = bundlepath.rfind(".");
if (bundlepath.substr(file_ext_index) == ".zip") {
asset_manager->PushBack(
std::make_unique<blink::ZipAssetStore>(bundlepath));
std::make_unique<blink::ZipAssetStore>(bundlepath, "flutter_assets"));
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the new codestd::make_unique<blink::ZipAssetStore>(bundlepath, "flutter_assets")); equivalent to std::make_unique<blink::ZipAssetStore>("flutter_assets/" + bundlepath)); in the old code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not equivalent because bundlepath is the path to the archive file, and directory is the path inside of the archive (just like in ApkAssetProvider).


std::stringstream file_name;
file_name << directory_.c_str() << "/" << asset_name;
auto found = stat_cache_.find(file_name.str());
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I think auto found = stat_cache_.find(directory_ + "/" + asset_name); might be cleaner.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

@liyuqian liyuqian left a comment

Choose a reason for hiding this comment

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

LGTM

@sbaranov sbaranov merged commit 868dc04 into flutter:master Dec 28, 2018
@sbaranov sbaranov deleted the zip-subdir branch December 28, 2018 18:34
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 28, 2018
engine-flutter-autoroll added a commit to flutter/flutter that referenced this pull request Dec 29, 2018
flutter/engine@1bd9d41...995e324

git log 1bd9d41..995e324 --no-merges --oneline
995e324 Roll src/third_party/skia f3e6b90461e4..aec8c7e280cb (1 commits) (flutter/engine#7326)
6f763fb Minor refactoring of dynamic patching code. (flutter/engine#7325)
868dc04 Support loading flutter assets from dynamic patch (flutter/engine#7308)

The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff ([email protected]), and stop
the roller if necessary.
kangwang1988 pushed a commit to XianyuTech/flutter that referenced this pull request Feb 12, 2019
flutter/engine@1bd9d41...995e324

git log 1bd9d41..995e324 --no-merges --oneline
995e324 Roll src/third_party/skia f3e6b90461e4..aec8c7e280cb (1 commits) (flutter/engine#7326)
6f763fb Minor refactoring of dynamic patching code. (flutter/engine#7325)
868dc04 Support loading flutter assets from dynamic patch (flutter/engine#7308)

The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff ([email protected]), and stop
the roller if necessary.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants