-
Notifications
You must be signed in to change notification settings - Fork 6k
[fuchsia] flutter-embedder-test and touch-input-test use Flatland Test UI Stack #43562
Conversation
…t UI Stack This change also: - removes reliance on fuchsia.ui.scenic.Scenic protocols - migrates touch-input-test from fuchsia.ui.gfx.DisplayInfo to fuchsia.ui.display.singleton.Info - adds support for fuchsia.ui.display.singleton.Info in portable_ui_test
|
cc @arbreng |
| "fuchsia-pkg://fuchsia.com/parent-view#meta/parent-view.cm"; | ||
| static constexpr auto kTestUiStackUrl = | ||
| "fuchsia-pkg://fuchsia.com/test-ui-stack#meta/test-ui-stack.cm"; | ||
| "fuchsia-pkg://fuchsia.com/flatland-scene-manager-test-ui-stack#meta/" |
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.
It's important to avoid encoding these implementation details in the name. This is in fact why alex chose to preserve the "test-ui-stack" name.
You can update the "test-ui-stack" package definition here to include the flatland-scene-manager config: https://source.corp.google.com/h/turquoise-internal/turquoise/+/main:src/ui/testing/test_ui_stack/BUILD.gn;l=82?q=test_ui_stack%2FBUILD.gn
Ideally, we could do that before landing this CL.
If that's not possible and we must wait until after this CL (due to soft transitions), then I insist that we file a bug and follow-up with another PR here to change the name to "test-ui-stack".
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.
Actually, we are going to have to come back here anyways with a followup, so it's no big deal to do this now.
Test UI Stack will be replaced with the UI Test Realm Proxy, which will abstract these details. Additionally, we will use subpackages to run the "touch-input-test" from in-tree here and eliminate the duplicated code.
The naming issues can be fixed at that time, so it's OK to defer them.
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.
Nice, thanks for the heads up!
shell/platform/fuchsia/flutter/tests/integration/touch-input/touch-input-test.cc
Show resolved
Hide resolved
|
"removes the embedding view hit test disabled case from touch-input test, since disabling the hit region is not currently supported in Flutter's integration with Flatland afaict" Please file a bug to re-add this support, via the in-tree touch-input-test, since it is possible to do so. The plan of record is to use subpackages to replace the duplicated test code in this repository, and instead run the in-tree touch-input-test here. So, if that functionality is tested via fuchsia's "touch-input-test", it can eventually be tested and re-enabled here. |
arbreng
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, but please file the bug about re-enabling the "hit testable" support and CC me on it
…atland Test UI Stack (flutter/engine#43562)
…130735) flutter/engine@09fe990...aa3239c 2023-07-17 [email protected] [fuchsia] flutter-embedder-test and touch-input-test use Flatland Test UI Stack (flutter/engine#43562) 2023-07-17 [email protected] Roll Skia from 4ec9f2497be1 to dc93f341ec38 (10 revisions) (flutter/engine#43739) 2023-07-17 [email protected] Roll Skia from 288c98d7ef0b to 4ec9f2497be1 (1 revision) (flutter/engine#43738) 2023-07-17 [email protected] Roll Fuchsia Mac SDK from VYjne_BEm9inQ5fnq... to jtvD_HgQVBqadF3jX... (flutter/engine#43736) 2023-07-17 [email protected] Roll Dart SDK from 827259dfffb9 to c1bfb2689f6f (1 revision) (flutter/engine#43735) Also rolling transitive DEPS: fuchsia/sdk/core/mac-amd64 from VYjne_BEm9in to jtvD_HgQVBqa 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 [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose 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/+doc/main/autoroll/README.md
…t UI Stack (flutter#43562) This change also: - removes reliance from both tests on `fuchsia.ui.scenic.Scenic` protocols - migrates `touch-input-test` to get display info from `fuchsia.ui.display.singleton.Info` instead of `fuchsia.ui.gfx.DisplayInfo` - adds support for `fuchsia.ui.display.singleton.Info` in `portable_ui_test` to support the above - migrates `flutter-embedder-test` to capture screenshots using `fuchsia.ui.composition.Screenshot` instead of `fuchsia.ui.scenic.Screenshot` - updates screenshot tooling accordingly - removes the embedding view hit test disabled case from `touch-input test`, since disabling the hit region is not currently supported in Flutter's integration with Flatland afaict ([this logic](https://github.com/flutter/engine/blob/0f0436b28430bd79e854f2294a60aa33417068fa/shell/platform/fuchsia/flutter/flatland_external_view_embedder.cc#L411-L434) appears to set hit regions to the full view size by default, and Flatland appears to drop the value [here](https://github.com/flutter/engine/blob/0f0436b28430bd79e854f2294a60aa33417068fa/shell/platform/fuchsia/flutter/flatland_external_view_embedder.cc#L578)) - removes hit test configurability on `touch-input-test` and `flutter-embedder-test` embedding views Bug: fxbug.dev/125514
…lutter#130735) flutter/engine@09fe990...aa3239c 2023-07-17 [email protected] [fuchsia] flutter-embedder-test and touch-input-test use Flatland Test UI Stack (flutter/engine#43562) 2023-07-17 [email protected] Roll Skia from 4ec9f2497be1 to dc93f341ec38 (10 revisions) (flutter/engine#43739) 2023-07-17 [email protected] Roll Skia from 288c98d7ef0b to 4ec9f2497be1 (1 revision) (flutter/engine#43738) 2023-07-17 [email protected] Roll Fuchsia Mac SDK from VYjne_BEm9inQ5fnq... to jtvD_HgQVBqadF3jX... (flutter/engine#43736) 2023-07-17 [email protected] Roll Dart SDK from 827259dfffb9 to c1bfb2689f6f (1 revision) (flutter/engine#43735) Also rolling transitive DEPS: fuchsia/sdk/core/mac-amd64 from VYjne_BEm9in to jtvD_HgQVBqa 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 [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose 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/+doc/main/autoroll/README.md
This change also:
fuchsia.ui.scenic.Scenicprotocolstouch-input-testto get display info fromfuchsia.ui.display.singleton.Infoinstead offuchsia.ui.gfx.DisplayInfofuchsia.ui.display.singleton.Infoinportable_ui_testto support the aboveflutter-embedder-testto capture screenshots usingfuchsia.ui.composition.Screenshotinstead offuchsia.ui.scenic.Screenshottouch-input test, since disabling the hit region is not currently supported in Flutter's integration with Flatland afaict (this logic appears to set hit regions to the full view size by default, and Flatland appears to drop the value here)touch-input-testandflutter-embedder-testembedding viewsBug: fxbug.dev/125514