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

[Embedder API] Introduce new update semantics callback #39807

Merged
merged 2 commits into from
Mar 22, 2023

Conversation

loic-sharma
Copy link
Member

@loic-sharma loic-sharma commented Feb 23, 2023

This introduces a new update semantics embedder API that is identical to the previous update semantics embedder API, except that its ABI is stable as:

  1. The callback uses array of pointers to structs instead of array of structs. This ensures that array indexing does not break if new members are added to the structs.
  2. The types FlutterSemanticsNode and FlutterSemanticsCustomAction were duplicated as FlutterSemanticsNode2 and FlutterSemanticsCustomAction2. This is necessary as FlutterSemanticsUpdate has an array of FlutterSemanticsNode and an array of FlutterSemanticsCustomAction. Adding new members to these structs would break the ABI compatibility of array indexing.

This change does not migrate embedders to this new embedder API, this will be done in subsequent changes: flutter/flutter#121176.

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.

@flutter-dashboard flutter-dashboard bot added the embedder Related to the embedder API label Feb 23, 2023
@loic-sharma loic-sharma changed the title [Embedder API] Introduce new update semantics callback [WIP][Embedder API] Introduce new update semantics callback Feb 23, 2023
@loic-sharma loic-sharma added the Work in progress (WIP) Not ready (yet) for review! label Feb 23, 2023
@loic-sharma loic-sharma force-pushed the update_semantics_2 branch 3 times, most recently from 7a23da1 to a11389e Compare March 3, 2023 01:01
@loic-sharma loic-sharma changed the title [WIP][Embedder API] Introduce new update semantics callback [Embedder API] Introduce new update semantics callback Mar 3, 2023
@loic-sharma loic-sharma removed the Work in progress (WIP) Not ready (yet) for review! label Mar 3, 2023
@loic-sharma loic-sharma marked this pull request as ready for review March 3, 2023 22:59
@loic-sharma loic-sharma force-pushed the update_semantics_2 branch 2 times, most recently from f9359c8 to 4d30835 Compare March 4, 2023 01:02
Copy link
Contributor

@yaakovschectman yaakovschectman left a comment

Choose a reason for hiding this comment

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

Makes sense

Copy link
Member

@cbracken cbracken left a comment

Choose a reason for hiding this comment

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

Overall looks good - mostly nitpicks.

@loic-sharma loic-sharma force-pushed the update_semantics_2 branch 3 times, most recently from ebe42f0 to 2b36fe3 Compare March 10, 2023 19:27
Copy link
Contributor

@chunhtai chunhtai left a comment

Choose a reason for hiding this comment

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

LGTM

@loic-sharma
Copy link
Member Author

@cbracken and @chinmaygarde This is ready for review. Please take your time as I'd like to make sure there aren't any further embedder API mistakes :)

Copy link
Member

@cbracken cbracken 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 a teensy nit.

This introduces a new update semantics embedder API that is identical to
the previous update semantics embedder API, except that its ABI is
stable as:

1. The callback uses array of pointers to structs instead of array of
structs. This ensures that array indexing does not break if new members
are added to the structs.
2. The types `FlutterSemanticsNode` and `FlutterSemanticsCustomAction`
were duplicated as `FlutterSemanticsNode2` and
`FlutterSemanticsCustomAction2`. This is necessary as
`FlutterSemanticsUpdate` has an array of `FlutterSemanticsNode` and an
array of `FlutterSemanticsCustomAction`. Adding new members to these
structs would break the ABI compatibility of array indexing.

This change does not migrate embedders to this new embedder API, this
will be done in subsequent changes:
flutter/flutter#121176.

Part of: flutter/flutter#121176
@loic-sharma loic-sharma added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 22, 2023
@auto-submit auto-submit bot merged commit 6597218 into flutter:main Mar 22, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 22, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 22, 2023
loic-sharma pushed a commit to loic-sharma/flutter-engine that referenced this pull request Mar 23, 2023
This moves the Windows embedder to the new update semantics embedder API
introduced by flutter#39807. This does not
affect functionality hence why existing accessibility integration tests are unaffected.

Part of: flutter/flutter#121176
loic-sharma pushed a commit to loic-sharma/flutter-engine that referenced this pull request Mar 24, 2023
This moves the Windows embedder to the new update semantics embedder API
introduced by flutter#39807. This does not
affect functionality hence why existing accessibility integration tests are unaffected.

Part of: flutter/flutter#121176
loic-sharma added a commit to loic-sharma/flutter-engine that referenced this pull request Mar 30, 2023
The old "update semantics" embedder API was deprecated as it could not
guarantee ABI stability (see flutter/flutter#121176).
It was replaced by a new embedder API (flutter#39807)
and all affected embedders were migrated to this new embedder API
(flutter#40072, flutter#40584).

This change:
1. Removes plumbing that was used for the old "update semantics" embedder API
2. Moves all remaining tests to the types used by the new "update semantics" embedder API

Completes: flutter/flutter#121176
loic-sharma added a commit to loic-sharma/flutter-engine that referenced this pull request Mar 30, 2023
The old "update semantics" embedder API was deprecated as it could not
guarantee ABI stability (see flutter/flutter#121176).
It was replaced by a new embedder API (flutter#39807)
and all affected embedders were migrated to this new embedder API
(flutter#40072, flutter#40584).

This change:
1. Removes plumbing that was used for the old "update semantics" embedder API
2. Moves all remaining tests to the types used by the new "update semantics" embedder API

Completes: flutter/flutter#121176
loic-sharma added a commit to loic-sharma/flutter-engine that referenced this pull request Mar 30, 2023
The old "update semantics" embedder API was deprecated as it could not
guarantee ABI stability (see flutter/flutter#121176).
It was replaced by a new embedder API (flutter#39807)
and all affected embedders were migrated to this new embedder API
(flutter#40072, flutter#40584).

This change:
1. Removes plumbing that was used for the old "update semantics" embedder API
2. Moves all remaining tests to the types used by the new "update semantics" embedder API

Completes: flutter/flutter#121176
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
autosubmit Merge PR when tree becomes green via auto submit App embedder Related to the embedder API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants