Embedder Asset Provider

[ds84182]

New embedder API for allowing embedders to handle asset requests. This is working towards the option for File I/O-less Engine builds. Some embedders may have custom filesystem APIs that are incompatible with fml's File & Directory APIs. Instead of trying to put a square peg in a round hole, allow embedders to handle I/O instead.

flutter/flutter#93838

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
• I listed at least one issue that this PR fixes in the description above.
• I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
  writing and running engine tests.
• I updated/added relevant documentation (doc comments with ///).
• I signed the CLA.
• All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[jmagman]

Mac Unopt error is at https://ci.chromium.org/p/flutter/builders/try/Mac%20Unopt/6180
Rebase onto #29668

[ds84182]

This PR is ready for review.

[chinmaygarde]

The public API is prone to misuse in the following cases:

• If the embedder creates and caches mapping objects (something that is very reasonable for embedders to do), the second instance of the engine asking the embedder to fetch the asset will lead to the mapping being held by two unique pointers (in the asset resolver). A release by the first instance will lead to an double free.
• If the mapping is created by the embedder but the embedder never returns it via a callback, they have no way of collecting the handle and it will be leaked.

In a C API, it's usually suspicious when a Create call is not balanced by a subsequent Collect. I suggest adding the two with a handle to a struct that has shared ownership of the mapping being returned to the embedder. Then, in the resolver, the engine can just ref the handle.

Other construction issues and nits added inline. Thanks and sorry about the delayed review.

[ds84182]

@chinmaygarde

If the embedder creates and caches mapping objects (something that is very reasonable for embedders to do), the second instance of the engine asking the embedder to fetch the asset will lead to the mapping being held by two unique pointers (in the asset resolver). A release by the first instance will lead to an double free.

The documentation on the API specifies that the embedder should always return fresh mapping objects from the callback. Otherwise yes they will encounter a double-free. If the embedder wants to share the underlying data with multiple mappings, they should maintain some sort of reference count. Should the documentation outline this more explicitly?

As a quick example of usage (from Rust for brevity, also possible with std::shared_ptr):

let my_shared_data: Arc<[u8; N]> = Arc::new(b"my file bytes here");

// Creating mappings for the shared data:
let mut create_info = FlutterEngineMappingCreateInfo::new();

create_info.data = my_shared_data.as_ptr();
create_info.data_size = my_shared_data.len();

// Increment the reference count and turn into a raw ptr
create_info.user_data = Arc::into_raw(Arc::clone(my_shared_data));
unsafe extern "C" fn destroy(user_data: *mut c_void) {
  // Turn raw ptr into the associated Arc and drop to decrement reference count
  Arc::from_raw(user_data as *const [u8; N]);
}
create_info.destruction_callback = destroy;

let mapping = FlutterEngineCreateMapping(&create_info);

If the mapping is created by the embedder but the embedder never returns it via a callback, they have no way of collecting the handle and it will be leaked.

Good call, I'll add a destroy method for the mappings. It also makes sense if we start handing mappings from the engine -> embedder. Would it be worthwhile to also add data + size getters to fully flesh out the API?

[ds84182]

I've updated the FlutterMapping API to be a bit more general. The comments on the mapping-related calls explain the required ownership semantics.

I was debating whether it's worthwhile to change the interface some more to be more explicit about the ownership requirements. For example using a different opaque struct for Engine-owned mappings vs Embedder-owned mappings, and having a call to "give ownership" of an Embedder mapping to the Engine. Something like:

typedef struct FlutterEngineOwnedMapping* FlutterEngineOwnedMapping;
typedef struct FlutterEmbedderOwnedMapping* FlutterEmbedderOwnedMapping;

// Releases an embedder-owned mapping to the engine. This is used to give the mapping to the engine inside of
// AssetResolver->get_asset.
// Internally just a pointer cast, but may do some additional debug-only book-keeping in the future.
FlutterEngineOwnedMapping FlutterReleaseMappingOwnership(FlutterEmbedderOwnedMapping mapping);

void FlutterCreateMapping(..., FlutterEmbedderOwnedMapping* out);
void FlutterDestroyMapping(FlutterEmbedderOwnedMapping mapping);
const uint8_t* FlutterGetMappingData(FlutterEmbedderOwnedMapping mapping, ...);

---

If we want to also have an engine-provided shared mapping, I think it should have a different interface. e.g.

typedef struct FlutterSharedMapping* FlutterSharedMapping;
void FlutterCreateSharedMapping(const FlutterSharedMappingCreateInfo* create_info, FlutterSharedMapping* out);

// Increases refcount, can be directly returned from AssetResolver->get_asset
FlutterEngineOwnedMapping FlutterMakeEngineMappingFromShared(FlutterSharedMapping mapping);

// The mapping may stay alive if there are any mappings owned by the engine.
// New mappings cannot be created from the shared mapping after this point.
void FlutterDestroySharedMapping(FlutterSharedMapping mapping);

However, I think this is tangential to this PR and can be implemented by embedders with the current API.

[flutter-dashboard]

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.

[zanderso]

What is the status of this PR?

[ds84182]

@zanderso Currently on the backburner. Focus has changed on my side somewhat, but I can spend a bit of time solving the merge conflict.

Other than that I think there's still some open discussion from @chinmaygarde about how FlutterMapping is exposed to the Embedder.

[zanderso]

Marking this WIP, but cc @iskakaushik regarding an issue from @akbiggs in flutter/flutter#100640.

[Hixie]

I'm going to close this one for now per @ds84182; they have the branch locally and will submit a new PR when ready.