Embedder API goof

update-semantics-abi.md

ABI stability for the update semantics API

Background

In Flutter 3.7 we introduced a new embedder API for semantics update (flutter/engine#37129):

/// A batch of updates to semantics nodes and custom actions.
typedef struct {
  /// The size of the struct. Must be sizeof(FlutterSemanticsUpdate).
  size_t struct_size;
  /// The number of semantics node updates.
  size_t nodes_count;
  // Array of semantics nodes. Has length `nodes_count`.
  FlutterSemanticsNode* nodes;
  /// The number of semantics custom action updates.
  size_t custom_actions_count;
  /// Array of semantics custom actions. Has length `custom_actions_count`.
  FlutterSemanticsCustomAction* custom_actions;
} FlutterSemanticsUpdate;

typedef void (*FlutterUpdateSemanticsCallback)(
  const FlutterSemanticsUpdate* /* semantics update */,
  void* /* user data*/);

typedef struct {
  ...
  
  /// The callback invoked by the engine in order to give the embedder the
  /// chance to respond to updates to semantics nodes and custom actions from
  /// the Dart application.
  ///
  /// The callback will be invoked on the thread on which the `FlutterEngineRun`
  /// call is made.
  ///
  /// If this callback is provided, update_semantics_node_callback and
  /// update_semantics_custom_action_callback must not be provided.
  FlutterUpdateSemanticsCallback update_semantics_callback;
} FlutterProjectArgs;

The Tizen embedder has migrated to this new API: flutter-tizen/embedder#24

Problem

Adding new members to FlutterSemanticsNode and FlutterSemanticsCustomAction may cause external embedders to crash if they use array indexing on nodes or custom_actions. For example, this external embedder code is bad:

FlutterSemanticsUpdate update = ...;
FlutterSemanticsNode first = update.nodes[0];
FlutterSemanticsNode second = update.nodes[1]; // BAD! This could crash.

External embedders that update their Flutter engine without recompiling will be broken.

Instead, external embedders must determine the size of FlutterSemanticsNode or FlutterSemanticsCustomAction using their struct_size member. Something like:

FlutterSemanticsUpdate update = ...;
FlutterSemanticsNode first = update.nodes[0];
FlutterSemanticsNode* second = static_cast<FlutterSemanticsNode*>(
  static_cast<char*>(update.nodes) + 1 * first.struct_size);

This is clearly error-prone.

Note that Flutter's first-party embedders are unaffected as they are recompiled whenever the Flutter engine change.

Solution

Update embedder.h's comment with:

// - Be careful with arrays of structures that are part of the ABI as
//   indexing elements is error-prone. Consider using arrays of pointers
//   to structures instead.

Solution 0: do nothing

We add a comment that explains how to access FlutterSemanticsUpdate's array elements.

Cons:

1. Unstable ABI. External embedders may crash when we modify semantics.

Solution 1: introduce a new type and callback

We add a new type and callback for semantic updates where the arrays contain pointers to FlutterSemanticsNode and FlutterSemanticsCustomAction:

/// A batch of updates to semantics nodes and custom actions.
typedef struct {
  /// The size of the struct. Must be sizeof(FlutterSemanticsUpdate).
  size_t struct_size;
  /// The number of semantics node updates.
  size_t nodes_count;
  // Array of semantics node pointers. Has length `nodes_count`.
  FlutterSemanticsNode** nodes;
  /// The number of semantics custom action updates.
  size_t custom_actions_count;
  /// Array of semantics custom action pointers. Has length `custom_actions_count`.
  FlutterSemanticsCustomAction** custom_actions;
} FlutterSemanticsUpdate2;

typedef void (*FlutterUpdateSemanticsCallback2)(
  const FlutterSemanticsUpdate2* /* semantics update */,
  void* /* user data*/);

typedef struct {
  ...

  /// DEPRECATED. Migrate to `update_semantics_callback2`!
  FlutterUpdateSemanticsCallback update_semantics_callback;
  
  /// New:
  FlutterUpdateSemanticsCallback2 update_semantics_callback2;
} FlutterProjectArgs;

Pro:

1. External embedders can migrate to the new API to prevent crashes.

Cons:

1. Unstable ABI. External embedders that haven't migrated yet may crash when we modify semantics.

Solution 2: provide helpers to access items

Provide a function to access FlutterSemanticsUpdate elements:

FlutterSemanticsNode*
FlutterEngineGetSemanticsNode(FlutterSemanticsUpdate* update, size_t index);

FlutterSemanticsCustomAction*
FlutterEngineGetSemanticsCustomAction(FlutterSemanticsUpdate* update, size_t index);

With this solution, we could also change FlutterSemanticsUpdate.nodes and FlutterSemanticsUpdate.custom_actions to void*. This would break external embedder's compilation so that they know to migrate.

Pros:

1. We don't need to introduce new types or members

Cons:

1. Unstable ABI. External embedders that haven't migrated yet may crash when we modify semantics.

Solution 3: introduce new fields

/// A batch of updates to semantics nodes and custom actions.
typedef struct {
  /// The size of the struct. Must be sizeof(FlutterSemanticsUpdate).
  size_t struct_size;
  /// ALWAYS 0.
  size_t nodes_count;
  // ALWAYS `nullptr`
  FlutterSemanticsNode* nodes;
  /// ALWAYS 0
  size_t custom_actions_count;
  /// ALWAYS `nullptr`
  FlutterSemanticsCustomAction* custom_actions;
  /// The number of semantics node updates.
  size_t nodes_count2;
  // Array of semantics node pointers. Has length `nodes_count`.
  FlutterSemanticsNode** nodes2;
  /// The number of semantics custom action updates.
  size_t custom_actions_count2;
  /// Array of semantics custom action pointers. Has length `custom_actions_count`.
  FlutterSemanticsCustomAction** custom_actions2;
} FlutterSemanticsUpdate2;

Pros:

1. External embedder would not crash if we break the ABI.

Cons:

1. This is a breaking change. An external embedder's accessibility support would not work as expected until they migrate.

Solution 4: introduce new semantic types (recommended)

Introduce FlutterSemanticsNode2, FlutterSemanticsCustomAction2, and FlutterSemanticsUpdate2 (see solution 1).

Initially, FlutterSemanticsNode2 and FlutterSemanticsCustomAction2 will be identical to FlutterSemanticsNode and FlutterSemanticsCustomAction. However, additions to the semantics API will only be added to FlutterSemanticsNode2 and FlutterSemanticsCustomAction2. The original FlutterSemanticsNode and FlutterSemanticsCustomAction structures will be guaranteed to never change.

Pros:

1. Existing external embedders will never be broken. Full ABI stability guaranteed.
2. Existing external embedders can migrate when they want to support new accessibility features.

Cons:

1. This bloats the embedder API

Thanks for writing this up! As discussed over chat, Solution 4 definitely LGTM. We should add a note to the list of warnings at the top of embedder.h about arrays of struct types.