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

Add new FlutterEngineAOTData argument to FlutterProjectArgs #18146

Merged
merged 14 commits into from
May 10, 2020

Conversation

MarcusTomlinson
Copy link
Contributor

Added a new FlutterEngineAOTData argument to FlutterProjectArgs. Embedders can instantiate and destroy this object via the new FlutterEngineCreateAOTData and FlutterEngineCollectAOTData methods provided.

If an embedder provides more than one source of AOT data to FlutterEngineInitialize or FlutterEngineRun (e.g. snapshots as well as FlutterEngineAOTData), the engine will error out.

Resolves: flutter/flutter#50778

@auto-assign auto-assign bot requested a review from flar May 5, 2020 14:12
@MarcusTomlinson
Copy link
Contributor Author

MarcusTomlinson commented May 5, 2020

For a real world example, here's the GLFW embedder updated to use the new API:

https://github.com/MarcusTomlinson/glfw

(see: README.md)

@MarcusTomlinson MarcusTomlinson changed the title Add new aot_data argument to FlutterProjectArgs Add new FlutterEngineAOTData argument to FlutterProjectArgs May 5, 2020
@flar flar removed their request for review May 5, 2020 20:21
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

I'll defer to Chinmay on the substantive part of the review; just some style nits from me. Looks great!

Copy link
Member

@chinmaygarde chinmaygarde left a comment

Choose a reason for hiding this comment

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

Apart from the pointer to AOT data in FluttereProjectArgs, everything else is minor nits. This is looking great. Thanks.

@MarcusTomlinson
Copy link
Contributor Author

Thanks for the reviews guys! Learnt a lot from them!

@MarcusTomlinson
Copy link
Contributor Author

@chinmaygarde, I just remembered, the additional kSnapshotsInitialize and kAOTDataInitialize initialisation preferences for the config builder are actually needed. If I default to populating just aot_data in AOT mode, we lose all coverage of the snapshots approach in the unit tests. I've put this back again.

Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

LGTM with nits; please wait for Chinmay's approval as well though.

Copy link
Member

@chinmaygarde chinmaygarde left a comment

Choose a reason for hiding this comment

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

One last comment about the lifecycle of the AOTData and when it may be collected. Other than that, lgtm. Thanks for the patch and the followups. This is great.

@MarcusTomlinson
Copy link
Contributor Author

@chinmaygarde, @stuartmorgan: I think that covers it :) Please could you merge when you get the chance. Thanks!

@stuartmorgan-g stuartmorgan-g merged commit d96f962 into flutter:master May 10, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 11, 2020
wandyers pushed a commit to wandyers/engine that referenced this pull request May 23, 2020
…18146)

Added a new `FlutterEngineAOTData` argument to `FlutterProjectArgs`. Embedders can instantiate and destroy this object via the new `FlutterEngineCreateAOTData` and `FlutterEngineCollectAOTData` methods provided.

If an embedder provides more than one source of AOT data to `FlutterEngineInitialize` or `FlutterEngineRun` (e.g. snapshots as well as `FlutterEngineAOTData`), the engine will error out.

Resolves: flutter/flutter#50778
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.

Expose embedder API affordances for working with AOT buffers from disparate sources.
4 participants