[Embedder API] Add 'array of pointers' guidance

[loic-sharma]

Update the embedder API's documentation to recommend arrays of pointers to structures instead of arrays to structures.

Violations of this new guidance:

1. FlutterSemanticsUpdate - Tracked by [Embedder API] Fix ABI stability of the update semantics API flutter#121176
2. FlutterFrameBufferWithDamageCallback - Adding members to FlutterRect will break the ABI, tracked by [Embedder API] Lock the FlutterRect struct to guarantee ABI stability  flutter#121347
3. FlutterEngineNotifyDisplayUpdate - Adding members to FlutterEngineDisplay will break the ABI (here), tracked by [Embedder API] Fix FlutterEngineNotifyDisplayUpdate's ABI stability  flutter#121352
4. FlutterEngineSendPointerEvent - ✅ The engine uses struct_size correctly

Part of: flutter/flutter#121176
See go/flutter-semantics-abi

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.

[cbracken]

FlutterMetalTextureFrameCallback - ⚠️ Adding members to FlutterMetalTextureHandle will break the ABI

This looks okay to me. FlutterMetalTextureHandle is just a typedef of void* so it'll always be sized to the size of a pointer.

FlutterFrameBufferWithDamageCallback - ⚠️ Adding members to FlutterRect will break the ABI

Agreed with this one, but I also suspect it's not worth fixing this one; it seems really unlikely we'll ever add any additional fields here. It may be worth adding a comment to that effect though.

FlutterEngineNotifyDisplayUpdate - ⚠️ Adding members to FlutterEngineDisplay will break the ABI (here)

I could definitely imagine someone wanting to add further fields to this struct, so we definitely want to add a comment at minimum.

Something that might be nice to have is a test we can run as part of CI that verifies that certain "locked down" structs are never altered. I don't even think it needs to be particularly complicated - even a tool that greps for certain blocks of code would do it.

[loic-sharma]

@cbracken Thanks for the feedback! I opened the following issues to fix ABI stability:

1. [Embedder API] Lock the FlutterRect struct to guarantee ABI stability  flutter#121347
2. [Embedder API] Fix FlutterEngineNotifyDisplayUpdate's ABI stability  flutter#121352

These will be addressed in subsequent changes. This pull request is ready for review :)

[cbracken]

This LGTM.

As you noted separately, we could limit this to embedder-facing arrays of structs passed back in e.g. callbacks since for incoming structs, since for structs going the other way (being passed into the engine) we always use safe dereference macros, but we can't guarantee that third-party embedders do the same.

All that said, I think there's a lot of value in keeping the advice here as succinct as possible and there are no downsides to doing this in both directions aside from making things a very small amount more onerous for embedders. @chinmaygarde probably has thoughts on this.

[cbracken]

lgtm; please wait for @chinmaygarde's thoughts on the question of whether we want to have separate advice for outgoing/incoming arrays.

[chinmaygarde]

I think both incoming and outgoing structures have to be arrays of pointers. We can't depend on the embedder struct sizes being ones we expect to be consistent. We have the do the same struct size checks on our end so we don't read past the end.