-
Notifications
You must be signed in to change notification settings - Fork 6k
Migrate flutter_runner from flutter_runner::{Thread,Loop} to fml::{Thread,MessageLoop} #15118
Conversation
dnfield
left a comment
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.
Most of this seems very reasonable, hoping @cbracken or @chinmaygarde can fill in some details as well.
| private: | ||
| Delegate& delegate_; | ||
| const std::string thread_label_; | ||
| flutter::ThreadHost thread_host_; |
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.
This seems like a strange class to own the thread host, but that's how we had it before I guess.
Is this migration going to help us migrate Fuchsia to the embedder API eventually, or is it completely unrelated?
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.
This change should definitely move us in the direction of migrating Fuchsia to using the embedder API.
|
CI failure on Fuchsia is an infra issue - bot update failed on one of the builders. |
739e190 to
110d157
Compare
fml::{Thread,MessageLoop}
6fa8e27 to
a4408d1
Compare
|
Some updates:
|
iskakaushik
left a comment
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.
LGTM
…fml::{Thread,MessageLoop} (flutter/engine#15118)
…fml::{Thread,MessageLoop} (flutter/engine#15118)
…fml::{Thread,MessageLoop} (flutter/engine#15118)
…fml::{Thread,MessageLoop} (flutter/engine#15118)
…fml::{Thread,MessageLoop} (flutter/engine#15118)
…fml::{Thread,MessageLoop} (flutter/engine#15118)
…read,MessageLoop} (flutter#15118)
…read,MessageLoop} (flutter#15118)
…read,MessageLoop} (flutter#15118)
…read,MessageLoop} (flutter#15118)
…fml::{Thread,MessageLoop} (flutter/engine#15118)
…fml::{Thread,MessageLoop} (flutter/engine#15118)
…fml::{Thread,MessageLoop} (flutter/engine#15118)
…fml::{Thread,MessageLoop} (flutter/engine#15118)
…fml::{Thread,MessageLoop} (flutter/engine#15118)
…fml::{Thread,MessageLoop} (flutter/engine#15118)
flutter/engine@f10f03a...51a7964 git log f10f03a..51a7964 --first-parent --oneline 2020-01-24 skia-flutter-autoroll@skia.org Roll src/third_party/dart bc9348829ef8..fc3af737c759 (2 commits) (flutter/engine#15965) 2020-01-24 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from Wc7e4... to 8Ns10... (flutter/engine#15964) 2020-01-24 skia-flutter-autoroll@skia.org Roll src/third_party/skia a7e557f3e353..3ea4d5bb857d (4 commits) (flutter/engine#15963) 2020-01-24 skia-flutter-autoroll@skia.org Roll src/third_party/dart c359b5943a52..bc9348829ef8 (1 commits) (flutter/engine#15962) 2020-01-24 gw280@google.com ensure we export the various dart snapshot symbols on Fuchsia (flutter/engine#15953) 2020-01-24 skia-flutter-autoroll@skia.org Roll src/third_party/dart 3eaae5405d37..c359b5943a52 (13 commits) (flutter/engine#15959) 2020-01-24 gw280@google.com Migrate flutter_runner from flutter_runner::{Thread,Loop} to fml::{Thread,MessageLoop} (flutter/engine#15118) 2020-01-24 gw280@google.com Re-arm timer as necessary in MessageLoopFuchsia 2020-01-24 skia-flutter-autoroll@skia.org Roll src/third_party/skia c88a3bc3f561..a7e557f3e353 (6 commits) (flutter/engine#15957) 2020-01-24 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from LAOYs... to 6_pZp... (flutter/engine#15954) 2020-01-24 zhongwuzw@qq.com Fixes FlutterCallbackInfomation leaks (flutter/engine#15089) 2020-01-24 zhongwuzw@qq.com Fixes oc leaks in platform plugin (flutter/engine#15041) 2020-01-24 jason-simmons@users.noreply.github.com Do not reset the child isolate preparer if the isolate group data already has one (flutter/engine#15952) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC aaclarke@google.com on the revert to ensure that a human is aware of the problem. To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md
…fml::{Thread,MessageLoop} (flutter#15118)"
This reverts commit 41e8ed0.
…fml::{Thread,MessageLoop} (flutter#15118)" (flutter#15903)
This reverts commit a50f1ef.
…read,MessageLoop} (flutter#15118)
…fml::{Thread,MessageLoop} (flutter#15118)" (flutter#16277)
This reverts commit 41e8ed0.
…oop} to fml::{Thread,MessageLoop} (flutter#15118)" (flutter#16277)"
This reverts commit 4b76686.
…fml::{Thread,MessageLoop} (flutter#15118)"
This reverts commit a181b0c.
…oop} to fml::{Thread,MessageLoop} (flutter#15118)" (flutter#15903)"
This reverts commit a37eb4c.
…read,MessageLoop} (flutter#15118)
…oop} to fml::{Thread,MessageLoop} (flutter#15118)" (flutter#15903)"
This reverts commit 69bc783.
…oop} to fml::{Thread,MessageLoop} (flutter#15118)" (flutter#15903)"
This reverts commit 69bc783.
…oop} to fml::{Thread,MessageLoop} (flutter#15118)" (flutter#15903)"
This reverts commit 69bc783.
Right now we are using our own Loop and Thread implementations on flutter_runner on Fuchsia. This patch migrates us to using the built-in fml types now that they work on Fuchsia (and have unit tests running on CI).
The biggest concern I have with this is the (hopefully temporary) plumbing I've added to extract the async::Loop object from the MessageLoop through the static method
FuchsiaLoopForMessageLooponMessageLoopFuchsia. This is currently necessary because we use the FuchsiaTracecomponent which requires theasync::Loopobject to initialise.