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

Conversation

@arbreng
Copy link
Contributor

@arbreng arbreng commented Aug 12, 2021

Unfortunately #27226 is causing multiple flakes in the fuchsia tree, and I cannot figure out a simple fix forward.

The failure seems to be non-deterministic, and I observed at least 3 different callstacks during my testing.

See: https://bugs.fuchsia.dev/p/fuchsia/issues/detail?id=82557

@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@google-cla google-cla bot added the cla: yes label Aug 12, 2021
@arbreng arbreng changed the title Revert '[fuchsia] Make dart_runner work with cfv2' Revert '[fuchsia] Make dart_runner work with cfv2 (#27226)' Aug 12, 2021
binary = "dart_jit${product_suffix}_runner"

cmx_file = rebase_path("meta/dart_jit${product_suffix}_runner.cmx")
cml_file = rebase_path("meta/dart_jit${product_suffix}_runner.cml")
Copy link
Contributor

@chandarrengoog chandarrengoog Aug 12, 2021

Choose a reason for hiding this comment

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

Are any of our components currently using the v2 pathway? or is this a regression in the v1 pathway?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The V1 pathway regressed because the fidl_compatibility_tests in-tree make use of this path / dart_runner

See: https://source.corp.google.com/fuchsia/src/tests/fidl/compatibility/

@arbreng arbreng merged commit cd24385 into flutter:master Aug 12, 2021
@arbreng arbreng deleted the revert-dart-v2 branch August 12, 2021 16:22
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 12, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 12, 2021
akbiggs added a commit that referenced this pull request Sep 8, 2021
Bug: 79871

Partial re-land of #27226 (which was reverted in #28036 due to a flaky test). Only adds in the CML files for a CFv2 dart_runner without the corresponding C++ and GN changes.
akbiggs added a commit that referenced this pull request Sep 14, 2021
Bug: fxb/79871

The code is still using the CFv1 implementation, so this should not affect the existing behavior.

For this version of the PR, I forked the existing DartComponentController. There were a few reasons for this:

Reduces the chance of the change breaking existing tests during Fuchsia GI.
Makes the V2 version of the DartComponentController simpler to read.
Makes it easier to switch over to using the V2 version of the DartComponentController once the migration is ready to happen (less cleanup).
The rate of changes to the DartComponentController is slow so there shouldn't be much maintenance required for the fork.
Changes to the DartRunner itself will happen in a follow-up PR - this PR only verifies that the ComponentController builds.

Relands GN and C++ changes for #27226 after rollback in #28036.
filmil pushed a commit to filmil/engine that referenced this pull request Apr 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants