Repository navigation
Support DynamicLibrary.codeAsset in Flutter - #188947
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces helper functions in the devicelab tests to verify loading native assets via DynamicLibrary.codeAsset, and refactors the C++ engine runtime to support a new dlopen_v2 API that provides detailed error reporting for missing or invalid native asset mappings. Feedback points out that the multi-line string replacement in the Dart test helper is sensitive to platform-specific line endings, and that a missing null check on asset_id in the C++ implementation could lead to a crash.
dcharkes
left a comment
There was a problem hiding this comment.
I don't know if the devicelab tests are run on the CI if we only make engine changes. And we don't have any engine-only native assets tests. So, we should make sure we run the devicelab test locally.
Also, we could add a test in packages/flutter_tools/test/integration.shard/isolated/native_assets_flutter_run_test.dart (This test runs on desktop. The devicelab runs android/iOS.)
a3dad08 to
b6084fa
Compare
|
This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
b6084fa to
d491697
Compare
dd3a7bc to
c3634b9
Compare
|
@dcharkes The Dart changes have landed and are now rolled into Flutter. PTAL. |
dcharkes
left a comment
There was a problem hiding this comment.
LGTM!
It looks green except for some warnings.
info • Specify the type of a local variable when the type is non-obvious. Try adding a type annotation • dev/devicelab/lib/tasks/native_assets_test.dart:200:3 • specify_nonobvious_local_variable_types
info • Specify the type of a local variable when the type is non-obvious. Try adding a type annotation • dev/devicelab/lib/tasks/native_assets_test.dart:202:3 • specify_nonobvious_local_variable_types
info • Specify the type of a local variable when the type is non-obvious. Try adding a type annotation • dev/devicelab/lib/tasks/native_assets_test.dart:209:3 • specify_nonobvious_local_variable_types
info • Specify the type of a local variable when the type is non-obvious. Try adding a type annotation • packages/flutter_tools/test/integration.shard/isolated/native_assets_test_utils.dart:150:3 • specify_nonobvious_local_variable_types
info • Specify the type of a local variable when the type is non-obvious. Try adding a type annotation • packages/flutter_tools/test/integration.shard/isolated/native_assets_test_utils.dart:151:3 • specify_nonobvious_local_variable_types
info • Specify the type of a local variable when the type is non-obvious. Try adding a type annotation • packages/flutter_tools/test/integration.shard/isolated/native_assets_test_utils.dart:153:3 • specify_nonobvious_local_variable_types
info • Specify the type of a local variable when the type is non-obvious. Try adding a type annotation • packages/flutter_tools/test/integration.shard/isolated/native_assets_test_utils.dart:165:3 • specify_nonobvious_local_variable_types
info • Specify the type of a local variable when the type is non-obvious. Try adding a type annotation • packages/flutter_tools/test/integration.shard/isolated/native_assets_test_utils.dart:187:3 • specify_nonobvious_local_variable_types
info • Specify the type of a local variable when the type is non-obvious. Try adding a type annotation • packages/flutter_tools/test/integration.shard/isolated/native_assets_test_utils.dart:191:3 • specify_nonobvious_local_variable_types
info • Specify the type of a local variable when the type is non-obvious. Try adding a type annotation • packages/flutter_tools/test/integration.shard/isolated/native_assets_test_utils.dart:192:3 • specify_nonobvious_local_variable_types
info • Specify the type of a local variable when the type is non-obvious. Try adding a type annotation • packages/flutter_tools/test/integration.shard/isolated/native_assets_test_utils.dart:205:3 • specify_nonobvious_local_variable_types
info • Specify the type of a local variable when the type is non-obvious. Try adding a type annotation • packages/flutter_tools/test/integration.shard/isolated/native_assets_test_utils.dart:223:3 • specify_nonobvious_local_variable_types
|
|
||
| final ProcessTestResult result = await runFlutter( | ||
| <String>['run', '-d$device', '--$buildMode'], | ||
| <String>[...getLocalEngineArguments(), 'run', '-d$device', '--$buildMode'], |
c3634b9 to
5a8da3f
Compare
|
@dcharkes warnings should be gone now. |
|
google testing looks like a real failure, possibly I have to update some build files internally, investigating. |
| static void InitDartFFIForIsolateGroup() { | ||
| NativeAssetsApi native_assets; | ||
| memset(&native_assets, 0, sizeof(native_assets)); | ||
| // TODO(dacoharkes): Remove after flutter_tools stops kernel embedding. |
There was a problem hiding this comment.
google testing looks like a real failure, possibly I have to update some build files internally, investigating.
We need to keep these for google-testing.
The internal build of google still uses the kernel mapping instead of of NativeAssetsManifest.json.
Please updated the comment to: Remove after Flutter stops kernel embedding in g3.
5a8da3f to
b13ae55
Compare
|
@dcharkes can merge now? |
|
Goldens are green, Dashboard is green but now Google Testing is red due to: Maybe we need to wait until that rolls into google for the testing to turn green? |
|
I manually did a Google check with a roll of #193288 (which hasn't rolled yet), then it is green. Overriding Google testing status. |
|
autosubmit label was removed for flutter/flutter/188947, because This PR has not met approval requirements for merging. The PR author is not a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.
|
…#13060) Manual roll Flutter from 4fcd90be0045 to c5a061b18fa2 (222 revisions) Manual roll requested by [email protected] flutter/flutter@4fcd90b...c5a061b 2026-09-28 [email protected] [cross imports] navigator_test.dart (flutter/flutter#190954) 2026-09-28 [email protected] Revert: Reland: Only render views that need to be rendered (flutter/flutter#193360) 2026-09-28 [email protected] ci(bringup): android_intent_security_test is green (flutter/flutter#193469) 2026-09-28 [email protected] Sync CHANGELOG.md from stable (flutter/flutter#193020) 2026-09-28 [email protected] Roll Fuchsia Linux SDK from ukukV5lEkKabtOATk... to QdgqP02_cYpRQQQNN... (flutter/flutter#193454) 2026-09-28 [email protected] Roll pub packages (flutter/flutter#193460) 2026-09-28 [email protected] Un-nest sceneBuildDuration and windowRenderDuration in web SceneBuilderRecorder (flutter/flutter#193260) 2026-09-28 [email protected] Fix unresolved doc comment references in dev/integration_tests and dev/benchmarks (flutter/flutter#193433) 2026-09-28 [email protected] Roll Packages from e55e7ac to ba0364a (9 revisions) (flutter/flutter#193450) 2026-09-28 [email protected] [tool] Require explicit dependency injection for FlutterDevice and FlutterDevice.create (flutter/flutter#192830) 2026-09-28 [email protected] Roll pub packages (flutter/flutter#193442) 2026-09-28 [email protected] [flutter_tools] Include data assets from hooks when pubspec.yaml is empty (flutter/flutter#193434) 2026-09-28 [email protected] Roll pub packages (flutter/flutter#193438) 2026-09-28 [email protected] Fix unresolved doc comment references in Material and Cupertino (flutter/flutter#193283) 2026-09-28 [email protected] Fix doc references to Material and Cupertino in the widgets library and its tests (flutter/flutter#193334) 2026-09-26 [email protected] wasm: enforce WasmGC opt-in capability checks and Firefox < 147 guard (flutter/flutter#193180) 2026-09-26 [email protected] [Android] Refuse external setters of engine entrypoint and cached engine arguments via `Intent`s (flutter/flutter#190249) 2026-09-26 [email protected] [web] Unskip TextPainter.getWordBoundary test (flutter/flutter#193378) 2026-09-26 [email protected] Specify non-obvious types in pattern variable declarations (flutter/flutter#192632) 2026-09-26 [email protected] Removes feedback loop from advanced filters without offscreen msaa and framebufferfetch (flutter/flutter#193306) 2026-09-26 [email protected] [web] Unskip 8 passing image tests in painting and rendering (flutter/flutter#193377) 2026-09-26 [email protected] Revert: Allow resetting test invariants in `addTearDown` (flutter/flutter#193383) 2026-09-25 [email protected] [tool] Migrate CoverageCollector to modular dependency injection (flutter/flutter#192924) 2026-09-25 [email protected] Fix more unresolved doc comment references (flutter/flutter#193336) 2026-09-25 [email protected] Allow resetting test invariants in `addTearDown` (flutter/flutter#192082) 2026-09-25 [email protected] [a11y] Add a semantics role for slider (flutter/flutter#193324) 2026-09-25 [email protected] Fix plugin tests after Pigeon plugin template changes (flutter/flutter#193302) 2026-09-25 [email protected] [Android 17] Bump Standard Test Apps in flutter/flutter to AGP 9.3.1 (flutter/flutter#193263) 2026-09-25 [email protected] SSBO-based gradients in UberSDF (flutter/flutter#192962) 2026-09-25 [email protected] [Linux] Handle FlView being destroyed before rendering is complete. (flutter/flutter#193268) 2026-09-25 [email protected] [flutter_tools] Handle Windows reserved characters in test target path (flutter/flutter#191900) 2026-09-25 [email protected] docs(tools): nit: say hook/build.dart in CMake native assets comment (flutter/flutter#192205) 2026-09-25 [email protected] [web] Enable Firefox Skwasm UI CI suites, configure COI configs, and fail fast on loader rejections (flutter/flutter#193187) 2026-09-25 [email protected] [Android 17] Bumped Engine Dependencies to 9.3.1 (flutter/flutter#193265) 2026-09-25 [email protected] [web] Omit group role on menu scrollables and assign region role to named routes (flutter/flutter#192965) 2026-09-25 [email protected] Roll Skia from 8eedeed98e79 to f441ca223b2b (5 revisions) (flutter/flutter#193353) 2026-09-25 [email protected] Roll Fuchsia Linux SDK from EbPpoJW-Lnsu-8dyZ... to ukukV5lEkKabtOATk... (flutter/flutter#193326) 2026-09-25 [email protected] Roll Packages from 431ea69 to e55e7ac (6 revisions) (flutter/flutter#193351) 2026-09-25 [email protected] Support DynamicLibrary.codeAsset in Flutter (flutter/flutter#188947) 2026-09-25 [email protected] Roll Dart SDK from 75d9e87d4e3d to 0e7642b85457 (1 revision) (flutter/flutter#193346) 2026-09-25 [email protected] Exclude iOS PRs from macOS link (flutter/flutter#193315) 2026-09-25 [email protected] Roll Skia from 6357608543ec to 8eedeed98e79 (4 revisions) (flutter/flutter#193341) 2026-09-25 [email protected] Roll Dart SDK from d3aedb186aab to 75d9e87d4e3d (3 revisions) (flutter/flutter#193335) 2026-09-25 [email protected] Cache the paint offset adjusted line metrics in TextPainter (flutter/flutter#191223) ...
Registers the Flutter native-assets callbacks required by
DynamicLibrary.codeAssetwhile preserving@Nativeprocess fallback.The required Dart change landed in dart-lang/sdk@eb5d798.
Verified with a local engine for flutter-tester and Linux desktop debug, including hot reload and hot restart.
cc: @dcharkes