-
Notifications
You must be signed in to change notification settings - Fork 6k
block thread merging with shared engines #23733
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. Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
shell/common/rasterizer.cc
Outdated
| "This is likely a result of using platform views with enigne " | ||
| "groups. See " | ||
| "https://github.com/flutter/flutter/issues/73620."; | ||
| abort(); |
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'm a little sad I don't know what the answer is off of the top of my head, but I feel like there was a prior conversation about this at some point and there was a reason why there isn't any abort() in our codebase currently.
b/170074271 vaguely references this but it's largely a reporting issue. Maybe it's ok.
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.
fml::KillProcess. Its an abort with a noreturn decoration so the compiler won't be mad about missing return statements from scopes that don't exit.
@xster Are you thinking of exit?
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.
If there's precedent then LGTM :)
shell/common/rasterizer.cc
Outdated
| "This is likely a result of using platform views with enigne " | ||
| "groups. See " | ||
| "https://github.com/flutter/flutter/issues/73620."; | ||
| abort(); |
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.
fml::KillProcess. Its an abort with a noreturn decoration so the compiler won't be mad about missing return statements from scopes that don't exit.
@xster Are you thinking of exit?
2eafae4 to
bdf7208
Compare
|
Landing on red, the previous red was a flake on a test we disabled, the current red is an infra failure. This should hopefully flush the infra failure. |
* d4a7358 Roll Dart SDK from c4214e6daaac to 4a6764bf28c2 (4 revisions) (flutter/engine#23770) * 9bc776a [web] Add --watch flag to 'felt test' (flutter/engine#23727) * 247ebc2 Roll Skia from bde06cc511d2 to f3087d8297fe (7 revisions) (flutter/engine#23772) * 8b27e6f skip flaky test (flutter/engine#23775) * 2927e9f block thread merging with shared engines (flutter/engine#23733) * df5f3b0 Implement handling of framework-handled key events (flutter/engine#23655) * f205ced Roll Skia from f3087d8297fe to e0fe62adaa3e (9 revisions) (flutter/engine#23781) * fa7aebf Roll Skia from e0fe62adaa3e to 18aeb5731b51 (1 revision) (flutter/engine#23784) * 9acfb7d Fix JNI void vs object method call (flutter/engine#23785) * df13ccf Roll Skia from 18aeb5731b51 to 7aa7f039b9ee (1 revision) (flutter/engine#23786) * e3e3b2b Roll Fuchsia Mac SDK from pc_veLlry... to xYraItnQp... (flutter/engine#23787) * 8a096d6 ci: Print output in case of compile error (flutter/engine#23522) * f1c3ced Roll Fuchsia Linux SDK from fByXAJ76e... to vs54lOVoj... (flutter/engine#23788) * 0c79393 Revert "Roll Dart SDK from c4214e6daaac to 4a6764bf28c2 (4 revisions) (#23770)" (flutter/engine#23791)
This will make sure that people don't use platform view with flutter engine groups until we've successfully accounted for them.
issue: flutter/flutter#73620
Pre-launch Checklist
writing and running engine tests.
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.