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

Conversation

@stuartmorgan-g
Copy link
Contributor

Description

Standard*Codec allows for extensions to support arbitrary types; this
had not previously been implemented for the C++ version.

Overview of changes:

  • EncodableValue's std::variant type now allows for a new CustomEncodableValue, which is a thin wrapper around std::any, to store arbitrary extension types.
  • ByteBufferStream* has been split into an interface class and the buffer-based implementation, with the former now part of the public API surface to be used in standard codec extensions.
    • They also gained utility methods for some common data types to simplify writing extensions.
  • StandardCodecSerializer is now part of the public API surface, and is subclassable.
  • StandardCodecSerializer's ReadValue has been split into ReadValue and ReadValueOfType to match the structure used when subclassing on the Dart side, for easier porting of custom extensions across languages.
  • Standard*Codec now optionally accepts a non-default serializer in GetInstance, providing a shared instance using that serializer.

Related Issues

Fixes flutter/flutter#31174

Tests

I added the following tests: Round-trip testing of method codec and message codec with custom types.

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the contributor guide and followed the process outlined there for submitting PRs.
  • I signed the CLA.
  • I read and followed the C++, Objective-C, Java style guides for the engine.
  • I read the tree hygiene wiki page, which explains my responsibilities.
  • I updated/added relevant documentation.
  • All existing and new tests are passing.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read handling breaking changes.

  • No, no existing tests failed, so this is not a breaking change.
  • Yes, this is a breaking change.

Standard*Codec allows for extensions to support arbitrary types; this
had not previously been implemented for the C++ version.

Fixes flutter/flutter#31174
@auto-assign auto-assign bot requested a review from flar August 7, 2020 02:11
@stuartmorgan-g
Copy link
Contributor Author

I don't love the necessity of the propagation of the singleton pattern from the codecs to the serializer, but I couldn't figure out a better solution. Since it's interface-based, I can't just copy the codecs around, and when I tried using shared_ptr instead of weak references I ran into issues where the template type deduction stopped working, which would have meant always having to declare the template type of channels, event streams, etc. instead of that being determined automatically by the passed codec. That didn't seem like a good trade-off since it means more boilerplate in every plugin.

If you have thoughts about other solutions, let me know!

@stuartmorgan-g stuartmorgan-g merged commit b28b1df into flutter:master Aug 10, 2020
@stuartmorgan-g stuartmorgan-g deleted the cpp-standard-codec-extensions branch August 10, 2020 20:41
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 11, 2020
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.

Allow extending C++ StandardMethodCodec

3 participants