[pigeon] Added basic C++ support

[azchohfi]

This PR adds basic C++ support for Pigeon. I've used the video_player pigeon file as a minimum bar for this, so there are things that are known to not work at all, like async. This code was based on a mix of the Obj-C generator and the Java generator.
I need to add more tests. I've added tests only to the new parameters, and not to the generator itself.
We can work together on getting a min set of tests for this before we merge it, but I wanted to share what I've came up with so far to reduce re-work.

Fixes flutter/flutter#70310

Pre-launch Checklist

• The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
• 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].
• 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.
• I updated pubspec.yaml with an appropriate new version according to the [pub versioning philosophy].
• I updated CHANGELOG.md to add a description of the change.
• I updated/added relevant documentation (doc comments with ///).
• I signed the [CLA].
• All existing and new tests are passing.

[stuartmorgan]

Thanks for the contribution!

@gaaclarke Can you work with @azchohfi on figuring out the right set of tests, what the missing coverage is, README updates, etc. to get this to a land-able state?

[gaaclarke]

Here's what we should have for this, a new flutter/platform_tests test for c++, something like flutter/platform_tests/cpp_tests. It will have a test runner (gtest?). run_tests.py will use the new generator to generate c++ code that will get linked into the test runner and run unit tests on the generated c++ code.

Something like:

flutter/
  ↳ platform_tests/
    ↳ cpp_tests/
      ↳ CMakeLists.txt
      ↳ src/
        ↳ simple_test.cc
        ↳ generated/
          ↳ some_generated_cc_file_from_pigeon.cc
          ↳ some_generated_cc_file_from_pigeon.h

If we go down that route we'll have to specify the location of a flutter library to link, probably a desktop one. I haven't played with the c++ platform channels much to know what makes the most sense. We could opt to run the test runner on an iOS simulator or Android emulator if that makes things easier.

Let me know what you think of that. I can get the basic outline working if need be.

[stuartmorgan]

It will have a test runner (gtest?).

GoogleTest is what we're using for Windows plugin unit tests now, yes.

If we go down that route we'll have to specify the location of a flutter library to link, probably a desktop one.

Specifically the Windows desktop library; it's the only one that uses the C++ channel code.

It looks like you're using a dummy app project purpose for this for iOS and Android? We should be able to do the same here. (Although it's not clear to me why these are being run in a custom harness using a fake app project rather than just being set up as plugin native unit tests using a partially-generated plugin.)

We could opt to run the test runner on an iOS simulator or Android emulator if that makes things easier.

Why would we want to run on an emulator instead of on the host?

[gaaclarke]

Specifically the Windows desktop library; it's the only one that uses the C++ channel code.

Is it the only engine that includes the c++ channels?

It looks like you're using a dummy app project purpose for this for iOS and Android? We should be able to do the same here.

Yep, that's possible. It's a bit more complicated though because we don't have a windows CI bot yet and all the test automation is in shell scripts.

If they develop it with CMake, it should be easier to write something that can be run locally on windows and run in the existing ci bots.

Why would we want to run on an emulator instead of on the host?

My thought is that we have existing infrastructure for running tests on android and iOS. For someone developing on windows it might be easier to latch into that.

[stuartmorgan]

Specifically the Windows desktop library; it's the only one that uses the C++ channel code.

Is it the only engine that includes the c++ channels?

Yes. It wraps C APIs that only the Windows embedding (of the official embeddings) has, so it would be useless in any of our other embeddings.

It looks like you're using a dummy app project purpose for this for iOS and Android? We should be able to do the same here.

Yep, that's possible. It's a bit more complicated though because we don't have a windows CI bot yet and all the test automation is in shell scripts.

We can get Windows LUCI bots set up here any time using the same structure we're now using for flutter/plugins. I can get that started tomorrow.

If they develop it with CMake, it should be easier to write something that can be run locally on windows and run in the existing ci bots.

Our CI should build and run in the same environment users would be building and running in, which is Windows. Minor compiler differences are still a very real issue for C++.

Why would we want to run on an emulator instead of on the host?

My thought is that we have existing infrastructure for running tests on android and iOS. For someone developing on windows it might be easier to latch into that.

Between my usual refrain of "we shouldn't continue down the road of making Pigeon tests increasingly bespoke" and the issue above (we should test what users will run; zero people will run the generated code on iOS or Android) I'm strongly against that idea. Building and running a GoogleTest binary on the host is trivial, and if we set this test up as an actual plugin (which I think is how all of these tests should work), we already have tooling support for running it in the repo. We just need a step before it in the CI script to do the generation.

[gaaclarke]

I played around with flutter windows a bit more to get a better idea. The layout for the tests will be more like this:

flutter/
  ↳ platform_tests/
    ↳ cpp_tests/
      ↳ pubspec.yaml
      ↳ windows/
        ↳ CMakeLists.txt
        ↳ runner/
          ↳ simple_test.cc
          ↳ generated/
            ↳ some_generated_cc_file_from_pigeon.cc
            ↳ some_generated_cc_file_from_pigeon.h

Where flutter/platform_tests/cpp_tests was created with flutter create --platforms window and the CMakeLists.txts are edited to add a test runner target. I guess the easiest thing would be to check in the gtest library in the repository to link in. Potentially we could add a download step the test harness once we have a better idea what that will look like.

[azchohfi]

Thanks for the help @gaaclarke and @stuartmorgan.
Since this is my very first contribution to Flutter, I'm still not sure on regards to where things should be.

