-
Notifications
You must be signed in to change notification settings - Fork 6k
[fuchsia] Create DartComponentController for CFv2. #28613
Conversation
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. |
shell/platform/fuchsia/dart_runner/dart_component_controller_v2.cc
Outdated
Show resolved
Hide resolved
shell/platform/fuchsia/dart_runner/dart_component_controller_v2.cc
Outdated
Show resolved
Hide resolved
shell/platform/fuchsia/dart_runner/dart_component_controller_v2.cc
Outdated
Show resolved
Hide resolved
shell/platform/fuchsia/dart_runner/dart_component_controller_v2.cc
Outdated
Show resolved
Hide resolved
|
||
tonic::DartMicrotaskQueue::StartForCurrentThread(); | ||
|
||
// TODO(fxb/...): Point these at the actual stdout/stderr handles of the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There will never be actual stdout/stderr handles for the component. If we want to pass these in for each component then we will need to create them on our own and proxy them to the runner's stdout/stderr.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. I think the distinction is a bit unclear to me - when you say "create them on our own and proxy them to the runner's stdout/stderr", do you mean the component creates them? And how does proxying them work again?
Maybe more generally is there more work required here? IIRC this was placeholder logic from the original PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a bit of design work that needs to be done here so we can follow up more offline.
Fuchsia does not have a concept of stdout/stderr, what is happening is that the dart runner gets launched by the elf runner which creates a handle that can be used for stdout/stderr. Internally, it takes anything that is written to that descriptor and forwards it on to the kernel log and decorates that it is coming from the dart runner. If we pass along that same handle to the dart components then all print statements will appear to come from the dart runner.
What I think we need to do is create a file descriptor for each component that is launched and listen for anything that is written to it. When something is written to it we just forward that message along to the fuchsia logger and decorate it with the tag that it came from the component.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, makes sense. Thanks so much, updated the TODO here.
Bug: 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:
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.