-
Notifications
You must be signed in to change notification settings - Fork 6k
Ensure that the platform view is created and destroyed when running the shell unittests. #6560
Conversation
…he shell unittests.
liyuqian
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.
RSLGTM
| #include "flutter/shell/common/rasterizer.h" | ||
| #include "flutter/shell/common/shell.h" | ||
| #include "flutter/shell/common/thread_host.h" | ||
| #include "flutter/shell/gpu/gpu_surface_software.h" |
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.
The following is a general comment on naming that does not need to be addressed in this PR:
The name of "gpu_surface_software" seems a little strange: if it's software, it's not a gpu surface...
This issue exists in many places such as "gpu thread" which may or may not involve gpu. Maybe at some point, we should substitute all "gpu" with "render" so they become "render_surface_software" and "render thread".
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.
Agreed. The support for various backends using different client rendering APIs was added much later and these naming patterns are an unfortunate relic of the old way of doing things. These APIs are not public however and so should be straightforward to rename to make more idiomatic.
shell/common/shell_unittests.cc
Outdated
| FML_DISALLOW_COPY_AND_ASSIGN(TestPlatformView); | ||
| }; | ||
|
|
||
| bool ValidateShell(Shell* shell) { |
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.
Nit: in Skia, we tend to put static before these functions (e.g., static bool ValidateShell) that are only used in this cc file.
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.
Done
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 is a pattern we follow in the engine too. I was just forgetful :(
flutter/engine@08272ee...04c860f git log 08272ee..04c860f --no-merges --oneline 04c860f Revert "Roll Dart to ac6d4f7e653deba11d4836768376537893a9e9d6. (flutter#6549)" (flutter/engine#6568) ef2ea12 Roll src/third_party/skia 5b90b84085cf..cb65ce7f77c9 (8 commits) (flutter/engine#6567) 065246e Roll src/third_party/skia 483536c242f3..5b90b84085cf (3 commits) (flutter/engine#6566) b0008ca Roll src/third_party/skia 583161c09a3b..483536c242f3 (2 commits) (flutter/engine#6565) e22da7c Roll src/third_party/skia 0a8faf3f557d..583161c09a3b (1 commits) (flutter/engine#6564) 915f5e0 Roll src/third_party/skia f831b64ed40e..0a8faf3f557d (5 commits) (flutter/engine#6563) ad0fbd5 Roll src/third_party/skia c9092eb46754..f831b64ed40e (14 commits) (flutter/engine#6561) 6697d9d Update @animation dartdoc directives to current api (flutter/engine#6552) 10f9cab Ensure that the platform view is created and destroyed when running the shell unittests. (flutter/engine#6560) df5f420 Roll src/third_party/skia 3d99b1e347be..c9092eb46754 (10 commits) (flutter/engine#6559) 215c146 Roll src/third_party/skia 1349ec4de78c..3d99b1e347be (3 commits) (flutter/engine#6556) fc376dd Roll src/third_party/skia a583a06f831e..1349ec4de78c (1 commits) (flutter/engine#6555) 2dbbb2a Roll src/third_party/skia 198b87e5be20..a583a06f831e (5 commits) (flutter/engine#6554) 126e61d Roll src/third_party/skia b3067e8e1b6e..198b87e5be20 (2 commits) (flutter/engine#6553) 2cd79d1 Roll src/third_party/skia 4b7b2ceb4ad9..b3067e8e1b6e (1 commits) (flutter/engine#6551) 62cd86c Advertise iOS Observatory port over mDNS/Bonjour (flutter/engine#6533) 26f437f Fix crash when removeViewImmediate invoked from platform view (flutter/engine#6266) aed6b8c Roll Dart to ac6d4f7e653deba11d4836768376537893a9e9d6. (flutter/engine#6549) 3ba6270 Roll src/third_party/skia 921ec976556c..4b7b2ceb4ad9 (14 commits) (flutter/engine#6550) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary.
flutter/engine@08272ee...04c860f git log 08272ee..04c860f --no-merges --oneline 04c860f Revert "Roll Dart to ac6d4f7e653deba11d4836768376537893a9e9d6. (#6549)" (flutter/engine#6568) ef2ea12 Roll src/third_party/skia 5b90b84085cf..cb65ce7f77c9 (8 commits) (flutter/engine#6567) 065246e Roll src/third_party/skia 483536c242f3..5b90b84085cf (3 commits) (flutter/engine#6566) b0008ca Roll src/third_party/skia 583161c09a3b..483536c242f3 (2 commits) (flutter/engine#6565) e22da7c Roll src/third_party/skia 0a8faf3f557d..583161c09a3b (1 commits) (flutter/engine#6564) 915f5e0 Roll src/third_party/skia f831b64ed40e..0a8faf3f557d (5 commits) (flutter/engine#6563) ad0fbd5 Roll src/third_party/skia c9092eb46754..f831b64ed40e (14 commits) (flutter/engine#6561) 6697d9d Update @animation dartdoc directives to current api (flutter/engine#6552) 10f9cab Ensure that the platform view is created and destroyed when running the shell unittests. (flutter/engine#6560) df5f420 Roll src/third_party/skia 3d99b1e347be..c9092eb46754 (10 commits) (flutter/engine#6559) 215c146 Roll src/third_party/skia 1349ec4de78c..3d99b1e347be (3 commits) (flutter/engine#6556) fc376dd Roll src/third_party/skia a583a06f831e..1349ec4de78c (1 commits) (flutter/engine#6555) 2dbbb2a Roll src/third_party/skia 198b87e5be20..a583a06f831e (5 commits) (flutter/engine#6554) 126e61d Roll src/third_party/skia b3067e8e1b6e..198b87e5be20 (2 commits) (flutter/engine#6553) 2cd79d1 Roll src/third_party/skia 4b7b2ceb4ad9..b3067e8e1b6e (1 commits) (flutter/engine#6551) 62cd86c Advertise iOS Observatory port over mDNS/Bonjour (flutter/engine#6533) 26f437f Fix crash when removeViewImmediate invoked from platform view (flutter/engine#6266) aed6b8c Roll Dart to ac6d4f7e653deba11d4836768376537893a9e9d6. (flutter/engine#6549) 3ba6270 Roll src/third_party/skia 921ec976556c..4b7b2ceb4ad9 (14 commits) (flutter/engine#6550) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary.
Also adds a failing test for flutter/flutter#21398 that will be address in a subsequent patch.