From 511a8ef6e69569419c54ec223eb20dd74e64eeed Mon Sep 17 00:00:00 2001 From: skia-flutter-autoroll Date: Wed, 24 Jul 2024 18:02:13 -0400 Subject: [PATCH 1/8] Roll Skia from 54d1434637a1 to 55ecdde3a5fa (3 revisions) (#54089) https://skia.googlesource.com/skia.git/+log/54d1434637a1..55ecdde3a5fa 2024-07-24 jvanverth@google.com Update comment in patheffects.cpp. 2024-07-24 skia-autoroll@skia-public.iam.gserviceaccount.com Roll skottie-base from 52a246a737a2 to a6d9a7de5704 2024-07-24 robertphillips@google.com [graphite] Add comments to public API re Graphite-specific deprecations If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/skia-flutter-autoroll Please CC brianosman@google.com,jamesgk@google.com,jonahwilliams@google.com,rmistry@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Skia: https://bugs.chromium.org/p/skia/issues/entry 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://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md --- DEPS | 2 +- ci/licenses_golden/licenses_skia | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/DEPS b/DEPS index ae1f71ff23396..929ba2d9ae6e9 100644 --- a/DEPS +++ b/DEPS @@ -14,7 +14,7 @@ vars = { 'flutter_git': 'https://flutter.googlesource.com', 'skia_git': 'https://skia.googlesource.com', 'llvm_git': 'https://llvm.googlesource.com', - 'skia_revision': '54d1434637a1fdcae59bc8b149689c9429de72f2', + 'skia_revision': '55ecdde3a5fab967bb40701a789076dfec9c1c6c', # WARNING: DO NOT EDIT canvaskit_cipd_instance MANUALLY # See `lib/web_ui/README.md` for how to roll CanvasKit to a new version. diff --git a/ci/licenses_golden/licenses_skia b/ci/licenses_golden/licenses_skia index e2bb0894af8a1..cc1d01cc03488 100644 --- a/ci/licenses_golden/licenses_skia +++ b/ci/licenses_golden/licenses_skia @@ -1,4 +1,4 @@ -Signature: 58e16b57769eac1473907787f5dfc93b +Signature: 4f505e90f0b36e0e81743d46ac5e4c07 ==================================================================================================== LIBRARY: etc1 From e1259b86ba02b931f5326603597165380209991e Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Wed, 24 Jul 2024 16:10:11 -0700 Subject: [PATCH 2/8] [iOS] Switch to FlutterMetalLayer by default. (#54086) For this to work, we need to provide our own capture scope otherwise the default scope won't capture our commands. This is required as part of the work to switch to unmerged threads for PVs (https://github.com/flutter/engine/pull/53826), as I can confirm @knopp 's observations that the performance is much worse with the default CAMetalLayer. Fixes https://github.com/flutter/flutter/issues/140901 --- impeller/renderer/backend/metal/context_mtl.h | 39 +++++++++++++++++-- .../renderer/backend/metal/context_mtl.mm | 33 ++++++++++++++++ impeller/renderer/backend/metal/surface_mtl.h | 5 +++ .../renderer/backend/metal/surface_mtl.mm | 3 ++ shell/gpu/gpu_surface_metal_impeller.mm | 9 +++++ .../gpu_surface_metal_impeller_unittests.mm | 32 ++++++++++++++- .../ios/framework/Source/FlutterMetalLayer.mm | 7 ++-- 7 files changed, 120 insertions(+), 8 deletions(-) diff --git a/impeller/renderer/backend/metal/context_mtl.h b/impeller/renderer/backend/metal/context_mtl.h index d1669206f0351..b09933c752dfc 100644 --- a/impeller/renderer/backend/metal/context_mtl.h +++ b/impeller/renderer/backend/metal/context_mtl.h @@ -31,6 +31,36 @@ namespace impeller { +/// @brief Creates and manages a Metal capture scope that supports frame capture +/// when using the FlutterMetalLayer backed drawable. +class ImpellerMetalCaptureManager { + public: + /// @brief Construct a new capture manager from the provided Metal device. + explicit ImpellerMetalCaptureManager(id device); + + ~ImpellerMetalCaptureManager() = default; + + /// Whether or not the Impeller capture scope is active. + /// + /// This is distinct from whether or not there is a session recording the + /// capture. That can be checked with `[[MTLCaptureManager + /// sharedCaptureManager] isCapturing].` + bool CaptureScopeActive() const; + + /// @brief Begin a new capture scope, no-op if the scope has already started. + void StartCapture(); + + /// @brief End the current capture scope. + void FinishCapture(); + + private: + id current_capture_scope_; + bool scope_active_ = false; + + ImpellerMetalCaptureManager(const ImpellerMetalCaptureManager&) = default; + ImpellerMetalCaptureManager(ImpellerMetalCaptureManager&&) = delete; +}; + class ContextMTL final : public Context, public BackendCast, public std::enable_shared_from_this { @@ -101,6 +131,8 @@ class ContextMTL final : public Context, #ifdef IMPELLER_DEBUG std::shared_ptr GetGPUTracer() const; + + const std::shared_ptr GetCaptureManager() const; #endif // IMPELLER_DEBUG // |Context| @@ -125,12 +157,13 @@ class ContextMTL final : public Context, std::shared_ptr resource_allocator_; std::shared_ptr device_capabilities_; std::shared_ptr is_gpu_disabled_sync_switch_; -#ifdef IMPELLER_DEBUG - std::shared_ptr gpu_tracer_; -#endif // IMPELLER_DEBUG std::deque> tasks_awaiting_gpu_; std::unique_ptr sync_switch_observer_; std::shared_ptr command_queue_ip_; +#ifdef IMPELLER_DEBUG + std::shared_ptr gpu_tracer_; + std::shared_ptr capture_manager_; +#endif // IMPELLER_DEBUG bool is_valid_ = false; ContextMTL(id device, diff --git a/impeller/renderer/backend/metal/context_mtl.mm b/impeller/renderer/backend/metal/context_mtl.mm index 22882ea0bd666..03c6ea073950f 100644 --- a/impeller/renderer/backend/metal/context_mtl.mm +++ b/impeller/renderer/backend/metal/context_mtl.mm @@ -3,6 +3,7 @@ // found in the LICENSE file. #include "impeller/renderer/backend/metal/context_mtl.h" +#include #include @@ -134,6 +135,7 @@ static bool DeviceSupportsComputeSubgroups(id device) { command_queue_ip_ = std::make_shared(); #ifdef IMPELLER_DEBUG gpu_tracer_ = std::make_shared(); + capture_manager_ = std::make_shared(device_); #endif // IMPELLER_DEBUG is_valid_ = true; } @@ -404,4 +406,35 @@ new ContextMTL(device, command_queue, return command_queue_ip_; } +#ifdef IMPELLER_DEBUG +const std::shared_ptr +ContextMTL::GetCaptureManager() const { + return capture_manager_; +} +#endif // IMPELLER_DEBUG + +ImpellerMetalCaptureManager::ImpellerMetalCaptureManager(id device) { + current_capture_scope_ = [[MTLCaptureManager sharedCaptureManager] + newCaptureScopeWithDevice:device]; + [current_capture_scope_ setLabel:@"Impeller Frame"]; +} + +bool ImpellerMetalCaptureManager::CaptureScopeActive() const { + return scope_active_; +} + +void ImpellerMetalCaptureManager::StartCapture() { + if (scope_active_) { + return; + } + scope_active_ = true; + [current_capture_scope_ beginScope]; +} + +void ImpellerMetalCaptureManager::FinishCapture() { + FML_DCHECK(scope_active_); + [current_capture_scope_ endScope]; + scope_active_ = false; +} + } // namespace impeller diff --git a/impeller/renderer/backend/metal/surface_mtl.h b/impeller/renderer/backend/metal/surface_mtl.h index 5ddc6973d06dd..dd028266a4aa7 100644 --- a/impeller/renderer/backend/metal/surface_mtl.h +++ b/impeller/renderer/backend/metal/surface_mtl.h @@ -61,6 +61,10 @@ class SurfaceMTL final : public Surface { // |Surface| bool Present() const override; + void SetFrameBoundary(bool frame_boundary) { + frame_boundary_ = frame_boundary; + } + private: std::weak_ptr context_; std::shared_ptr resolve_texture_; @@ -69,6 +73,7 @@ class SurfaceMTL final : public Surface { std::shared_ptr destination_texture_; bool requires_blit_ = false; std::optional clip_rect_; + bool frame_boundary_ = false; static bool ShouldPerformPartialRepaint(std::optional damage_rect); diff --git a/impeller/renderer/backend/metal/surface_mtl.mm b/impeller/renderer/backend/metal/surface_mtl.mm index 9219aca941278..e8cd572c5f5b1 100644 --- a/impeller/renderer/backend/metal/surface_mtl.mm +++ b/impeller/renderer/backend/metal/surface_mtl.mm @@ -231,6 +231,9 @@ - (void)flutterPrepareForPresent:(nonnull id)commandBuffer; #ifdef IMPELLER_DEBUG context->GetResourceAllocator()->DebugTraceMemoryStatistics(); + if (frame_boundary_) { + ContextMTL::Cast(context.get())->GetCaptureManager()->FinishCapture(); + } #endif // IMPELLER_DEBUG if (requires_blit_) { diff --git a/shell/gpu/gpu_surface_metal_impeller.mm b/shell/gpu/gpu_surface_metal_impeller.mm index bc9bfd78d2ffb..e14ac5c4a224b 100644 --- a/shell/gpu/gpu_surface_metal_impeller.mm +++ b/shell/gpu/gpu_surface_metal_impeller.mm @@ -106,6 +106,10 @@ last_texture_.reset([drawable.texture retain]); } +#ifdef IMPELLER_DEBUG + impeller::ContextMTL::Cast(*impeller_renderer_->GetContext()).GetCaptureManager()->StartCapture(); +#endif // IMPELLER_DEBUG + id last_texture = static_cast>(last_texture_); SurfaceFrame::SubmitCallback submit_callback = fml::MakeCopyable([damage = damage_, @@ -186,6 +190,7 @@ display_list->Dispatch(impeller_dispatcher, sk_cull_rect); auto picture = impeller_dispatcher.EndRecordingAsPicture(); const bool reset_host_buffer = surface_frame.submit_info().frame_boundary; + surface->SetFrameBoundary(surface_frame.submit_info().frame_boundary); return renderer->Render( std::move(surface), @@ -233,6 +238,10 @@ last_texture_.reset([mtl_texture retain]); } +#ifdef IMPELLER_DEBUG + impeller::ContextMTL::Cast(*impeller_renderer_->GetContext()).GetCaptureManager()->StartCapture(); +#endif // IMPELLER_DEBUG + SurfaceFrame::SubmitCallback submit_callback = fml::MakeCopyable([disable_partial_repaint = disable_partial_repaint_, // damage = damage_, diff --git a/shell/gpu/gpu_surface_metal_impeller_unittests.mm b/shell/gpu/gpu_surface_metal_impeller_unittests.mm index 13ccbba3f6906..a5927ff035783 100644 --- a/shell/gpu/gpu_surface_metal_impeller_unittests.mm +++ b/shell/gpu/gpu_surface_metal_impeller_unittests.mm @@ -103,7 +103,7 @@ GPUCAMetalLayerHandle GetCAMetalLayer(const SkISize& frame_info) const override auto context = CreateImpellerContext(); std::unique_ptr surface = - std::make_unique(delegate.get(), CreateImpellerContext()); + std::make_unique(delegate.get(), context); ASSERT_TRUE(surface->IsValid()); @@ -124,5 +124,35 @@ GPUCAMetalLayerHandle GetCAMetalLayer(const SkISize& frame_info) const override EXPECT_EQ(host_buffer.GetStateForTest().current_frame, 1u); } +#ifdef IMPELLER_DEBUG +TEST(GPUSurfaceMetalImpeller, CreatesImpellerCaptureScope) { + auto delegate = std::make_shared(); + delegate->SetDevice(); + + auto context = CreateImpellerContext(); + + EXPECT_FALSE(context->GetCaptureManager()->CaptureScopeActive()); + + std::unique_ptr surface = + std::make_unique(delegate.get(), context); + auto frame_1 = surface->AcquireFrame(SkISize::Make(100, 100)); + frame_1->set_submit_info({.frame_boundary = false}); + + EXPECT_TRUE(context->GetCaptureManager()->CaptureScopeActive()); + + std::unique_ptr surface_2 = + std::make_unique(delegate.get(), context); + auto frame_2 = surface->AcquireFrame(SkISize::Make(100, 100)); + frame_2->set_submit_info({.frame_boundary = true}); + + EXPECT_TRUE(context->GetCaptureManager()->CaptureScopeActive()); + + ASSERT_TRUE(frame_1->Submit()); + EXPECT_TRUE(context->GetCaptureManager()->CaptureScopeActive()); + ASSERT_TRUE(frame_2->Submit()); + EXPECT_FALSE(context->GetCaptureManager()->CaptureScopeActive()); +} +#endif // IMPELLER_DEBUG + } // namespace testing } // namespace flutter diff --git a/shell/platform/darwin/ios/framework/Source/FlutterMetalLayer.mm b/shell/platform/darwin/ios/framework/Source/FlutterMetalLayer.mm index 7a3a7d99e28ab..8a7551dca42c0 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterMetalLayer.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterMetalLayer.mm @@ -424,15 +424,14 @@ - (void)returnTexture:(FlutterTexture*)texture { } + (BOOL)enabled { - static BOOL enabled = NO; + static BOOL enabled = YES; static BOOL didCheckInfoPlist = NO; if (!didCheckInfoPlist) { didCheckInfoPlist = YES; NSNumber* use_flutter_metal_layer = [[NSBundle mainBundle] objectForInfoDictionaryKey:@"FLTUseFlutterMetalLayer"]; - if (use_flutter_metal_layer != nil && [use_flutter_metal_layer boolValue]) { - enabled = YES; - FML_LOG(WARNING) << "Using FlutterMetalLayer. This is an experimental feature."; + if (use_flutter_metal_layer != nil && ![use_flutter_metal_layer boolValue]) { + enabled = NO; } } return enabled; From 95e9885f7c03c38ee1a591bb1ef4b861c609841a Mon Sep 17 00:00:00 2001 From: skia-flutter-autoroll Date: Wed, 24 Jul 2024 19:12:03 -0400 Subject: [PATCH 3/8] Roll Skia from 55ecdde3a5fa to 746d444f3efd (2 revisions) (#54091) https://skia.googlesource.com/skia.git/+log/55ecdde3a5fa..746d444f3efd 2024-07-24 johnstiles@google.com Avoid using optional<> for ModuleType. 2024-07-24 johnstiles@google.com Add Analysis::FindSpecializedArgumentsForCall helper function. If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/skia-flutter-autoroll Please CC brianosman@google.com,jamesgk@google.com,jonahwilliams@google.com,rmistry@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Skia: https://bugs.chromium.org/p/skia/issues/entry 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://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md --- DEPS | 2 +- ci/licenses_golden/licenses_skia | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/DEPS b/DEPS index 929ba2d9ae6e9..ba3cd1723639a 100644 --- a/DEPS +++ b/DEPS @@ -14,7 +14,7 @@ vars = { 'flutter_git': 'https://flutter.googlesource.com', 'skia_git': 'https://skia.googlesource.com', 'llvm_git': 'https://llvm.googlesource.com', - 'skia_revision': '55ecdde3a5fab967bb40701a789076dfec9c1c6c', + 'skia_revision': '746d444f3efdc41216d94ae53b07bac3c949f887', # WARNING: DO NOT EDIT canvaskit_cipd_instance MANUALLY # See `lib/web_ui/README.md` for how to roll CanvasKit to a new version. diff --git a/ci/licenses_golden/licenses_skia b/ci/licenses_golden/licenses_skia index cc1d01cc03488..93c5d74324a87 100644 --- a/ci/licenses_golden/licenses_skia +++ b/ci/licenses_golden/licenses_skia @@ -1,4 +1,4 @@ -Signature: 4f505e90f0b36e0e81743d46ac5e4c07 +Signature: d2579775e0789348764217fa28a61cff ==================================================================================================== LIBRARY: etc1 From 97b5c9549d1a1a94c33fc47eb4a0c211816171b0 Mon Sep 17 00:00:00 2001 From: Zachary Anderson Date: Wed, 24 Jul 2024 16:15:56 -0700 Subject: [PATCH 4/8] [et] Better RBE defaults (#54059) This PR adopts some RBE configuration from the way that chromium uses RBE https://chromium.googlesource.com/chromium/tools/depot_tools.git/+/refs/heads/main/reclient_helper.py These changes should bias both local and CI builds more towards using the worker pool, which we recently expanded, and should help limit the bandwidth used, which is a bottleneck for build times on a slow connection. --- tools/engine_tool/test/utils.dart | 29 +++- tools/gn | 9 ++ .../lib/src/build_config_runner.dart | 116 ++++++++++++++-- .../test/build_config_runner_test.dart | 129 ++++++++++++++++++ 4 files changed, 273 insertions(+), 10 deletions(-) diff --git a/tools/engine_tool/test/utils.dart b/tools/engine_tool/test/utils.dart index db5d2c95dab97..f9f94bac7ebe3 100644 --- a/tools/engine_tool/test/utils.dart +++ b/tools/engine_tool/test/utils.dart @@ -35,6 +35,10 @@ class CannedProcess { FakeProcess get fakeProcess { return FakeProcess(exitCode: _exitCode, stdout: _stdout, stderr: _stderr); } + + io.ProcessResult get processResult { + return io.ProcessResult(0, _exitCode, _stdout, _stderr); + } } /// ExecutedProcess includes the command and the result. @@ -79,7 +83,19 @@ class TestEnvironment { }); return processResult; }, onRun: (List command) { - throw UnimplementedError('onRun'); + final io.ProcessResult result = _getCannedProcessResult( + command, cannedProcesses, + ); + processHistory.add(ExecutedProcess( + command, + FakeProcess( + exitCode: result.exitCode, + stdout: result.stdout as String, + stderr: result.stderr as String, + ), + result.exitCode, + )); + return result; })), logger: logger, verbose: verbose, @@ -184,6 +200,17 @@ FakeProcess _getCannedResult( return FakeProcess(); } +io.ProcessResult _getCannedProcessResult( + List command, List cannedProcesses) { + for (final CannedProcess cp in cannedProcesses) { + final bool matched = cp.commandMatcher(command); + if (matched) { + return cp.processResult; + } + } + return io.ProcessResult(0, 0, '', ''); +} + typedef CommandMatcher = bool Function(List command); /// Returns a [Matcher] that fails the test if no process has a matching command. diff --git a/tools/gn b/tools/gn index ac3cd935881a0..75dd36d0c3319 100755 --- a/tools/gn +++ b/tools/gn @@ -226,6 +226,15 @@ def setup_rbe(args): # care of starting and stopping the compiler proxy. running_on_luci = os.environ.get('LUCI_CONTEXT') is not None + # The default is 'racing', which eagerly attempts to use the local machine + # to run build actions. This is not a performance improvement in CI where the + # 'local machine' is nearly always a VM anyway, and is not any more powerful + # than the 'remote' RBE workers. Only attempt a 'local' build if the remote + # worker times-out or otherwise fails. See also docs on RbeExecStrategy in the + # engine_build_config package under tools/pkg. + if running_on_luci: + rbe_gn_args['rbe_exec_strategy'] = 'remote_local_fallback' + if args.rbe_server_address: rbe_gn_args['rbe_server_address'] = args.rbe_server_address if args.rbe_exec_strategy: diff --git a/tools/pkg/engine_build_configs/lib/src/build_config_runner.dart b/tools/pkg/engine_build_configs/lib/src/build_config_runner.dart index df702191f8548..20068571658e8 100644 --- a/tools/pkg/engine_build_configs/lib/src/build_config_runner.dart +++ b/tools/pkg/engine_build_configs/lib/src/build_config_runner.dart @@ -5,7 +5,7 @@ import 'dart:async'; import 'dart:convert'; import 'dart:ffi' as ffi; -import 'dart:io' as io show Directory, File, Process; +import 'dart:io' as io show Directory, File, Process, ProcessResult; import 'dart:math'; import 'package:path/path.dart' as p; @@ -14,9 +14,9 @@ import 'package:process_runner/process_runner.dart'; import 'build_config.dart'; -/// The base clase for events generated by a command. +/// The base class for events generated by a command. sealed class RunnerEvent { - RunnerEvent(this.name, this.command, this.timestamp); + RunnerEvent(this.name, this.command, this.timestamp, [this.environment]); /// The name of the task or command. final String name; @@ -26,11 +26,14 @@ sealed class RunnerEvent { /// When the event happened. final DateTime timestamp; + + /// Additional environment variables set during the command, if any. + final Map? environment; } /// A [RunnerEvent] representing the start of a command. final class RunnerStart extends RunnerEvent { - RunnerStart(super.name, super.command, super.timestamp); + RunnerStart(super.name, super.command, super.timestamp, [super.environment]); @override String toString() { @@ -120,6 +123,85 @@ final class RunnerError extends RunnerEvent { } } +/// The strategy that RBE uses for local vs. remote actions. +enum RbeExecStrategy { + /// On a cache miss, all actions will be executed locally. + local, + + /// On a cache miss, actions will run both remotely and locally (capacity + /// allowing), with the action completing when the faster of the two finishes. + racing, + + /// On a cache miss, all actions will be executed remotely. + remote, + + /// On a cache miss, actions will be executed locally if the latency of the + /// remote action completing is too high. + remoteLocalFallback; + + @override + String toString() => switch (this) { + RbeExecStrategy.local => 'local', + RbeExecStrategy.racing => 'racing', + RbeExecStrategy.remote => 'remote', + RbeExecStrategy.remoteLocalFallback => 'remote_local_fallback', + }; +} + +/// Configuration options that affect how RBE works. +class RbeConfig { + const RbeConfig({ + this.remoteDisabled = false, + this.execStrategy = RbeExecStrategy.racing, + this.racingBias = 0.95, + this.localResourceFraction = 0.2, + }); + + /// Whether all remote queries/actions are disabled. + /// + /// When this is true, not even the remote cache will be queried. All actions + /// will be performed locally. + final bool remoteDisabled; + + /// The RBE execution strategy. + final RbeExecStrategy execStrategy; + + /// When the RBE execution strategy is 'racing', how much to bias towards + /// local vs. remote. + /// + /// Values should be in the range of [0, 1]. Closer to 1 implies biasing + /// towards remote. + final double racingBias; + + /// When the RBE execution strategy is 'racing', how much of the local + /// machine to use. + final double localResourceFraction; + + /// Environment variables to set when running RBE related subprocesses. + /// Defaults mirroring: + /// https://chromium.googlesource.com/chromium/tools/depot_tools.git/+/refs/heads/main/reclient_helper.py + Map get environment => { + if (remoteDisabled) + 'RBE_remote_disabled': '1' + else ...{ + 'RBE_exec_strategy': execStrategy.toString(), + if (execStrategy == RbeExecStrategy.racing) ...{ + 'RBE_racing_bias': racingBias.toString(), + 'RBE_local_resource_fraction': localResourceFraction.toString(), + }, + }, + // Reduce the cas concurrency. Lower value doesn't impact + // performance when on high-speed connection, but does show improvements + // on easily congested networks. + 'RBE_cas_concurrency': '100', + // Enable the deps cache. Mac needs a larger deps cache as it + // seems to have larger dependency sets per action. A larger deps cache + // on other platforms doesn't necessarily help but also won't hurt. + 'RBE_enable_deps_cache': '1', + 'RBE_deps_cache_max_mb': '1024', + }; +} + /// The type of a callback that handles [RunnerEvent]s while a [Runner] /// is executing its `run()` method. typedef RunnerEventHandler = void Function(RunnerEvent); @@ -190,6 +272,7 @@ final class BuildRunner extends Runner { ffi.Abi? abi, required io.Directory engineSrcDir, required this.build, + this.rbeConfig = const RbeConfig(), this.concurrency = 0, this.extraGnArgs = const [], this.extraNinjaArgs = const [], @@ -210,6 +293,9 @@ final class BuildRunner extends Runner { /// The [Build] to run. final Build build; + /// Configuration options for RBE. + final RbeConfig rbeConfig; + /// The maximum number of concurrent jobs. /// /// This currently only applies to the ninja build, passed as the -j @@ -450,14 +536,22 @@ final class BuildRunner extends Runner { '${build.name}: RBE ${shutdown ? 'shutdown' : 'startup'}', bootstrapCommand, DateTime.now(), + rbeConfig.environment, )); final ProcessRunnerResult bootstrapResult; if (dryRun) { bootstrapResult = _dryRunResult; } else { - bootstrapResult = await processRunner.runProcess( + final io.ProcessResult result = await processRunner.processManager.run( bootstrapCommand, - failOk: true, + environment: rbeConfig.environment, + ); + bootstrapResult = ProcessRunnerResult( + result.exitCode, + (result.stdout as String).codeUnits, // stdout. + (result.stderr as String).codeUnits, // stderr. + [], // combined, + pid: result.pid, // pid, ); } String okMessage = bootstrapResult.stdout.trim(); @@ -515,9 +609,12 @@ final class BuildRunner extends Runner { ...extraNinjaArgs, ...build.ninja.targets, ]; - eventHandler( - RunnerStart('${build.name}: ninja', command, DateTime.now()), - ); + eventHandler(RunnerStart( + '${build.name}: ninja', + command, + DateTime.now(), + rbeConfig.environment, + )); final ProcessRunnerResult processResult; if (dryRun) { processResult = _dryRunResult; @@ -525,6 +622,7 @@ final class BuildRunner extends Runner { final io.Process process = await processRunner.processManager.start( command, workingDirectory: engineSrcDir.path, + environment: rbeConfig.environment, ); final List stderrOutput = []; final List stdoutOutput = []; diff --git a/tools/pkg/engine_build_configs/test/build_config_runner_test.dart b/tools/pkg/engine_build_configs/test/build_config_runner_test.dart index b2e28d2072213..226be900e4979 100644 --- a/tools/pkg/engine_build_configs/test/build_config_runner_test.dart +++ b/tools/pkg/engine_build_configs/test/build_config_runner_test.dart @@ -283,6 +283,135 @@ void main() { expect(events[5].name, equals('$buildName: ninja')); }); + test('GlobalBuildRunner sets default RBE env vars in an RBE build', () async { + final Build targetBuild = buildConfig.builds[0]; + final BuildRunner buildRunner = BuildRunner( + platform: FakePlatform( + operatingSystem: Platform.linux, + numberOfProcessors: 32, + ), + processRunner: ProcessRunner( + processManager: _fakeProcessManager(), + ), + abi: ffi.Abi.linuxX64, + engineSrcDir: engine.srcDir, + build: targetBuild, + concurrency: 500, + extraGnArgs: ['--rbe'], + dryRun: true, + ); + final List events = []; + void handler(RunnerEvent event) => events.add(event); + final bool runResult = await buildRunner.run(handler); + + final String buildName = targetBuild.name; + + expect(runResult, isTrue); + + // Check that the events for the Ninja command are correct. + expect(events[4] is RunnerStart, isTrue); + expect(events[4].name, equals('$buildName: ninja')); + expect(events[4].environment, isNotNull); + expect(events[4].environment!.containsKey('RBE_exec_strategy'), isTrue); + expect( + events[4].environment!['RBE_exec_strategy'], + equals(RbeExecStrategy.racing.toString()), + ); + expect(events[4].environment!.containsKey('RBE_racing_bias'), isTrue); + expect(events[4].environment!['RBE_racing_bias'], equals('0.95')); + expect( + events[4].environment!.containsKey('RBE_local_resource_fraction'), + isTrue, + ); + expect( + events[4].environment!['RBE_local_resource_fraction'], + equals('0.2'), + ); + }); + + test('GlobalBuildRunner sets RBE_disable_remote when remote builds are disabled', () async { + final Build targetBuild = buildConfig.builds[0]; + final BuildRunner buildRunner = BuildRunner( + platform: FakePlatform( + operatingSystem: Platform.linux, + numberOfProcessors: 32, + ), + processRunner: ProcessRunner( + processManager: _fakeProcessManager(), + ), + abi: ffi.Abi.linuxX64, + engineSrcDir: engine.srcDir, + build: targetBuild, + concurrency: 500, + rbeConfig: const RbeConfig(remoteDisabled: true), + extraGnArgs: ['--rbe'], + dryRun: true, + ); + final List events = []; + void handler(RunnerEvent event) => events.add(event); + final bool runResult = await buildRunner.run(handler); + + final String buildName = targetBuild.name; + + expect(runResult, isTrue); + + // Check that the events for the Ninja command are correct. + expect(events[4] is RunnerStart, isTrue); + expect(events[4].name, equals('$buildName: ninja')); + expect(events[4].environment, isNotNull); + expect(events[4].environment!.containsKey('RBE_remote_disabled'), isTrue); + expect(events[4].environment!['RBE_remote_disabled'], equals('1')); + expect(events[4].environment!.containsKey('RBE_exec_strategy'), isFalse); + expect(events[4].environment!.containsKey('RBE_racing_bias'), isFalse); + expect( + events[4].environment!.containsKey('RBE_local_resource_fraction'), + isFalse, + ); + }); + + test('GlobalBuildRunner sets RBE_exec_strategy when a non-default value is passed in the RbeConfig', () async { + final Build targetBuild = buildConfig.builds[0]; + final BuildRunner buildRunner = BuildRunner( + platform: FakePlatform( + operatingSystem: Platform.linux, + numberOfProcessors: 32, + ), + processRunner: ProcessRunner( + processManager: _fakeProcessManager(), + ), + abi: ffi.Abi.linuxX64, + engineSrcDir: engine.srcDir, + build: targetBuild, + concurrency: 500, + rbeConfig: const RbeConfig(execStrategy: RbeExecStrategy.local), + extraGnArgs: ['--rbe'], + dryRun: true, + ); + final List events = []; + void handler(RunnerEvent event) => events.add(event); + final bool runResult = await buildRunner.run(handler); + + final String buildName = targetBuild.name; + + expect(runResult, isTrue); + + // Check that the events for the Ninja command are correct. + expect(events[4] is RunnerStart, isTrue); + expect(events[4].name, equals('$buildName: ninja')); + expect(events[4].environment, isNotNull); + expect(events[4].environment!.containsKey('RBE_remote_disabled'), isFalse); + expect(events[4].environment!.containsKey('RBE_exec_strategy'), isTrue); + expect( + events[4].environment!['RBE_exec_strategy'], + equals(RbeExecStrategy.local.toString()), + ); + expect(events[4].environment!.containsKey('RBE_racing_bias'), isFalse); + expect( + events[4].environment!.containsKey('RBE_local_resource_fraction'), + isFalse, + ); + }); + test('GlobalBuildRunner passes the specified -j when explicitly provided in a non-RBE build', () async { final Build targetBuild = buildConfig.builds[0]; final BuildRunner buildRunner = BuildRunner( From 805a1578169a69bd88b262141e19062fff7c4910 Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Wed, 24 Jul 2024 18:03:16 -0700 Subject: [PATCH 5/8] Remove incorrect line (#54021) ## Description Removes a line that shadows setting the device type correctly for keyboard events. ## Related Issues - Fixes https://github.com/flutter/flutter/issues/151308 --- .../android/KeyEmbedderResponder.java | 1 - .../android/KeyboardManagerTest.java | 87 ++++++++++++++++++- .../test/io/flutter/util/FakeKeyEvent.java | 6 ++ 3 files changed, 92 insertions(+), 2 deletions(-) diff --git a/shell/platform/android/io/flutter/embedding/android/KeyEmbedderResponder.java b/shell/platform/android/io/flutter/embedding/android/KeyEmbedderResponder.java index eb42164d833ad..de5af6b0ff013 100644 --- a/shell/platform/android/io/flutter/embedding/android/KeyEmbedderResponder.java +++ b/shell/platform/android/io/flutter/embedding/android/KeyEmbedderResponder.java @@ -369,7 +369,6 @@ private boolean handleEventImpl( output.physicalKey = physicalKey; output.character = character; output.synthesized = false; - output.deviceType = KeyData.DeviceType.kKeyboard; sendKeyEvent(output, onKeyEventHandledCallback); for (final Runnable postSyncEvent : postSynchronizeEvents) { diff --git a/shell/platform/android/test/io/flutter/embedding/android/KeyboardManagerTest.java b/shell/platform/android/test/io/flutter/embedding/android/KeyboardManagerTest.java index 8cf0c7e4629ab..d59d01ffb5e8c 100644 --- a/shell/platform/android/test/io/flutter/embedding/android/KeyboardManagerTest.java +++ b/shell/platform/android/test/io/flutter/embedding/android/KeyboardManagerTest.java @@ -14,6 +14,7 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import android.view.InputDevice; import android.view.KeyCharacterMap; import android.view.KeyEvent; import androidx.annotation.NonNull; @@ -88,7 +89,7 @@ enum Kind { /** * Construct an empty call record. * - *

Use the static functions to constuct specific types instead. + *

Use the static functions to construct specific types instead. */ private CallRecord() {} @@ -1921,4 +1922,88 @@ public void getKeyboardState() { new FakeKeyEvent(ACTION_UP, SCAN_KEY_A, KEYCODE_A, 0, 'a', 0)); assertEquals(tester.keyboardManager.getKeyboardState(), Map.of()); } + + @Test + public void deviceTypeFromInputDevice() { + final KeyboardTester tester = new KeyboardTester(); + final ArrayList calls = new ArrayList<>(); + + tester.recordEmbedderCallsTo(calls); + tester.respondToTextInputWith(true); + + // Keyboard + final KeyEvent keyboardEvent = + new FakeKeyEvent( + ACTION_DOWN, SCAN_KEY_A, KEYCODE_A, 0, 'a', 0, InputDevice.SOURCE_KEYBOARD); + assertEquals(true, tester.keyboardManager.handleEvent(keyboardEvent)); + verifyEmbedderEvents( + calls, + new KeyData[] { + buildKeyData(Type.kDown, PHYSICAL_KEY_A, LOGICAL_KEY_A, "a", false, DeviceType.kKeyboard), + }); + calls.clear(); + + // Directional pad + final KeyEvent directionalPadEvent = + new FakeKeyEvent( + ACTION_DOWN, SCAN_ARROW_LEFT, KEYCODE_DPAD_LEFT, 0, '\0', 0, InputDevice.SOURCE_DPAD); + assertEquals(true, tester.keyboardManager.handleEvent(directionalPadEvent)); + verifyEmbedderEvents( + calls, + new KeyData[] { + buildKeyData( + Type.kDown, + PHYSICAL_ARROW_LEFT, + LOGICAL_ARROW_LEFT, + null, + false, + DeviceType.kDirectionalPad), + }); + calls.clear(); + + // Gamepad + final KeyEvent gamepadEvent = + new FakeKeyEvent( + ACTION_DOWN, SCAN_ARROW_LEFT, KEYCODE_BUTTON_A, 0, '\0', 0, InputDevice.SOURCE_GAMEPAD); + assertEquals(true, tester.keyboardManager.handleEvent(gamepadEvent)); + verifyEmbedderEvents( + calls, + new KeyData[] { + buildKeyData( + Type.kUp, PHYSICAL_ARROW_LEFT, LOGICAL_ARROW_LEFT, null, true, DeviceType.kKeyboard), + buildKeyData( + Type.kDown, + PHYSICAL_ARROW_LEFT, + LOGICAL_GAME_BUTTON_A, + null, + false, + DeviceType.kGamepad), + }); + calls.clear(); + + // HDMI + final KeyEvent hdmiEvent = + new FakeKeyEvent( + ACTION_DOWN, SCAN_ARROW_LEFT, KEYCODE_BUTTON_A, 0, '\0', 0, InputDevice.SOURCE_HDMI); + assertEquals(true, tester.keyboardManager.handleEvent(hdmiEvent)); + verifyEmbedderEvents( + calls, + new KeyData[] { + buildKeyData( + Type.kUp, + PHYSICAL_ARROW_LEFT, + LOGICAL_GAME_BUTTON_A, + null, + true, + DeviceType.kKeyboard), + buildKeyData( + Type.kDown, + PHYSICAL_ARROW_LEFT, + LOGICAL_GAME_BUTTON_A, + null, + false, + DeviceType.kHdmi), + }); + calls.clear(); + } } diff --git a/shell/platform/android/test/io/flutter/util/FakeKeyEvent.java b/shell/platform/android/test/io/flutter/util/FakeKeyEvent.java index 4faa39107f50b..35ef6731d6ec8 100644 --- a/shell/platform/android/test/io/flutter/util/FakeKeyEvent.java +++ b/shell/platform/android/test/io/flutter/util/FakeKeyEvent.java @@ -20,6 +20,12 @@ public FakeKeyEvent( this.character = character; } + public FakeKeyEvent( + int action, int scancode, int code, int repeat, char character, int metaState, int source) { + super(0, 0, action, code, repeat, metaState, 0, scancode, 0, source); + this.character = character; + } + private char character = 0; public final int getUnicodeChar() { From a57655cccb55b373728ca8c13d72c97e4cd6c397 Mon Sep 17 00:00:00 2001 From: skia-flutter-autoroll Date: Wed, 24 Jul 2024 21:17:27 -0400 Subject: [PATCH 6/8] Roll Dart SDK from 0b3c00feefb1 to 693848f200d7 (1 revision) (#54092) https://dart.googlesource.com/sdk.git/+log/0b3c00feefb1..693848f200d7 2024-07-24 dart-internal-merge@dart-ci-internal.iam.gserviceaccount.com Version 3.6.0-78.0.dev If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/dart-sdk-flutter-engine Please CC dart-vm-team@google.com,jonahwilliams@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter Engine: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md --- DEPS | 2 +- ci/licenses_golden/licenses_dart | 4 ++-- sky/packages/sky_engine/LICENSE | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/DEPS b/DEPS index ba3cd1723639a..1b2a0ab7a1a88 100644 --- a/DEPS +++ b/DEPS @@ -56,7 +56,7 @@ vars = { # Dart is: https://github.com/dart-lang/sdk/blob/main/DEPS # You can use //tools/dart/create_updated_flutter_deps.py to produce # updated revision list of existing dependencies. - 'dart_revision': '0b3c00feefb1e35ebbe5e4860394712d24e3ffa7', + 'dart_revision': '693848f200d710d33b7acdce28667ca2c3257221', # WARNING: DO NOT EDIT MANUALLY # The lines between blank lines above and below are generated by a script. See create_updated_flutter_deps.py diff --git a/ci/licenses_golden/licenses_dart b/ci/licenses_golden/licenses_dart index 8d61032404d45..5eb5cda3c5e39 100644 --- a/ci/licenses_golden/licenses_dart +++ b/ci/licenses_golden/licenses_dart @@ -1,4 +1,4 @@ -Signature: 85b71a1ff50f67a725d4a49dcdab9ec2 +Signature: 66d4558807d9e0f9bba2fde2750c9559 ==================================================================================================== LIBRARY: dart @@ -4773,7 +4773,7 @@ Exhibit B - "Incompatible With Secondary Licenses" Notice This Source Code Form is "Incompatible With Secondary Licenses", as defined by the Mozilla Public License, v. 2.0. -You may obtain a copy of this library's Source Code Form from: https://dart.googlesource.com/sdk/+/e53beb0390934b2c9b67d011615d8c6794454c92 +You may obtain a copy of this library's Source Code Form from: https://dart.googlesource.com/sdk/+/693848f200d710d33b7acdce28667ca2c3257221 /third_party/fallback_root_certificates/ ==================================================================================================== diff --git a/sky/packages/sky_engine/LICENSE b/sky/packages/sky_engine/LICENSE index e1d7bc64f732f..848b240603c40 100644 --- a/sky/packages/sky_engine/LICENSE +++ b/sky/packages/sky_engine/LICENSE @@ -31926,7 +31926,7 @@ Exhibit B - "Incompatible With Secondary Licenses" Notice This Source Code Form is "Incompatible With Secondary Licenses", as defined by the Mozilla Public License, v. 2.0. -You may obtain a copy of this library's Source Code Form from: https://dart.googlesource.com/sdk/+/0b3c00feefb1e35ebbe5e4860394712d24e3ffa7 +You may obtain a copy of this library's Source Code Form from: https://dart.googlesource.com/sdk/+/693848f200d710d33b7acdce28667ca2c3257221 /third_party/fallback_root_certificates/ -------------------------------------------------------------------------------- From f4f0bf3fe8118c5f58e2f39fa46a8a75e81354b4 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Wed, 24 Jul 2024 19:34:16 -0700 Subject: [PATCH 7/8] Disable FlutterMetalLayer by default. (#54095) Backing textures aren't getting deallocated, which is causing memory leaks and crashing devicelab. --- .../darwin/ios/framework/Source/FlutterMetalLayer.mm | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterMetalLayer.mm b/shell/platform/darwin/ios/framework/Source/FlutterMetalLayer.mm index 8a7551dca42c0..a58b1c0bf4a61 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterMetalLayer.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterMetalLayer.mm @@ -424,14 +424,14 @@ - (void)returnTexture:(FlutterTexture*)texture { } + (BOOL)enabled { - static BOOL enabled = YES; + static BOOL enabled = NO; static BOOL didCheckInfoPlist = NO; if (!didCheckInfoPlist) { didCheckInfoPlist = YES; NSNumber* use_flutter_metal_layer = [[NSBundle mainBundle] objectForInfoDictionaryKey:@"FLTUseFlutterMetalLayer"]; - if (use_flutter_metal_layer != nil && ![use_flutter_metal_layer boolValue]) { - enabled = NO; + if (use_flutter_metal_layer != nil && [use_flutter_metal_layer boolValue]) { + enabled = YES; } } return enabled; From 74737820a8ee57f893ec325c1fd221aa8b8557fd Mon Sep 17 00:00:00 2001 From: Yegor Date: Wed, 24 Jul 2024 19:42:05 -0700 Subject: [PATCH 8/8] [web] better class names for semantics (#54070) A few non-functional clean-ups in semantics: * Rename `RoleManager` to `SemanticBehavior`. * Rename `PrimaryRoleManager` to `SemanticRole`. * Remove the `Role` enum. Move the enum docs into the respective classes. ## Why? Previous naming was confusing. It's not clear what the difference is between a "role manager" and a "primary role manager". The word "manager" is a meaningless addition; the `Semantic*` prefix is much more meaningful. The `Role` enum was only used for tests, but tests can just use `SemanticRole.runtimeType`. ## New state of the world After this PR the semantics system has "objects" (class `SemanticsObject`), "roles" (class `SemanticRole`), and "behaviors" (class `SemanticBehavior`). - A semantic _object_ is an object attached to the framework-side `SemanticNode`. It lives as long as the semantic node does, and provides basic functionality that's common across all nodes. - A semantic object has exactly one semantic _role_. This role is determined from the flags set on the semantic node. Flags can change, causing a semantic object to change its role, which is why these are two separate classes. If an object had just one permanent role, we could combine these classes into one (maybe one day we'll do it, as changing roles dynamically is weird, but that needs major changes in the framework). - A semantic role may have zero or more semantic _behaviors_. A behavior supplies a piece of functionality, such as focusability, clickability/tappability, live regions, etc. A behavior can be shared by multiple roles. For example, both `Button` and `Checkable` roles use the `Tappable` behavior. This is why there's a many-to-many relationship between roles and behaviors. Or in entity relationship terms: ```mermaid --- title: Semantic object relationships --- erDiagram SemanticsNode ||--|| SemanticsObject : managed-by SemanticsObject ||--o{ SemanticRole : has-a SemanticRole }o--o{ SemanticBehavior : has ``` --- .../lib/src/engine/semantics/checkable.dart | 6 +- .../lib/src/engine/semantics/dialog.dart | 42 +-- .../lib/src/engine/semantics/focusable.dart | 27 +- .../lib/src/engine/semantics/heading.dart | 6 +- .../lib/src/engine/semantics/image.dart | 10 +- .../src/engine/semantics/incrementable.dart | 6 +- .../src/engine/semantics/label_and_value.dart | 29 +- lib/web_ui/lib/src/engine/semantics/link.dart | 6 +- .../lib/src/engine/semantics/live_region.dart | 16 +- .../src/engine/semantics/platform_view.dart | 6 +- .../lib/src/engine/semantics/scrollable.dart | 6 +- .../lib/src/engine/semantics/semantics.dart | 282 ++++++++---------- .../lib/src/engine/semantics/tappable.dart | 19 +- .../lib/src/engine/semantics/text_field.dart | 14 +- .../test/engine/semantics/semantics_test.dart | 124 ++++---- .../engine/semantics/semantics_tester.dart | 11 +- .../engine/semantics/semantics_text_test.dart | 16 +- .../engine/semantics/text_field_test.dart | 16 +- 18 files changed, 308 insertions(+), 334 deletions(-) diff --git a/lib/web_ui/lib/src/engine/semantics/checkable.dart b/lib/web_ui/lib/src/engine/semantics/checkable.dart index 30b7928a131cb..dcc6b6c42c56c 100644 --- a/lib/web_ui/lib/src/engine/semantics/checkable.dart +++ b/lib/web_ui/lib/src/engine/semantics/checkable.dart @@ -49,11 +49,11 @@ _CheckableKind _checkableKindFromSemanticsFlag( /// See also [ui.SemanticsFlag.hasCheckedState], [ui.SemanticsFlag.isChecked], /// [ui.SemanticsFlag.isInMutuallyExclusiveGroup], [ui.SemanticsFlag.isToggled], /// [ui.SemanticsFlag.hasToggledState] -class Checkable extends PrimaryRoleManager { - Checkable(SemanticsObject semanticsObject) +class SemanticCheckable extends SemanticRole { + SemanticCheckable(SemanticsObject semanticsObject) : _kind = _checkableKindFromSemanticsFlag(semanticsObject), super.withBasics( - PrimaryRole.checkable, + SemanticRoleKind.checkable, semanticsObject, preferredLabelRepresentation: LabelRepresentation.ariaLabel, ) { diff --git a/lib/web_ui/lib/src/engine/semantics/dialog.dart b/lib/web_ui/lib/src/engine/semantics/dialog.dart index 0eab7758a1d93..84896677dd21b 100644 --- a/lib/web_ui/lib/src/engine/semantics/dialog.dart +++ b/lib/web_ui/lib/src/engine/semantics/dialog.dart @@ -6,12 +6,10 @@ import '../dom.dart'; import '../semantics.dart'; import '../util.dart'; -/// Provides accessibility for dialogs. -/// -/// See also [Role.dialog]. -class Dialog extends PrimaryRoleManager { - Dialog(SemanticsObject semanticsObject) : super.blank(PrimaryRole.dialog, semanticsObject) { - // The following secondary roles can coexist with dialog. Generic `RouteName` +/// Provides accessibility for routes, including dialogs and pop-up menus. +class SemanticDialog extends SemanticRole { + SemanticDialog(SemanticsObject semanticsObject) : super.blank(SemanticRoleKind.dialog, semanticsObject) { + // The following behaviors can coexist with dialog. Generic `RouteName` // and `LabelAndValue` are not used by this role because when the dialog // names its own route an `aria-label` is used instead of `aria-describedby`. addFocusManagement(); @@ -39,14 +37,14 @@ class Dialog extends PrimaryRoleManager { void _setDefaultFocus() { semanticsObject.visitDepthFirstInTraversalOrder((SemanticsObject node) { - final PrimaryRoleManager? roleManager = node.primaryRole; - if (roleManager == null) { + final SemanticRole? role = node.semanticRole; + if (role == null) { return true; } // If the node does not take focus (e.g. focusing on it does not make // sense at all). Despair not. Keep looking. - final bool didTakeFocus = roleManager.focusAsRouteDefault(); + final bool didTakeFocus = role.focusAsRouteDefault(); return !didTakeFocus; }); } @@ -99,14 +97,18 @@ class Dialog extends PrimaryRoleManager { } } -/// Supplies a description for the nearest ancestor [Dialog]. -class RouteName extends RoleManager { - RouteName( - SemanticsObject semanticsObject, - PrimaryRoleManager owner, - ) : super(Role.routeName, semanticsObject, owner); +/// Supplies a description for the nearest ancestor [SemanticDialog]. +/// +/// This role is assigned to nodes that have `namesRoute` set but not +/// `scopesRoute`. When both flags are set the node only gets the [SemanticDialog] role. +/// +/// If the ancestor dialog is missing, this role has no effect. It is up to the +/// framework, widget, and app authors to make sure a route name is scoped under +/// a route. +class RouteName extends SemanticBehavior { + RouteName(super.semanticsObject, super.owner); - Dialog? _dialog; + SemanticDialog? _dialog; @override void update() { @@ -124,7 +126,7 @@ class RouteName extends RoleManager { } if (semanticsObject.isLabelDirty) { - final Dialog? dialog = _dialog; + final SemanticDialog? dialog = _dialog; if (dialog != null) { // Already attached to a dialog, just update the description. dialog.describeBy(this); @@ -143,11 +145,11 @@ class RouteName extends RoleManager { void _lookUpNearestAncestorDialog() { SemanticsObject? parent = semanticsObject.parent; - while (parent != null && parent.primaryRole?.role != PrimaryRole.dialog) { + while (parent != null && parent.semanticRole?.kind != SemanticRoleKind.dialog) { parent = parent.parent; } - if (parent != null && parent.primaryRole?.role == PrimaryRole.dialog) { - _dialog = parent.primaryRole! as Dialog; + if (parent != null && parent.semanticRole?.kind == SemanticRoleKind.dialog) { + _dialog = parent.semanticRole! as SemanticDialog; } } } diff --git a/lib/web_ui/lib/src/engine/semantics/focusable.dart b/lib/web_ui/lib/src/engine/semantics/focusable.dart index 4d10a7570b008..b3289e35da829 100644 --- a/lib/web_ui/lib/src/engine/semantics/focusable.dart +++ b/lib/web_ui/lib/src/engine/semantics/focusable.dart @@ -12,9 +12,9 @@ import 'semantics.dart'; /// Supplies generic accessibility focus features to semantics nodes that have /// [ui.SemanticsFlag.isFocusable] set. /// -/// Assumes that the element being focused on is [SemanticsObject.element]. Role -/// managers with special needs can implement custom focus management and -/// exclude this role manager. +/// Assumes that the element being focused on is [SemanticsObject.element]. +/// Semantic roles with special needs can implement custom focus management and +/// exclude this behavior. /// /// `"tab-index=0"` is used because `` is not intrinsically /// focusable. Examples of intrinsically focusable elements include: @@ -27,10 +27,9 @@ import 'semantics.dart'; /// See also: /// /// * https://developer.mozilla.org/en-US/docs/Web/Accessibility/Keyboard-navigable_JavaScript_widgets -class Focusable extends RoleManager { - Focusable(SemanticsObject semanticsObject, PrimaryRoleManager owner) - : _focusManager = AccessibilityFocusManager(semanticsObject.owner), - super(Role.focusable, semanticsObject, owner); +class Focusable extends SemanticBehavior { + Focusable(super.semanticsObject, super.owner) + : _focusManager = AccessibilityFocusManager(semanticsObject.owner); final AccessibilityFocusManager _focusManager; @@ -44,9 +43,9 @@ class Focusable extends RoleManager { /// programmatically, simulating the screen reader choosing a default element /// to focus on. /// - /// Returns `true` if the role manager took the focus. Returns `false` if - /// this role manager did not take the focus. The return value can be used to - /// decide whether to stop searching for a node that should take focus. + /// Returns `true` if the node took the focus. Returns `false` if the node did + /// not take the focus. The return value can be used to decide whether to stop + /// searching for a node that should take focus. bool focusAsRouteDefault() { _focusManager._lastEvent = AccessibilityFocusManagerEvent.requestedFocus; owner.element.focusWithoutScroll(); @@ -106,10 +105,10 @@ enum AccessibilityFocusManagerEvent { /// /// Unlike [Focusable], which implements focus features on [SemanticsObject]s /// whose [SemanticsObject.element] is directly focusable, this class can help -/// implementing focus features on custom elements. For example, [Incrementable] -/// uses a custom `` tag internally while its root-level element is not -/// focusable. However, it can still use this class to manage the focus of the -/// internal element. +/// implementing focus features on custom elements. For example, +/// [SemanticIncrementable] uses a custom `` tag internally while its +/// root-level element is not focusable. However, it can still use this class to +/// manage the focus of the internal element. class AccessibilityFocusManager { /// Creates a focus manager tied to a specific [EngineSemanticsOwner]. AccessibilityFocusManager(this._owner); diff --git a/lib/web_ui/lib/src/engine/semantics/heading.dart b/lib/web_ui/lib/src/engine/semantics/heading.dart index c1abfe3c0fe08..2088bacd26290 100644 --- a/lib/web_ui/lib/src/engine/semantics/heading.dart +++ b/lib/web_ui/lib/src/engine/semantics/heading.dart @@ -8,9 +8,9 @@ import 'semantics.dart'; /// Renders semantics objects as headings with the corresponding /// level (h1 ... h6). -class Heading extends PrimaryRoleManager { - Heading(SemanticsObject semanticsObject) - : super.blank(PrimaryRole.heading, semanticsObject) { +class SemanticHeading extends SemanticRole { + SemanticHeading(SemanticsObject semanticsObject) + : super.blank(SemanticRoleKind.heading, semanticsObject) { addFocusManagement(); addLiveRegion(); addRouteName(); diff --git a/lib/web_ui/lib/src/engine/semantics/image.dart b/lib/web_ui/lib/src/engine/semantics/image.dart index da36ad2a87b5d..6e79f9050d79b 100644 --- a/lib/web_ui/lib/src/engine/semantics/image.dart +++ b/lib/web_ui/lib/src/engine/semantics/image.dart @@ -10,11 +10,11 @@ import 'semantics.dart'; /// Uses aria img role to convey this semantic information to the element. /// /// Screen-readers takes advantage of "aria-label" to describe the visual. -class ImageRoleManager extends PrimaryRoleManager { - ImageRoleManager(SemanticsObject semanticsObject) - : super.blank(PrimaryRole.image, semanticsObject) { - // The following secondary roles can coexist with images. `LabelAndValue` is - // not used because this role manager uses special auxiliary elements to +class SemanticImage extends SemanticRole { + SemanticImage(SemanticsObject semanticsObject) + : super.blank(SemanticRoleKind.image, semanticsObject) { + // The following behaviors can coexist with images. `LabelAndValue` is + // not used because this behavior uses special auxiliary elements to // supply ARIA labels. // TODO(yjbanov): reevaluate usage of aux elements, https://github.com/flutter/flutter/issues/129317 addFocusManagement(); diff --git a/lib/web_ui/lib/src/engine/semantics/incrementable.dart b/lib/web_ui/lib/src/engine/semantics/incrementable.dart index 65e4a9f5f9e59..c1b6cbd4dc5d6 100644 --- a/lib/web_ui/lib/src/engine/semantics/incrementable.dart +++ b/lib/web_ui/lib/src/engine/semantics/incrementable.dart @@ -19,10 +19,10 @@ import 'semantics.dart'; /// The input element is disabled whenever the gesture mode switches to pointer /// events. This is to prevent the browser from taking over drag gestures. Drag /// gestures must be interpreted by the Flutter framework. -class Incrementable extends PrimaryRoleManager { - Incrementable(SemanticsObject semanticsObject) +class SemanticIncrementable extends SemanticRole { + SemanticIncrementable(SemanticsObject semanticsObject) : _focusManager = AccessibilityFocusManager(semanticsObject.owner), - super.blank(PrimaryRole.incrementable, semanticsObject) { + super.blank(SemanticRoleKind.incrementable, semanticsObject) { // The following generic roles can coexist with incrementables. Generic focus // management is not used by this role because the root DOM element is not // the one being focused on, but the internal `` element. diff --git a/lib/web_ui/lib/src/engine/semantics/label_and_value.dart b/lib/web_ui/lib/src/engine/semantics/label_and_value.dart index 729d3538f29ba..006be31641f1a 100644 --- a/lib/web_ui/lib/src/engine/semantics/label_and_value.dart +++ b/lib/web_ui/lib/src/engine/semantics/label_and_value.dart @@ -48,7 +48,7 @@ enum LabelRepresentation { sizedSpan; /// Creates the behavior for this label representation. - LabelRepresentationBehavior createBehavior(PrimaryRoleManager owner) { + LabelRepresentationBehavior createBehavior(SemanticRole owner) { return switch (this) { ariaLabel => AriaLabelRepresentation._(owner), domText => DomTextRepresentation._(owner), @@ -63,8 +63,8 @@ abstract final class LabelRepresentationBehavior { final LabelRepresentation kind; - /// The role manager that this label representation is attached to. - final PrimaryRoleManager owner; + /// The role that this label representation is attached to. + final SemanticRole owner; /// Convenience getter for the corresponding semantics object. SemanticsObject get semanticsObject => owner.semanticsObject; @@ -109,7 +109,7 @@ abstract final class LabelRepresentationBehavior { /// /// final class AriaLabelRepresentation extends LabelRepresentationBehavior { - AriaLabelRepresentation._(PrimaryRoleManager owner) : super(LabelRepresentation.ariaLabel, owner); + AriaLabelRepresentation._(SemanticRole owner) : super(LabelRepresentation.ariaLabel, owner); String? _previousLabel; @@ -143,7 +143,7 @@ final class AriaLabelRepresentation extends LabelRepresentationBehavior { /// no ARIA role set, or the role does not size the element, then the /// [SizedSpanRepresentation] representation can be used. final class DomTextRepresentation extends LabelRepresentationBehavior { - DomTextRepresentation._(PrimaryRoleManager owner) : super(LabelRepresentation.domText, owner); + DomTextRepresentation._(SemanticRole owner) : super(LabelRepresentation.domText, owner); DomText? _domText; String? _previousLabel; @@ -233,7 +233,7 @@ typedef _Measurement = ({ /// * Use an existing non-text role, e.g. "heading". Sizes correctly, but breaks /// the message (reads "heading"). final class SizedSpanRepresentation extends LabelRepresentationBehavior { - SizedSpanRepresentation._(PrimaryRoleManager owner) : super(LabelRepresentation.sizedSpan, owner) { + SizedSpanRepresentation._(SemanticRole owner) : super(LabelRepresentation.sizedSpan, owner) { _domText.style // `inline-block` is needed for two reasons: // - It supports measuring the true size of the text. Pure `block` would @@ -433,14 +433,13 @@ final class SizedSpanRepresentation extends LabelRepresentationBehavior { DomElement get focusTarget => _domText; } -/// Renders [SemanticsObject.label] and/or [SemanticsObject.value] to the semantics DOM. +/// Renders the label for a [SemanticsObject] that can be scanned by screen +/// readers, web crawlers, and other automated agents. /// -/// The value is not always rendered. Some semantics nodes correspond to -/// interactive controls. In such case the value is reported via that element's -/// `value` attribute rather than rendering it separately. -class LabelAndValue extends RoleManager { - LabelAndValue(SemanticsObject semanticsObject, PrimaryRoleManager owner, { required this.preferredRepresentation }) - : super(Role.labelAndValue, semanticsObject, owner); +/// See [computeDomSemanticsLabel] for the exact logic that constructs the label +/// of a semantic node. +class LabelAndValue extends SemanticBehavior { + LabelAndValue(super.semanticsObject, super.owner, { required this.preferredRepresentation }); /// The preferred representation of the label in the DOM. /// @@ -471,7 +470,7 @@ class LabelAndValue extends RoleManager { /// If the node has children always use an `aria-label`. Using extra child /// nodes to represent the label will cause layout shifts and confuse the /// screen reader. If the are no children, use the representation preferred - /// by the primary role manager. + /// by the role. LabelRepresentationBehavior _getEffectiveRepresentation() { final LabelRepresentation effectiveRepresentation = semanticsObject.hasChildren ? LabelRepresentation.ariaLabel @@ -491,7 +490,7 @@ class LabelAndValue extends RoleManager { /// combination is present. String? _computeLabel() { // If the node is incrementable the value is reported to the browser via - // the respective role manager. We do not need to also render it again here. + // the respective role. We do not need to also render it again here. final bool shouldDisplayValue = !semanticsObject.isIncrementable && semanticsObject.hasValue; return computeDomSemanticsLabel( diff --git a/lib/web_ui/lib/src/engine/semantics/link.dart b/lib/web_ui/lib/src/engine/semantics/link.dart index d957e90d2967b..d13f69fac09d5 100644 --- a/lib/web_ui/lib/src/engine/semantics/link.dart +++ b/lib/web_ui/lib/src/engine/semantics/link.dart @@ -6,9 +6,9 @@ import '../dom.dart'; import '../semantics.dart'; /// Provides accessibility for links. -class Link extends PrimaryRoleManager { - Link(SemanticsObject semanticsObject) : super.withBasics( - PrimaryRole.link, +class SemanticLink extends SemanticRole { + SemanticLink(SemanticsObject semanticsObject) : super.withBasics( + SemanticRoleKind.link, semanticsObject, preferredLabelRepresentation: LabelRepresentation.domText, ) { diff --git a/lib/web_ui/lib/src/engine/semantics/live_region.dart b/lib/web_ui/lib/src/engine/semantics/live_region.dart index 9669f72d8f1dc..25d0291db90c0 100644 --- a/lib/web_ui/lib/src/engine/semantics/live_region.dart +++ b/lib/web_ui/lib/src/engine/semantics/live_region.dart @@ -10,15 +10,21 @@ import 'semantics.dart'; /// Manages semantics configurations that represent live regions. /// -/// Assistive technologies treat "aria-live" attribute differently. To keep -/// the behavior consistent, [AccessibilityAnnouncements.announce] is used. +/// A live region is a region whose changes will be announced by the screen +/// reader without the user moving focus onto the node. +/// +/// Examples of live regions include snackbars and text field errors. Once +/// identified with this role, they will be able to get the assistive +/// technology's attention right away. +/// +/// Different assistive technologies treat "aria-live" attribute differently. To +/// keep the behavior consistent, [AccessibilityAnnouncements.announce] is used. /// /// When there is an update to [LiveRegion], assistive technologies read the /// label of the element. See [LabelAndValue]. If there is no label provided /// no content will be read. -class LiveRegion extends RoleManager { - LiveRegion(SemanticsObject semanticsObject, PrimaryRoleManager owner) - : super(Role.liveRegion, semanticsObject, owner); +class LiveRegion extends SemanticBehavior { + LiveRegion(super.semanticsObject, super.owner); String? _lastAnnouncement; diff --git a/lib/web_ui/lib/src/engine/semantics/platform_view.dart b/lib/web_ui/lib/src/engine/semantics/platform_view.dart index 89b61507a7942..d66ff170bfd63 100644 --- a/lib/web_ui/lib/src/engine/semantics/platform_view.dart +++ b/lib/web_ui/lib/src/engine/semantics/platform_view.dart @@ -20,10 +20,10 @@ import 'semantics.dart'; /// See also: /// * https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-owns /// * https://bugs.webkit.org/show_bug.cgi?id=223798 -class PlatformViewRoleManager extends PrimaryRoleManager { - PlatformViewRoleManager(SemanticsObject semanticsObject) +class SemanticPlatformView extends SemanticRole { + SemanticPlatformView(SemanticsObject semanticsObject) : super.withBasics( - PrimaryRole.platformView, + SemanticRoleKind.platformView, semanticsObject, preferredLabelRepresentation: LabelRepresentation.ariaLabel, ); diff --git a/lib/web_ui/lib/src/engine/semantics/scrollable.dart b/lib/web_ui/lib/src/engine/semantics/scrollable.dart index 03534cf54dd8a..9679b85f37a21 100644 --- a/lib/web_ui/lib/src/engine/semantics/scrollable.dart +++ b/lib/web_ui/lib/src/engine/semantics/scrollable.dart @@ -22,10 +22,10 @@ import 'package:ui/ui.dart' as ui; /// contents is less than the size of the viewport the browser snaps /// "scrollTop" back to zero. If there is more content than available in the /// viewport "scrollTop" may take positive values. -class Scrollable extends PrimaryRoleManager { - Scrollable(SemanticsObject semanticsObject) +class SemanticScrollable extends SemanticRole { + SemanticScrollable(SemanticsObject semanticsObject) : super.withBasics( - PrimaryRole.scrollable, + SemanticRoleKind.scrollable, semanticsObject, preferredLabelRepresentation: LabelRepresentation.ariaLabel, ) { diff --git a/lib/web_ui/lib/src/engine/semantics/semantics.dart b/lib/web_ui/lib/src/engine/semantics/semantics.dart index 1e9f26157b73c..a35651ba210e4 100644 --- a/lib/web_ui/lib/src/engine/semantics/semantics.dart +++ b/lib/web_ui/lib/src/engine/semantics/semantics.dart @@ -343,11 +343,11 @@ class SemanticsNodeUpdate { final String? linkUrl; } -/// Identifies [PrimaryRoleManager] implementations. +/// Identifies [SemanticRole] implementations. /// /// Each value corresponds to the most specific role a semantics node plays in /// the semantics tree. -enum PrimaryRole { +enum SemanticRoleKind { /// Supports incrementing and/or decrementing its value. incrementable, @@ -393,7 +393,7 @@ enum PrimaryRole { /// it as a dialog would be wrong. dialog, - /// The node's primary role is to host a platform view. + /// The node's role is to host a platform view. platformView, /// A role used when a more specific role cannot be assigend to @@ -406,48 +406,15 @@ enum PrimaryRole { link, } -/// Identifies one of the secondary [RoleManager]s of a [PrimaryRoleManager]. -enum Role { - /// Supplies generic accessibility focus features to semantics nodes that have - /// [ui.SemanticsFlag.isFocusable] set. - focusable, - - /// Supplies generic tapping/clicking functionality. - tappable, - - /// Provides an `aria-label` from `label`, `value`, and/or `tooltip` values. - /// - /// The two are combined into the same role because they interact with each - /// other. - labelAndValue, - - /// Contains a region whose changes will be announced to the screen reader - /// without having to be in focus. - /// - /// These regions can be a snackbar or a text field error. Once identified - /// with this role, they will be able to get the assistive technology's - /// attention right away. - liveRegion, - - /// Provides a description for an ancestor dialog. - /// - /// This role is assigned to nodes that have `namesRoute` set but not - /// `scopesRoute`. When both flags are set the node only gets the dialog - /// role (see [dialog]). - /// - /// If the ancestor dialog is missing, this role does nothing useful. - routeName, -} - -/// Responsible for setting the `role` ARIA attribute and for attaching zero or -/// more secondary [RoleManager]s to a [SemanticsObject]. -abstract class PrimaryRoleManager { +/// Responsible for setting the `role` ARIA attribute, for attaching +/// [SemanticBehavior]s, and for supplying behaviors unique to the role. +abstract class SemanticRole { /// Initializes a role for a [semanticsObject] that includes basic /// functionality for focus, labels, live regions, and route names. /// /// If `labelRepresentation` is true, configures the [LabelAndValue] role with /// [LabelAndValue.labelRepresentation] set to true. - PrimaryRoleManager.withBasics(this.role, this.semanticsObject, { required LabelRepresentation preferredLabelRepresentation }) { + SemanticRole.withBasics(this.kind, this.semanticsObject, { required LabelRepresentation preferredLabelRepresentation }) { element = _initElement(createElement(), semanticsObject); addFocusManagement(); addLiveRegion(); @@ -458,29 +425,23 @@ abstract class PrimaryRoleManager { /// Initializes a blank role for a [semanticsObject]. /// /// Use this constructor for highly specialized cases where - /// [RoleManager.withBasics] does not work, for example when the default focus + /// [SemanticRole.withBasics] does not work, for example when the default focus /// management intereferes with the widget's functionality. - PrimaryRoleManager.blank(this.role, this.semanticsObject) { + SemanticRole.blank(this.kind, this.semanticsObject) { element = _initElement(createElement(), semanticsObject); } late final DomElement element; - /// The primary role identifier. - final PrimaryRole role; + /// The kind of the role that this . + final SemanticRoleKind kind; /// The semantics object managed by this role. final SemanticsObject semanticsObject; - /// Secondary role managers, if any. - List? get secondaryRoleManagers => _secondaryRoleManagers; - List? _secondaryRoleManagers; - - /// Identifiers of secondary roles used by this primary role manager. - /// - /// This is only meant to be used in tests. - @visibleForTesting - List get debugSecondaryRoles => _secondaryRoleManagers?.map((RoleManager manager) => manager.role).toList() ?? const []; + /// Semantic behaviors provided by this role, if any. + List? get behaviors => _behaviors; + List? _behaviors; @protected DomElement createElement() => domDocument.createElement('flt-semantics'); @@ -517,16 +478,16 @@ abstract class PrimaryRoleManager { return element; } - /// A lifecycle method called after the DOM [element] for this role manager - /// is initialized, and the association with the corresponding - /// [SemanticsObject] established. + /// A lifecycle method called after the DOM [element] for this role is + /// initialized, and the association with the corresponding [SemanticsObject] + /// established. /// /// Override this method to implement expensive one-time initialization of a - /// role manager's state. It is more efficient to do such work in this method - /// compared to [update], because [update] can be called many times during the + /// role's state. It is more efficient to do such work in this method compared + /// to [update], because [update] can be called many times during the /// lifecycle of the semantic node. /// - /// It is safe to access [element], [semanticsObject], [secondaryRoleManagers] + /// It is safe to access [element], [semanticsObject], [behaviors] /// and all helper methods that access these fields, such as [append], /// [focusable], etc. void initState() {} @@ -551,51 +512,51 @@ abstract class PrimaryRoleManager { void removeEventListener(String type, DomEventListener? listener, [bool? useCapture]) => element.removeEventListener(type, listener, useCapture); - /// Convenience getter for the [Focusable] role manager, if any. + /// Convenience getter for the [Focusable] behavior, if any. Focusable? get focusable => _focusable; Focusable? _focusable; /// Adds generic focus management features. void addFocusManagement() { - addSecondaryRole(_focusable = Focusable(semanticsObject, this)); + addSemanticBehavior(_focusable = Focusable(semanticsObject, this)); } /// Adds generic live region features. void addLiveRegion() { - addSecondaryRole(LiveRegion(semanticsObject, this)); + addSemanticBehavior(LiveRegion(semanticsObject, this)); } /// Adds generic route name features. void addRouteName() { - addSecondaryRole(RouteName(semanticsObject, this)); + addSemanticBehavior(RouteName(semanticsObject, this)); } - /// Convenience getter for the [LabelAndValue] role manager, if any. + /// Convenience getter for the [LabelAndValue] behavior, if any. LabelAndValue? get labelAndValue => _labelAndValue; LabelAndValue? _labelAndValue; /// Adds generic label features. void addLabelAndValue({ required LabelRepresentation preferredRepresentation }) { - addSecondaryRole(_labelAndValue = LabelAndValue(semanticsObject, this, preferredRepresentation: preferredRepresentation)); + addSemanticBehavior(_labelAndValue = LabelAndValue(semanticsObject, this, preferredRepresentation: preferredRepresentation)); } /// Adds generic functionality for handling taps and clicks. void addTappable() { - addSecondaryRole(Tappable(semanticsObject, this)); + addSemanticBehavior(Tappable(semanticsObject, this)); } - /// Adds a secondary role to this primary role manager. + /// Adds a semantic behavior to this role. /// /// This method should be called by concrete implementations of - /// [PrimaryRoleManager] during initialization. + /// [SemanticRole] during initialization. @protected - void addSecondaryRole(RoleManager secondaryRoleManager) { + void addSemanticBehavior(SemanticBehavior behavior) { assert( - _secondaryRoleManagers?.any((RoleManager manager) => manager.role == secondaryRoleManager.role) != true, - 'Cannot add secondary role ${secondaryRoleManager.role}. This object already has this secondary role.', + _behaviors?.any((existing) => existing.runtimeType == behavior.runtimeType) != true, + 'Cannot add semantic behavior ${behavior.runtimeType}. This object already has it.', ); - _secondaryRoleManagers ??= []; - _secondaryRoleManagers!.add(secondaryRoleManager); + _behaviors ??= []; + _behaviors!.add(behavior); } /// Called immediately after the fields of the [semanticsObject] are updated @@ -605,16 +566,16 @@ abstract class PrimaryRoleManager { /// "is*Dirty" getters to find out exactly what's changed and apply the /// minimum DOM updates. /// - /// The base implementation requests every secondary role manager to update + /// The base implementation requests every semantics behavior to update /// the object. @mustCallSuper void update() { - final List? secondaryRoles = _secondaryRoleManagers; - if (secondaryRoles == null) { + final List? behaviors = _behaviors; + if (behaviors == null) { return; } - for (final RoleManager secondaryRole in secondaryRoles) { - secondaryRole.update(); + for (final SemanticBehavior behavior in behaviors) { + behavior.update(); } if (semanticsObject.isIdentifierDirty) { @@ -630,7 +591,7 @@ abstract class PrimaryRoleManager { } } - /// Whether this role manager was disposed of. + /// Whether this role was disposed of. bool get isDisposed => _isDisposed; bool _isDisposed = false; @@ -648,7 +609,7 @@ abstract class PrimaryRoleManager { } /// Transfers the accessibility focus to the [element] managed by this role - /// manager as a result of this node taking focus by default. + /// as a result of this node taking focus by default. /// /// For example, when a dialog pops up it is expected that one of its child /// nodes takes accessibility focus. @@ -658,16 +619,16 @@ abstract class PrimaryRoleManager { /// input focus. For example, a plain text node cannot take input focus, but /// it can take accessibility focus. /// - /// Returns `true` if the role manager took the focus. Returns `false` if - /// this role manager did not take the focus. The return value can be used to - /// decide whether to stop searching for a node that should take focus. + /// Returns `true` if the role took the focus. Returns `false` if this role + /// did not take the focus. The return value can be used to decide whether to + /// stop searching for a node that should take focus. bool focusAsRouteDefault(); } /// A role used when a more specific role couldn't be assigned to the node. -final class GenericRole extends PrimaryRoleManager { +final class GenericRole extends SemanticRole { GenericRole(SemanticsObject semanticsObject) : super.withBasics( - PrimaryRole.generic, + SemanticRoleKind.generic, semanticsObject, // Prefer sized span because if this is a leaf it is frequently a Text widget. // But if it turns out to be a container, then LabelAndValue will automatically @@ -749,26 +710,29 @@ final class GenericRole extends PrimaryRoleManager { /// Provides a piece of functionality to a [SemanticsObject]. /// -/// A secondary role must not set the `role` ARIA attribute. That responsibility -/// falls on the [PrimaryRoleManager]. One [SemanticsObject] may have more than -/// one [RoleManager] but an element may only have one ARIA role, so setting the -/// `role` attribute from a [RoleManager] would cause conflicts. +/// Semantic behaviors can be shared by multiple types of [SemanticRole]s. For +/// example, [SemanticButton] and [SemanticCheckable] both use the [Tappable] behavior. If a +/// semantic role needs bespoke functionality, it is simpler to implement it +/// directly in the [SemanticRole] implementation. /// -/// The [PrimaryRoleManager] decides the list of [RoleManager]s a given semantics -/// node should use. -abstract class RoleManager { - /// Initializes a secondary role for [semanticsObject]. +/// A behavior must not set the `role` ARIA attribute. That responsibility +/// falls on the [SemanticRole]. One [SemanticsObject] may have more than +/// one [SemanticBehavior] but an element may only have one ARIA role, so +/// setting the `role` attribute from a [SemanticBehavior] would cause +/// conflicts. +/// +/// The [SemanticRole] decides the list of [SemanticBehavior]s a given +/// semantics node should use. +abstract class SemanticBehavior { + /// Initializes a behavior for the [semanticsObject]. /// - /// A single role object manages exactly one [SemanticsObject]. - RoleManager(this.role, this.semanticsObject, this.owner); - - /// Role identifier. - final Role role; + /// A single [SemanticBehavior] object manages exactly one [SemanticsObject]. + SemanticBehavior(this.semanticsObject, this.owner); /// The semantics object managed by this role. final SemanticsObject semanticsObject; - final PrimaryRoleManager owner; + final SemanticRole owner; /// Called immediately after the [semanticsObject] updates some of its fields. /// @@ -777,7 +741,7 @@ abstract class RoleManager { /// minimum DOM updates. void update(); - /// Whether this role manager was disposed of. + /// Whether this behavior was disposed of. bool get isDisposed => _isDisposed; bool _isDisposed = false; @@ -1185,7 +1149,7 @@ class SemanticsObject { bool _isDirty(int fieldIndex) => (_dirtyFields & fieldIndex) != 0; /// The dom element of this semantics object. - DomElement get element => primaryRole!.element; + DomElement get element => semanticRole!.element; /// Returns the HTML element that contains the HTML elements of direct /// children of this object. @@ -1288,13 +1252,9 @@ class SemanticsObject { !isButton; /// Whether this node defines a scope for a route. - /// - /// See also [Role.dialog]. bool get scopesRoute => hasFlag(ui.SemanticsFlag.scopesRoute); /// Whether this node describes a route. - /// - /// See also [Role.dialog]. bool get namesRoute => hasFlag(ui.SemanticsFlag.namesRoute); /// Whether this object carry enabled/disabled state (and if so whether it is @@ -1471,7 +1431,7 @@ class SemanticsObject { } // Apply updates to the DOM. - _updateRoles(); + _updateRole(); // All properties that affect positioning and sizing are checked together // any one of them triggers position and size recomputation. @@ -1668,87 +1628,87 @@ class SemanticsObject { _currentChildrenInRenderOrder = childrenInRenderOrder; } - /// The primary role of this node. + /// The role of this node. /// - /// The primary role is assigned by [updateSelf] based on the combination of + /// The role is assigned by [updateSelf] based on the combination of /// semantics flags and actions. - PrimaryRoleManager? primaryRole; + SemanticRole? semanticRole; - PrimaryRole _getPrimaryRoleIdentifier() { + SemanticRoleKind _getSemanticRoleKind() { // The most specific role should take precedence. if (isPlatformView) { - return PrimaryRole.platformView; + return SemanticRoleKind.platformView; } else if (isHeading) { - return PrimaryRole.heading; + return SemanticRoleKind.heading; } else if (isTextField) { - return PrimaryRole.textField; + return SemanticRoleKind.textField; } else if (isIncrementable) { - return PrimaryRole.incrementable; + return SemanticRoleKind.incrementable; } else if (isVisualOnly) { - return PrimaryRole.image; + return SemanticRoleKind.image; } else if (isCheckable) { - return PrimaryRole.checkable; + return SemanticRoleKind.checkable; } else if (isButton) { - return PrimaryRole.button; + return SemanticRoleKind.button; } else if (isScrollContainer) { - return PrimaryRole.scrollable; + return SemanticRoleKind.scrollable; } else if (scopesRoute) { - return PrimaryRole.dialog; + return SemanticRoleKind.dialog; } else if (isLink) { - return PrimaryRole.link; + return SemanticRoleKind.link; } else { - return PrimaryRole.generic; + return SemanticRoleKind.generic; } } - PrimaryRoleManager _createPrimaryRole(PrimaryRole role) { + SemanticRole _createSemanticRole(SemanticRoleKind role) { return switch (role) { - PrimaryRole.textField => TextField(this), - PrimaryRole.scrollable => Scrollable(this), - PrimaryRole.incrementable => Incrementable(this), - PrimaryRole.button => Button(this), - PrimaryRole.checkable => Checkable(this), - PrimaryRole.dialog => Dialog(this), - PrimaryRole.image => ImageRoleManager(this), - PrimaryRole.platformView => PlatformViewRoleManager(this), - PrimaryRole.link => Link(this), - PrimaryRole.heading => Heading(this), - PrimaryRole.generic => GenericRole(this), + SemanticRoleKind.textField => SemanticTextField(this), + SemanticRoleKind.scrollable => SemanticScrollable(this), + SemanticRoleKind.incrementable => SemanticIncrementable(this), + SemanticRoleKind.button => SemanticButton(this), + SemanticRoleKind.checkable => SemanticCheckable(this), + SemanticRoleKind.dialog => SemanticDialog(this), + SemanticRoleKind.image => SemanticImage(this), + SemanticRoleKind.platformView => SemanticPlatformView(this), + SemanticRoleKind.link => SemanticLink(this), + SemanticRoleKind.heading => SemanticHeading(this), + SemanticRoleKind.generic => GenericRole(this), }; } - /// Detects the roles that this semantics object corresponds to and asks the - /// respective role managers to update the DOM. - void _updateRoles() { - PrimaryRoleManager? currentPrimaryRole = primaryRole; - final PrimaryRole roleId = _getPrimaryRoleIdentifier(); - final DomElement? previousElement = primaryRole?.element; + /// Detects the role that this semantics object corresponds to and asks it to + /// update the DOM. + void _updateRole() { + SemanticRole? currentSemanticRole = semanticRole; + final SemanticRoleKind kind = _getSemanticRoleKind(); + final DomElement? previousElement = semanticRole?.element; - if (currentPrimaryRole != null) { - if (currentPrimaryRole.role == roleId) { - // Already has a primary role assigned and the role is the same as before, + if (currentSemanticRole != null) { + if (currentSemanticRole.kind == kind) { + // Already has a role assigned and the role is the same as before, // so simply perform an update. - currentPrimaryRole.update(); + currentSemanticRole.update(); return; } else { // Role changed. This should be avoided as much as possible, but the // web engine will attempt a best with the switch by cleaning old ARIA // role data and start anew. - currentPrimaryRole.dispose(); - currentPrimaryRole = null; - primaryRole = null; + currentSemanticRole.dispose(); + currentSemanticRole = null; + semanticRole = null; } } // This handles two cases: - // * The node was just created and needs a primary role manager. - // * (Uncommon) the node changed its primary role, its previous primary - // role manager was disposed of, and now it needs a new one. - if (currentPrimaryRole == null) { - currentPrimaryRole = _createPrimaryRole(roleId); - primaryRole = currentPrimaryRole; - currentPrimaryRole.initState(); - currentPrimaryRole.update(); + // * The node was just created and needs a role. + // * (Uncommon) the node changed its role, its previous role was disposed + // of, and now it needs a new one. + if (currentSemanticRole == null) { + currentSemanticRole = _createSemanticRole(kind); + semanticRole = currentSemanticRole; + currentSemanticRole.initState(); + currentSemanticRole.update(); } // Reparent element. @@ -1786,7 +1746,7 @@ class SemanticsObject { /// Role-specific adjustment of the vertical position of the child container. /// - /// This is used, for example, by the [Scrollable] to compensate for the + /// This is used, for example, by the [SemanticScrollable] to compensate for the /// `scrollTop` offset in the DOM. /// /// This field must not be null. @@ -1795,7 +1755,7 @@ class SemanticsObject { /// Role-specific adjustment of the horizontal position of the child /// container. /// - /// This is used, for example, by the [Scrollable] to compensate for the + /// This is used, for example, by the [SemanticScrollable] to compensate for the /// `scrollLeft` offset in the DOM. /// /// This field must not be null. @@ -1970,8 +1930,8 @@ class SemanticsObject { _isDisposed = true; element.remove(); _parent = null; - primaryRole?.dispose(); - primaryRole = null; + semanticRole?.dispose(); + semanticRole = null; } } @@ -2013,7 +1973,7 @@ enum SemanticsUpdatePhase { idle, /// Updating individual [SemanticsObject] nodes by calling - /// [RoleManager.update] and fixing parent-child relationships. + /// [SemanticBehavior.update] and fixing parent-child relationships. /// /// After this phase is done, the owner enters the [postUpdate] phase. updating, @@ -2022,7 +1982,7 @@ enum SemanticsUpdatePhase { /// /// At this point all nodes have been updated, the parent child hierarchy has /// been established, the DOM tree is in sync with the semantics tree, and - /// [RoleManager.dispose] has been called on removed nodes. + /// [SemanticBehavior.dispose] has been called on removed nodes. /// /// After this phase is done, the owner switches back to [idle]. postUpdate, @@ -2626,7 +2586,7 @@ AFTER: $description /// Declares that a semantics node will explicitly request focus. /// - /// This prevents others, [Dialog] in particular, from requesting autofocus, + /// This prevents others, [SemanticDialog] in particular, from requesting autofocus, /// as focus can only be taken by one element. Explicit focus has higher /// precedence than autofocus. void willRequestFocus() { diff --git a/lib/web_ui/lib/src/engine/semantics/tappable.dart b/lib/web_ui/lib/src/engine/semantics/tappable.dart index d5f58d6a6cf94..1bd62cab88dd9 100644 --- a/lib/web_ui/lib/src/engine/semantics/tappable.dart +++ b/lib/web_ui/lib/src/engine/semantics/tappable.dart @@ -6,9 +6,9 @@ import 'package:ui/src/engine.dart'; import 'package:ui/ui.dart' as ui; /// Sets the "button" ARIA role. -class Button extends PrimaryRoleManager { - Button(SemanticsObject semanticsObject) : super.withBasics( - PrimaryRole.button, +class SemanticButton extends SemanticRole { + SemanticButton(SemanticsObject semanticsObject) : super.withBasics( + SemanticRoleKind.button, semanticsObject, preferredLabelRepresentation: LabelRepresentation.domText, ) { @@ -31,15 +31,18 @@ class Button extends PrimaryRoleManager { } } -/// Listens to HTML "click" gestures detected by the browser. +/// Implements clicking and tapping behavior for a semantics node. /// -/// This gestures is different from the click and tap gestures detected by the +/// Listens to HTML DOM "click" events detected by the browser. +/// +/// A DOM "click" is different from the click and tap gestures detected by the /// framework from raw pointer events. When an assistive technology is enabled /// the browser may not send us pointer events. In that mode we forward HTML /// click as [ui.SemanticsAction.tap]. -class Tappable extends RoleManager { - Tappable(SemanticsObject semanticsObject, PrimaryRoleManager owner) - : super(Role.tappable, semanticsObject, owner) { +/// +/// See also [ClickDebouncer]. +class Tappable extends SemanticBehavior { + Tappable(super.semanticsObject, super.owner) { _clickListener = createDomEventListener((DomEvent click) { PointerBinding.clickDebouncer.onClick( click, diff --git a/lib/web_ui/lib/src/engine/semantics/text_field.dart b/lib/web_ui/lib/src/engine/semantics/text_field.dart index 84f0518a652ae..29e88754aebf2 100644 --- a/lib/web_ui/lib/src/engine/semantics/text_field.dart +++ b/lib/web_ui/lib/src/engine/semantics/text_field.dart @@ -42,7 +42,7 @@ class SemanticsTextEditingStrategy extends DefaultTextEditingStrategy { /// The text field whose DOM element is currently used for editing. /// /// If this field is null, no editing takes place. - TextField? activeTextField; + SemanticTextField? activeTextField; /// Current input configuration supplied by the "flutter/textinput" channel. InputConfiguration? inputConfig; @@ -66,7 +66,7 @@ class SemanticsTextEditingStrategy extends DefaultTextEditingStrategy { /// /// This method must be called after [enable] to name sure that [inputConfig], /// [onChange], and [onAction] are not null. - void activate(TextField textField) { + void activate(SemanticTextField textField) { assert( inputConfig != null && onChange != null && onAction != null, '"enable" should be called before "enableFromSemantics" and initialize input configuration', @@ -91,7 +91,7 @@ class SemanticsTextEditingStrategy extends DefaultTextEditingStrategy { /// /// Typically at this point the element loses focus (blurs) and stops being /// used for editing. - void deactivate(TextField textField) { + void deactivate(SemanticTextField textField) { if (activeTextField == textField) { disable(); } @@ -167,7 +167,7 @@ class SemanticsTextEditingStrategy extends DefaultTextEditingStrategy { @override void initializeElementPlacement() { - // Element placement is done by [TextField]. + // Element placement is done by [SemanticTextField]. } @override @@ -176,7 +176,7 @@ class SemanticsTextEditingStrategy extends DefaultTextEditingStrategy { @override void updateElementPlacement(EditableTextGeometry textGeometry) { - // Element placement is done by [TextField]. + // Element placement is done by [SemanticTextField]. } EditableTextStyle? _queuedStyle; @@ -208,8 +208,8 @@ class SemanticsTextEditingStrategy extends DefaultTextEditingStrategy { /// browser gestures when in pointer mode. In Safari on iOS pointer events are /// used to detect text box invocation. This is because Safari issues touch /// events even when VoiceOver is enabled. -class TextField extends PrimaryRoleManager { - TextField(SemanticsObject semanticsObject) : super.blank(PrimaryRole.textField, semanticsObject) { +class SemanticTextField extends SemanticRole { + SemanticTextField(SemanticsObject semanticsObject) : super.blank(SemanticRoleKind.textField, semanticsObject) { _initializeEditableElement(); } diff --git a/lib/web_ui/test/engine/semantics/semantics_test.dart b/lib/web_ui/test/engine/semantics/semantics_test.dart index 93ca2bfbf4ace..5c5e25fda0269 100644 --- a/lib/web_ui/test/engine/semantics/semantics_test.dart +++ b/lib/web_ui/test/engine/semantics/semantics_test.dart @@ -48,11 +48,11 @@ void runSemanticsTests() { group('longestIncreasingSubsequence', () { _testLongestIncreasingSubsequence(); }); - group(PrimaryRoleManager, () { - _testPrimaryRoleManager(); + group(SemanticRole, () { + _testSemanticRole(); }); - group('Role managers', () { - _testRoleManagerLifecycle(); + group('Roles', () { + _testRoleLifecycle(); }); group('Text', () { _testText(); @@ -113,7 +113,7 @@ void runSemanticsTests() { }); } -void _testPrimaryRoleManager() { +void _testSemanticRole() { test('Sets id and flt-semantics-identifier on the element', () { semantics() ..debugOverrideTimestampFunction(() => _testTime) @@ -175,8 +175,8 @@ void _testPrimaryRoleManager() { }); } -void _testRoleManagerLifecycle() { - test('Secondary role managers are added upon node initialization', () { +void _testRoleLifecycle() { + test('Semantic behaviors are added upon node initialization', () { semantics() ..debugOverrideTimestampFunction(() => _testTime) ..semanticsEnabled = true; @@ -194,10 +194,10 @@ void _testRoleManagerLifecycle() { tester.expectSemantics(''); final SemanticsObject node = owner().debugSemanticsTree![0]!; - expect(node.primaryRole?.role, PrimaryRole.button); + expect(node.semanticRole?.kind, SemanticRoleKind.button); expect( - node.primaryRole?.debugSecondaryRoles, - containsAll([Role.focusable, Role.tappable, Role.labelAndValue]), + node.semanticRole?.debugSemanticBehaviorTypes, + containsAll([Focusable, Tappable, LabelAndValue]), ); expect(tester.getSemanticsObject(0).element.tabIndex, -1); } @@ -217,10 +217,10 @@ void _testRoleManagerLifecycle() { tester.expectSemantics('a label'); final SemanticsObject node = owner().debugSemanticsTree![0]!; - expect(node.primaryRole?.role, PrimaryRole.button); + expect(node.semanticRole?.kind, SemanticRoleKind.button); expect( - node.primaryRole?.debugSecondaryRoles, - containsAll([Role.focusable, Role.tappable, Role.labelAndValue]), + node.semanticRole?.debugSemanticBehaviorTypes, + containsAll([Focusable, Tappable, LabelAndValue]), ); expect(tester.getSemanticsObject(0).element.tabIndex, 0); } @@ -661,16 +661,16 @@ void _testEngineSemanticsOwner() { SemanticsUpdatePhase.idle, ); - // Rudely replace the role manager with a mock, and trigger an update. - final MockRoleManager mockRoleManager = MockRoleManager(PrimaryRole.generic, semanticsObject); - semanticsObject.primaryRole = mockRoleManager; + // Rudely replace the role with a mock, and trigger an update. + final MockRole mockRole = MockRole(SemanticRoleKind.generic, semanticsObject); + semanticsObject.semanticRole = mockRole; pumpSemantics(label: 'World'); expect( reason: 'While updating must be in SemanticsUpdatePhase.updating phase', - mockRoleManager.log, - [ + mockRole.log, + [ (method: 'update', phase: SemanticsUpdatePhase.updating), ], ); @@ -679,15 +679,15 @@ void _testEngineSemanticsOwner() { }); } -typedef MockRoleManagerLogEntry = ({ +typedef MockRoleLogEntry = ({ String method, SemanticsUpdatePhase phase, }); -class MockRoleManager extends PrimaryRoleManager { - MockRoleManager(super.role, super.semanticsObject) : super.blank(); +class MockRole extends SemanticRole { + MockRole(super.role, super.semanticsObject) : super.blank(); - final List log = []; + final List log = []; void _log(String method) { log.add(( @@ -882,9 +882,9 @@ void _testText() { ); final SemanticsObject node = owner().debugSemanticsTree![0]!; - expect(node.primaryRole?.role, PrimaryRole.generic); + expect(node.semanticRole?.kind, SemanticRoleKind.generic); expect( - node.primaryRole!.secondaryRoleManagers!.map((RoleManager m) => m.runtimeType).toList(), + node.semanticRole!.behaviors!.map((m) => m.runtimeType).toList(), [ Focusable, LiveRegion, @@ -915,9 +915,9 @@ void _testText() { ); final SemanticsObject node = owner().debugSemanticsTree![0]!; - expect(node.primaryRole?.role, PrimaryRole.generic); + expect(node.semanticRole?.kind, SemanticRoleKind.generic); expect( - node.primaryRole!.secondaryRoleManagers!.map((RoleManager m) => m.runtimeType).toList(), + node.semanticRole!.behaviors!.map((m) => m.runtimeType).toList(), [ Focusable, LiveRegion, @@ -1710,11 +1710,11 @@ void _testIncrementables() { '''); final SemanticsObject node = owner().debugSemanticsTree![0]!; - expect(node.primaryRole?.role, PrimaryRole.incrementable); + expect(node.semanticRole?.kind, SemanticRoleKind.incrementable); expect( reason: 'Incrementables use custom focus management', - node.primaryRole!.debugSecondaryRoles, - isNot(contains(Role.focusable)), + node.semanticRole!.debugSemanticBehaviorTypes, + isNot(contains(Focusable)), ); semantics().semanticsEnabled = false; @@ -1905,7 +1905,7 @@ void _testTextField() { final SemanticsObject node = owner().debugSemanticsTree![0]!; - final TextField textFieldRole = node.primaryRole! as TextField; + final SemanticTextField textFieldRole = node.semanticRole! as SemanticTextField; final DomHTMLInputElement inputElement = textFieldRole.editableElement as DomHTMLInputElement; // TODO(yjbanov): this used to attempt to test that value="hello" but the @@ -1914,11 +1914,11 @@ void _testTextField() { // https://github.com/flutter/flutter/issues/147200 expect(inputElement.value, ''); - expect(node.primaryRole?.role, PrimaryRole.textField); + expect(node.semanticRole?.kind, SemanticRoleKind.textField); expect( reason: 'Text fields use custom focus management', - node.primaryRole!.debugSecondaryRoles, - isNot(contains(Role.focusable)), + node.semanticRole!.debugSemanticBehaviorTypes, + isNot(contains(Focusable)), ); semantics().semanticsEnabled = false; @@ -1952,11 +1952,11 @@ void _testCheckables() { '''); final SemanticsObject node = owner().debugSemanticsTree![0]!; - expect(node.primaryRole?.role, PrimaryRole.checkable); + expect(node.semanticRole?.kind, SemanticRoleKind.checkable); expect( - reason: 'Checkables use generic secondary roles', - node.primaryRole!.debugSecondaryRoles, - containsAll([Role.focusable, Role.tappable]), + reason: 'Checkables use generic semantic behaviors', + node.semanticRole!.debugSemanticBehaviorTypes, + containsAll([Focusable, Tappable]), ); semantics().semanticsEnabled = false; @@ -2253,10 +2253,10 @@ void _testTappable() { '''); final SemanticsObject node = owner().debugSemanticsTree![0]!; - expect(node.primaryRole?.role, PrimaryRole.button); + expect(node.semanticRole?.kind, SemanticRoleKind.button); expect( - node.primaryRole?.debugSecondaryRoles, - containsAll([Role.focusable, Role.tappable]), + node.semanticRole?.debugSemanticBehaviorTypes, + containsAll([Focusable, Tappable]), ); expect(tester.getSemanticsObject(0).element.tabIndex, 0); @@ -2999,8 +2999,8 @@ void _testDialog() { '''); expect( - owner().debugSemanticsTree![0]!.primaryRole?.role, - PrimaryRole.dialog, + owner().debugSemanticsTree![0]!.semanticRole?.kind, + SemanticRoleKind.dialog, ); semantics().semanticsEnabled = false; @@ -3044,8 +3044,8 @@ void _testDialog() { '''); expect( - owner().debugSemanticsTree![0]!.primaryRole?.role, - PrimaryRole.dialog, + owner().debugSemanticsTree![0]!.semanticRole?.kind, + SemanticRoleKind.dialog, ); semantics().semanticsEnabled = false; @@ -3093,16 +3093,16 @@ void _testDialog() { pumpSemantics(label: 'Dialog label'); expect( - owner().debugSemanticsTree![0]!.primaryRole?.role, - PrimaryRole.dialog, + owner().debugSemanticsTree![0]!.semanticRole?.kind, + SemanticRoleKind.dialog, ); expect( - owner().debugSemanticsTree![2]!.primaryRole?.role, - PrimaryRole.generic, + owner().debugSemanticsTree![2]!.semanticRole?.kind, + SemanticRoleKind.generic, ); expect( - owner().debugSemanticsTree![2]!.primaryRole?.debugSecondaryRoles, - contains(Role.routeName), + owner().debugSemanticsTree![2]!.semanticRole?.debugSemanticBehaviorTypes, + contains(RouteName), ); pumpSemantics(label: 'Updated dialog label'); @@ -3131,12 +3131,12 @@ void _testDialog() { '''); expect( - owner().debugSemanticsTree![0]!.primaryRole?.role, - PrimaryRole.dialog, + owner().debugSemanticsTree![0]!.semanticRole?.kind, + SemanticRoleKind.dialog, ); expect( - owner().debugSemanticsTree![0]!.primaryRole?.secondaryRoleManagers, - isNot(contains(Role.routeName)), + owner().debugSemanticsTree![0]!.semanticRole?.behaviors, + isNot(contains(RouteName)), ); semantics().semanticsEnabled = false; @@ -3179,12 +3179,12 @@ void _testDialog() { '''); expect( - owner().debugSemanticsTree![0]!.primaryRole?.role, - PrimaryRole.generic, + owner().debugSemanticsTree![0]!.semanticRole?.kind, + SemanticRoleKind.generic, ); expect( - owner().debugSemanticsTree![2]!.primaryRole?.debugSecondaryRoles, - contains(Role.routeName), + owner().debugSemanticsTree![2]!.semanticRole?.debugSemanticBehaviorTypes, + contains(RouteName), ); semantics().semanticsEnabled = false; @@ -3550,12 +3550,12 @@ void _testFocusable() { final SemanticsObject node = owner().debugSemanticsTree![1]!; expect(node.isFocusable, isTrue); expect( - node.primaryRole?.role, - PrimaryRole.generic, + node.semanticRole?.kind, + SemanticRoleKind.generic, ); expect( - node.primaryRole?.debugSecondaryRoles, - contains(Role.focusable), + node.semanticRole?.debugSemanticBehaviorTypes, + contains(Focusable), ); final DomElement element = node.element; diff --git a/lib/web_ui/test/engine/semantics/semantics_tester.dart b/lib/web_ui/test/engine/semantics/semantics_tester.dart index 5621ff8c20063..d639b7f72b9d2 100644 --- a/lib/web_ui/test/engine/semantics/semantics_tester.dart +++ b/lib/web_ui/test/engine/semantics/semantics_tester.dart @@ -338,9 +338,9 @@ class SemanticsTester { return owner.debugSemanticsTree![id]!; } - /// Locates the [TextField] role manager of the semantics object with the give [id]. - TextField getTextField(int id) { - return getSemanticsObject(id).primaryRole! as TextField; + /// Locates the [SemanticTextField] role of the semantics object with the give [id]. + SemanticTextField getTextField(int id) { + return getSemanticsObject(id).semanticRole! as SemanticTextField; } void expectSemantics(String semanticsHtml) { @@ -401,3 +401,8 @@ class SemanticsActionLogger { Stream get actionLog => _actionLog; late Stream _actionLog; } + +extension SemanticRoleExtension on SemanticRole { + /// Types of semantics behaviors used by this role. + List get debugSemanticBehaviorTypes => behaviors?.map((behavior) => behavior.runtimeType).toList() ?? const []; +} diff --git a/lib/web_ui/test/engine/semantics/semantics_text_test.dart b/lib/web_ui/test/engine/semantics/semantics_text_test.dart index 3a4e1f495aba2..599d1f7dc66ca 100644 --- a/lib/web_ui/test/engine/semantics/semantics_text_test.dart +++ b/lib/web_ui/test/engine/semantics/semantics_text_test.dart @@ -49,11 +49,11 @@ Future testMain() async { ); final SemanticsObject node = owner().debugSemanticsTree![0]!; - expect(node.primaryRole?.role, PrimaryRole.generic); + expect(node.semanticRole?.kind, SemanticRoleKind.generic); expect( reason: 'A node with a label should get a LabelAndValue role', - node.primaryRole!.debugSecondaryRoles, - contains(Role.labelAndValue), + node.semanticRole!.debugSemanticBehaviorTypes, + contains(LabelAndValue), ); } @@ -213,7 +213,7 @@ Future testMain() async { final DomElement span = node.element.querySelector('span')!; expect(span.getAttribute('tabindex'), isNull); - node.primaryRole!.focusAsRouteDefault(); + node.semanticRole!.focusAsRouteDefault(); expect(span.getAttribute('tabindex'), '-1'); expect(domDocument.activeElement, span); @@ -237,7 +237,7 @@ Future testMain() async { final SemanticsObject node = owner().debugSemanticsTree![0]!; // Set DOM text as preferred representation - final LabelAndValue lav = node.primaryRole!.labelAndValue!; + final LabelAndValue lav = node.semanticRole!.labelAndValue!; lav.preferredRepresentation = LabelRepresentation.domText; lav.update(); @@ -246,7 +246,7 @@ Future testMain() async { ); expect(node.element.getAttribute('tabindex'), isNull); - node.primaryRole!.focusAsRouteDefault(); + node.semanticRole!.focusAsRouteDefault(); expect(node.element.getAttribute('tabindex'), '-1'); expect(domDocument.activeElement, node.element); @@ -270,7 +270,7 @@ Future testMain() async { final SemanticsObject node = owner().debugSemanticsTree![0]!; // Set DOM text as preferred representation - final LabelAndValue lav = node.primaryRole!.labelAndValue!; + final LabelAndValue lav = node.semanticRole!.labelAndValue!; lav.preferredRepresentation = LabelRepresentation.ariaLabel; lav.update(); @@ -279,7 +279,7 @@ Future testMain() async { ); expect(node.element.getAttribute('tabindex'), isNull); - node.primaryRole!.focusAsRouteDefault(); + node.semanticRole!.focusAsRouteDefault(); expect(node.element.getAttribute('tabindex'), '-1'); expect(domDocument.activeElement, node.element); diff --git a/lib/web_ui/test/engine/semantics/text_field_test.dart b/lib/web_ui/test/engine/semantics/text_field_test.dart index bd5f6f81d9fa2..fb1ad1991d1bf 100644 --- a/lib/web_ui/test/engine/semantics/text_field_test.dart +++ b/lib/web_ui/test/engine/semantics/text_field_test.dart @@ -60,7 +60,7 @@ void testMain() { value: 'hi', isFocused: true, ); - final TextField textField = textFieldSemantics.primaryRole! as TextField; + final SemanticTextField textField = textFieldSemantics.semanticRole! as SemanticTextField; // ensureInitialized() isn't called prior to calling dispose() here. // Since we are conditionally calling dispose() on our @@ -102,7 +102,7 @@ void testMain() { // make sure it tests the right things: // https://github.com/flutter/flutter/issues/147200 final SemanticsObject node = owner().debugSemanticsTree![0]!; - final TextField textFieldRole = node.primaryRole! as TextField; + final SemanticTextField textFieldRole = node.semanticRole! as SemanticTextField; final DomHTMLInputElement inputElement = textFieldRole.editableElement as DomHTMLInputElement; expect(inputElement.tagName.toLowerCase(), 'input'); @@ -114,7 +114,7 @@ void testMain() { createTextFieldSemantics(isEnabled: false, value: 'hello'); expectSemanticsTree(owner(), ''''''); final SemanticsObject node = owner().debugSemanticsTree![0]!; - final TextField textFieldRole = node.primaryRole! as TextField; + final SemanticTextField textFieldRole = node.semanticRole! as SemanticTextField; final DomHTMLInputElement inputElement = textFieldRole.editableElement as DomHTMLInputElement; expect(inputElement.tagName.toLowerCase(), 'input'); @@ -170,7 +170,7 @@ void testMain() { rect: const ui.Rect.fromLTWH(0, 0, 10, 15), ); - final TextField textField = textFieldSemantics.primaryRole! as TextField; + final SemanticTextField textField = textFieldSemantics.semanticRole! as SemanticTextField; expect(owner().semanticsHost.ownerDocument?.activeElement, strategy.domElement); expect(textField.editableElement, strategy.domElement); @@ -238,7 +238,7 @@ void testMain() { isFocused: true, rect: const ui.Rect.fromLTWH(0, 0, 10, 15)); - final TextField textField = textFieldSemantics.primaryRole! as TextField; + final SemanticTextField textField = textFieldSemantics.semanticRole! as SemanticTextField; final DomHTMLInputElement editableElement = textField.editableElement as DomHTMLInputElement; @@ -269,7 +269,7 @@ void testMain() { isFocused: true, rect: const ui.Rect.fromLTWH(0, 0, 10, 15)); - final TextField textField = textFieldSemantics.primaryRole! as TextField; + final SemanticTextField textField = textFieldSemantics.semanticRole! as SemanticTextField; final DomHTMLInputElement editableElement = textField.editableElement as DomHTMLInputElement; @@ -311,7 +311,7 @@ void testMain() { isFocused: true, ); - final TextField textField = textFieldSemantics.primaryRole! as TextField; + final SemanticTextField textField = textFieldSemantics.semanticRole! as SemanticTextField; expect(textField.editableElement, strategy.domElement); expect(owner().semanticsHost.ownerDocument?.activeElement, strategy.domElement); @@ -347,7 +347,7 @@ void testMain() { expect(strategy.domElement, isNull); // It doesn't remove the DOM element. - final TextField textField = textFieldSemantics.primaryRole! as TextField; + final SemanticTextField textField = textFieldSemantics.semanticRole! as SemanticTextField; expect(owner().semanticsHost.contains(textField.editableElement), isTrue); // Editing element is not enabled. expect(strategy.isEnabled, isFalse);