1. @gaaclarke, when you say flutter/platform_tests/cpp_tests, are you saying that I should send another PR to https://github.com/flutter/platform_tests with the tests, or that I should be creating a cpp_tests folder here in the pigeon package, similar to https://github.com/flutter/packages/tree/master/packages/pigeon/platform_tests/android_unit_tests? I'm leaning towards the latter, but I'm not sure.

2. I never did any GTest, but I believe I can use this commit as a reference:
   flutter/plugins@90fd90e
   @stuartmorgan, let me know if you have a better example.

3. Also, I would probably need some help with the CI scripts to run these in a Windows machine, as I also don't know where they live, or how they work.

[stuartmorgan]

I guess the easiest thing would be to check in the gtest library in the repository to link in.

We could, but for a one-off I would use the same structure we're currently using for plugin tests where it's pulled by the CMake setup.

[stuartmorgan]

2. I never did any GTest, but I believe I can use this commit as a reference:
   flutter/plugins@90fd90e
   @stuartmorgan, let me know if you have a better example.

That's the best reference. The caveat is that it's a plugin, not an app—thus my comments above about potentially changing the way Pigeon native tests are structured. But if @gaaclarke is committed to the app-based version, it shouldn't be hard to adapt.

3. Also, I would probably need some help with the CI scripts to run these in a Windows machine, as I also don't know where they live, or how they work.

Currently they don't :) I'm starting the process of getting those configured for this repo though; hopefully it'll be up and running by sometime next week. Because of the way Pigeon tests are currently structured though, ideally the CI would just call Pigeon's run_tests.sh and that would do everything that's needed.

(Alternately, this could be an opportunity to start the process of moving Pigeon away from a complex local script, and toward standard repository tooling; I can discuss with @gaaclarke offline and we can follow up here.)

[stuartmorgan]

I'm starting the process of getting those configured for this repo though; hopefully it'll be up and running by sometime next week.

I haven't forgotten about this, there was just a combination of technical snags and unexpected OOO that slowed this down. It's still something we're making progress on, and hopefully the latest unexpected hurdle is the last surprise.

[stuartmorgan]

The Windows CI support just landed, so it should be possible to add tests for this now; you'll just need to merge in the latest master.

The tooling changes to move to running the tests here with the standard plugin tooling are less trivial than I initially thought, since they don't run sub-packages (and adding that without unintended effects could be a bit tricky). I think a hybrid is the easiest way to move forward. We could:

• land a dummy plugin project that includes Windows
• incorporate the unit test structure that's being adopted in our 1P plugins (since it's not yet in the template)
• make a script—either via additions to run.sh (our Windows CI has bash) or a new Dart script to make it easier to run locally on Windows—that generates the test files into that plugin, and runs the tests.

If/when we later add the necessary support to flutter_plugin_tools to run the tests, we could eliminate that part of the script; the rest would continue to work as-is.

[stuartmorgan]

If we use std::unique_ptr, then the classes are not copy constructable, so we can't pass them to CustomEncodableValue to serialization. Is there a way around this?

Hm, maybe there's a more fundamental limitation in the Windows platform channel API.

Sounds like this would need a lot more investigation, and maybe even engine API changes, for truly efficient complex object handling. Given that, and that very complex objects is probably not a common case, let's go with the references for now, and if we need to make a breaking change in Pigeon someday to change things (maybe after engine changes), we can always do that.

[gaaclarke]

@azchohfi for the Swift generator PR I added a validation phase that blocks features that exist in (dart, java, objc) but isn't in swift yet: 817e9ff We should do that here too. Let me know if you want to take a stab at it, otherwise I can do it too. Also let me know if you already have a list of things we don't support yet.

[azchohfi]

@gaaclarke The commit you mentioned is very specific to the feature that the Swift one doesn't support. It should be quite trivial to add these validations to the C++ generator, but I would appreciate some help with this, as the initial code was written to support an older version of pigeon, so some features might actually be really missing. I know that I've added some of them given the feedback on the PR (like non-nullable references), but only to a minimal extent (things build, but not enough tests to mark this as not experimental).

[gaaclarke]

I ran through some of the more recent tests and I think you've implemented nullable returns and nonnull arguments. I think the only thing that isn't supported yet is non-null fields. I added a validator for that.

[azchohfi]

@gaaclarke Should we just remove the non_null_fields.dart file from the tests then? The tests are failing because of it now.

[gaaclarke]

@gaaclarke Should we just remove the non_null_fields.dart file from the tests then? The tests are failing because of it now.

Yep, sorry I missed that.

[azchohfi]

@gaaclarke do you want me to remove the file from the tests?

[gaaclarke]

@gaaclarke do you want me to remove the file from the tests?

Yea, I'd just comment them out the invocations from the run_tests script with a note.

[azchohfi]

@gaaclarke anything needed here?

[stuartmorgan]

The changes LGTM, so we're good to land this and then iterate from there.

@azchohfi Thanks for all the great work on this! I'm looking forward to trying it out on file_selector to get some hands-on experience with it :)

[stuartmorgan]

I filed flutter/flutter#104278 to track non-null field support.

[stuartmorgan]

@azchohfi By the way, if you would like contributor access just let me know; I'd be more than happy to sponsor you.

[azchohfi]

@stuartmorgan that would be awesome! I appreciate it!

[stuartmorgan]

Just ping me on our Discord when you get a chance (that's where the mechanics of adding someone are handled by the admins), and I'll start the process!

[azchohfi]

@stuartmorgan just sent you a friend request. Thanks again.

[clarkezone]

Great to see this landed, congrats @azchohfi !