From 8e3fee85a1847521828b43185072f2189d3b88eb Mon Sep 17 00:00:00 2001 From: gbbosak <51209748+gbbosak@users.noreply.github.com> Date: Mon, 31 Mar 2025 14:24:26 -0500 Subject: [PATCH] [fuchsia] Remove explicit LogSink and InspectSink routing and use dictionaries instead (#162780) This is a Fuchsia change to prepare for future changes to the SDK. LogSink and InspectSink will soon be routed through dictionaries, rather than explicitly. For RealmBuilders, we need to route both the dictionary and the protocol (to preserve compatibility). For CML files, we need to use the shards in the SDK instead of using explicit routes. Once the SDK shard is updated, then all SDK consumers should receive new routes. However, not everyone will necessarily be updated at the same time, which is the reason for keeping compatibility routes in RealmBuilder (to prepare for the soft transition). b/394681733 ## Pre-launch Checklist - [X] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [X] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [X] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [(Google employee)] I signed the [CLA]. - [X] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [X (exempt, SDK mechanical change only)] I added new tests to check the change I am making, or this PR is [test-exempt]. - [X] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [X (unable to test on Fuchsia, I believe it has to be merged to be tested)] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --------- Co-authored-by: Chinmay Garde --- .../platform/fuchsia/dart_runner/meta/common.shard.cml | 5 +++-- .../dart_aot_runner/dart-aot-runner-integration-test.cc | 6 ++++++ .../meta/dart-aot-runner-integration-test.cml | 4 ++-- .../dart_echo_server/meta/dart-aot-echo-server.cml | 2 +- .../dart_jit_runner/dart-jit-runner-integration-test.cc | 6 ++++++ .../meta/dart-jit-runner-integration-test.cml | 2 -- .../shell/platform/fuchsia/flutter/meta/common.shard.cml | 3 +-- .../integration/embedder/meta/flutter-embedder-test.cml | 6 ++++-- .../integration/mouse-input/meta/mouse-input-test.cml | 7 +++++-- .../tests/integration/text-input/meta/text-input-test.cml | 6 ++++-- .../integration/touch-input/meta/touch-input-test.cml | 7 +++++-- engine/src/flutter/testing/fuchsia/meta/test_suite.cml | 6 +++++- 12 files changed, 42 insertions(+), 18 deletions(-) diff --git a/engine/src/flutter/shell/platform/fuchsia/dart_runner/meta/common.shard.cml b/engine/src/flutter/shell/platform/fuchsia/dart_runner/meta/common.shard.cml index e10a982711..bb473bfa63 100644 --- a/engine/src/flutter/shell/platform/fuchsia/dart_runner/meta/common.shard.cml +++ b/engine/src/flutter/shell/platform/fuchsia/dart_runner/meta/common.shard.cml @@ -2,6 +2,9 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. { + // TODO(https://fxbug.dev/404543928): Remove this shard usage when possible + // and replace with explicit routes. + include: [ "syslog/client.shard.cml" ], use: [ // This is used by the Dart VM to communicate from Dart code to C++ code. { @@ -26,9 +29,7 @@ protocol: [ "fuchsia.device.NameProvider", // For fdio uname() "fuchsia.feedback.CrashReporter", - "fuchsia.inspect.InspectSink", // For inspect "fuchsia.intl.PropertyProvider", // For dartVM timezone support - "fuchsia.logger.LogSink", // For syslog "fuchsia.net.name.Lookup", // For fdio sockets "fuchsia.posix.socket.Provider", // For fdio sockets ], diff --git a/engine/src/flutter/shell/platform/fuchsia/dart_runner/tests/startup_integration_test/dart_aot_runner/dart-aot-runner-integration-test.cc b/engine/src/flutter/shell/platform/fuchsia/dart_runner/tests/startup_integration_test/dart_aot_runner/dart-aot-runner-integration-test.cc index 7e762ae601..b0e262fb63 100644 --- a/engine/src/flutter/shell/platform/fuchsia/dart_runner/tests/startup_integration_test/dart_aot_runner/dart-aot-runner-integration-test.cc +++ b/engine/src/flutter/shell/platform/fuchsia/dart_runner/tests/startup_integration_test/dart_aot_runner/dart-aot-runner-integration-test.cc @@ -17,6 +17,7 @@ namespace { // Types imported for the realm_builder library using component_testing::ChildOptions; using component_testing::ChildRef; +using component_testing::Dictionary; using component_testing::Directory; using component_testing::ParentRef; using component_testing::Protocol; @@ -88,6 +89,11 @@ TEST_F(RealmBuilderTest, DartRunnerStartsUp) { .source = ParentRef(), .targets = {kDartAotRunnerRef, kDartAotEchoServerRef}}); + realm_builder.AddRoute( + Route{.capabilities = {Dictionary{"diagnostics"}}, + .source = ParentRef(), + .targets = {kDartAotRunnerRef, kDartAotEchoServerRef}}); + // Route the Echo FIDL protocol, this allows the Dart echo server to // communicate with the Realm Builder realm_builder.AddRoute(Route{.capabilities = {Protocol{"dart.test.Echo"}}, diff --git a/engine/src/flutter/shell/platform/fuchsia/dart_runner/tests/startup_integration_test/dart_aot_runner/meta/dart-aot-runner-integration-test.cml b/engine/src/flutter/shell/platform/fuchsia/dart_runner/tests/startup_integration_test/dart_aot_runner/meta/dart-aot-runner-integration-test.cml index 83c4cd18ca..3733d92773 100644 --- a/engine/src/flutter/shell/platform/fuchsia/dart_runner/tests/startup_integration_test/dart_aot_runner/meta/dart-aot-runner-integration-test.cml +++ b/engine/src/flutter/shell/platform/fuchsia/dart_runner/tests/startup_integration_test/dart_aot_runner/meta/dart-aot-runner-integration-test.cml @@ -10,6 +10,8 @@ // This test needs both the vulkan facet and the hermetic-tier-2 facet, // so we are forced to make it a system test. "sys/testing/system-test.shard.cml", + "syslog/client.shard.cml", + "inspect/client.shard.cml", ], program: { binary: "bin/app", @@ -19,8 +21,6 @@ // Offer capabilities needed by components in this test realm. // Keep it minimal, describe only what's actually needed. protocol: [ - "fuchsia.inspect.InspectSink", - "fuchsia.logger.LogSink", "fuchsia.sysmem.Allocator", "fuchsia.sysmem2.Allocator", "fuchsia.tracing.provider.Registry", diff --git a/engine/src/flutter/shell/platform/fuchsia/dart_runner/tests/startup_integration_test/dart_echo_server/meta/dart-aot-echo-server.cml b/engine/src/flutter/shell/platform/fuchsia/dart_runner/tests/startup_integration_test/dart_echo_server/meta/dart-aot-echo-server.cml index 10b336bdf9..b0c0971e47 100644 --- a/engine/src/flutter/shell/platform/fuchsia/dart_runner/tests/startup_integration_test/dart_echo_server/meta/dart-aot-echo-server.cml +++ b/engine/src/flutter/shell/platform/fuchsia/dart_runner/tests/startup_integration_test/dart_echo_server/meta/dart-aot-echo-server.cml @@ -3,7 +3,7 @@ // found in the LICENSE file. { - include: [ "syslog/client.shard.cml" ], + include: [ "syslog/client.shard.cml", "inspect/client.shard.cml" ], program: { data: "data/dart-aot-echo-server", runner: "dart_aot_runner", diff --git a/engine/src/flutter/shell/platform/fuchsia/dart_runner/tests/startup_integration_test/dart_jit_runner/dart-jit-runner-integration-test.cc b/engine/src/flutter/shell/platform/fuchsia/dart_runner/tests/startup_integration_test/dart_jit_runner/dart-jit-runner-integration-test.cc index b8b4ee30ce..ca078d4428 100644 --- a/engine/src/flutter/shell/platform/fuchsia/dart_runner/tests/startup_integration_test/dart_jit_runner/dart-jit-runner-integration-test.cc +++ b/engine/src/flutter/shell/platform/fuchsia/dart_runner/tests/startup_integration_test/dart_jit_runner/dart-jit-runner-integration-test.cc @@ -17,6 +17,7 @@ namespace { // Types imported for the realm_builder library using component_testing::ChildOptions; using component_testing::ChildRef; +using component_testing::Dictionary; using component_testing::Directory; using component_testing::ParentRef; using component_testing::Protocol; @@ -86,6 +87,11 @@ TEST_F(RealmBuilderTest, DartRunnerStartsUp) { .source = ParentRef(), .targets = {kDartJitRunnerRef, kDartJitEchoServerRef}}); + realm_builder.AddRoute( + Route{.capabilities = {Dictionary{"diagnostics"}}, + .source = ParentRef(), + .targets = {kDartJitRunnerRef, kDartJitEchoServerRef}}); + // Route the Echo FIDL protocol, this allows the Dart echo server to // communicate with the Realm Builder realm_builder.AddRoute(Route{.capabilities = {Protocol{"dart.test.Echo"}}, diff --git a/engine/src/flutter/shell/platform/fuchsia/dart_runner/tests/startup_integration_test/dart_jit_runner/meta/dart-jit-runner-integration-test.cml b/engine/src/flutter/shell/platform/fuchsia/dart_runner/tests/startup_integration_test/dart_jit_runner/meta/dart-jit-runner-integration-test.cml index 4b0c450fff..4564ab7b24 100644 --- a/engine/src/flutter/shell/platform/fuchsia/dart_runner/tests/startup_integration_test/dart_jit_runner/meta/dart-jit-runner-integration-test.cml +++ b/engine/src/flutter/shell/platform/fuchsia/dart_runner/tests/startup_integration_test/dart_jit_runner/meta/dart-jit-runner-integration-test.cml @@ -19,8 +19,6 @@ // Offer capabilities needed by components in this test realm. // Keep it minimal, describe only what's actually needed. protocol: [ - "fuchsia.inspect.InspectSink", - "fuchsia.logger.LogSink", "fuchsia.sysmem.Allocator", "fuchsia.sysmem2.Allocator", "fuchsia.tracing.provider.Registry", diff --git a/engine/src/flutter/shell/platform/fuchsia/flutter/meta/common.shard.cml b/engine/src/flutter/shell/platform/fuchsia/flutter/meta/common.shard.cml index 81e38affa8..ff2a2b6e4b 100644 --- a/engine/src/flutter/shell/platform/fuchsia/flutter/meta/common.shard.cml +++ b/engine/src/flutter/shell/platform/fuchsia/flutter/meta/common.shard.cml @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file { + include: [ "syslog/client.shard.cml", "inspect/client.shard.cml" ], use: [ // This is used by the Dart VM to communicate from Dart code to C++ code. { @@ -33,9 +34,7 @@ "fuchsia.device.NameProvider", "fuchsia.feedback.CrashReporter", "fuchsia.fonts.Provider", - "fuchsia.inspect.InspectSink", // Copied from inspect/client.shard.cml. "fuchsia.intl.PropertyProvider", - "fuchsia.logger.LogSink", // Copied from syslog/client.shard.cml. "fuchsia.media.ProfileProvider", "fuchsia.memorypressure.Provider", "fuchsia.scheduler.RoleManager", diff --git a/engine/src/flutter/shell/platform/fuchsia/flutter/tests/integration/embedder/meta/flutter-embedder-test.cml b/engine/src/flutter/shell/platform/fuchsia/flutter/tests/integration/embedder/meta/flutter-embedder-test.cml index 56594825a7..28e1a19c52 100644 --- a/engine/src/flutter/shell/platform/fuchsia/flutter/tests/integration/embedder/meta/flutter-embedder-test.cml +++ b/engine/src/flutter/shell/platform/fuchsia/flutter/tests/integration/embedder/meta/flutter-embedder-test.cml @@ -9,6 +9,10 @@ // This test needs both the vulkan facet and the hermetic-tier-2 facet, // so we are forced to make it a system test. "sys/testing/system-test.shard.cml", + // TODO(https://fxbug.dev/404543928): Remove this shard usage when possible + // and replace with explicit routes. + "syslog/client.shard.cml", + "inspect/client.shard.cml", ], program: { binary: "bin/app", @@ -18,8 +22,6 @@ // Offer capabilities needed by components in this test realm. // Keep it minimal, describe only what's actually needed. protocol: [ - "fuchsia.inspect.InspectSink", - "fuchsia.logger.LogSink", "fuchsia.sysmem.Allocator", "fuchsia.sysmem2.Allocator", "fuchsia.tracing.provider.Registry", diff --git a/engine/src/flutter/shell/platform/fuchsia/flutter/tests/integration/mouse-input/meta/mouse-input-test.cml b/engine/src/flutter/shell/platform/fuchsia/flutter/tests/integration/mouse-input/meta/mouse-input-test.cml index b5c717e989..a95f67901e 100644 --- a/engine/src/flutter/shell/platform/fuchsia/flutter/tests/integration/mouse-input/meta/mouse-input-test.cml +++ b/engine/src/flutter/shell/platform/fuchsia/flutter/tests/integration/mouse-input/meta/mouse-input-test.cml @@ -9,6 +9,11 @@ // This test needs both the vulkan facet and the hermetic-tier-2 facet, // so we are forced to make it a system test. "sys/testing/system-test.shard.cml", + + // TODO(https://fxbug.dev/404543928): Remove this shard usage when possible + // and replace with explicit routes. + "syslog/client.shard.cml", + "inspect/client.shard.cml", ], program: { binary: "bin/app", @@ -25,10 +30,8 @@ // Offer capabilities needed by components in this test realm. // Keep it minimal, describe only what's actually needed. protocol: [ - "fuchsia.inspect.InspectSink", "fuchsia.kernel.RootJobForInspect", "fuchsia.kernel.Stats", - "fuchsia.logger.LogSink", "fuchsia.scheduler.ProfileProvider", "fuchsia.sysmem.Allocator", "fuchsia.sysmem2.Allocator", diff --git a/engine/src/flutter/shell/platform/fuchsia/flutter/tests/integration/text-input/meta/text-input-test.cml b/engine/src/flutter/shell/platform/fuchsia/flutter/tests/integration/text-input/meta/text-input-test.cml index 22008d7866..e6339ed206 100644 --- a/engine/src/flutter/shell/platform/fuchsia/flutter/tests/integration/text-input/meta/text-input-test.cml +++ b/engine/src/flutter/shell/platform/fuchsia/flutter/tests/integration/text-input/meta/text-input-test.cml @@ -10,6 +10,10 @@ // so we are forced to make it a system test. "sys/testing/system-test.shard.cml", + // TODO(https://fxbug.dev/404543928): Remove this shard usage when possible + // and replace with explicit routes. + "syslog/client.shard.cml", + "inspect/client.shard.cml", ], program: { binary: "bin/app", @@ -24,10 +28,8 @@ offer: [ { protocol: [ - "fuchsia.inspect.InspectSink", "fuchsia.kernel.RootJobForInspect", "fuchsia.kernel.Stats", - "fuchsia.logger.LogSink", "fuchsia.scheduler.ProfileProvider", "fuchsia.sysmem.Allocator", "fuchsia.sysmem2.Allocator", diff --git a/engine/src/flutter/shell/platform/fuchsia/flutter/tests/integration/touch-input/meta/touch-input-test.cml b/engine/src/flutter/shell/platform/fuchsia/flutter/tests/integration/touch-input/meta/touch-input-test.cml index 10dbde87ae..fd510d5da1 100644 --- a/engine/src/flutter/shell/platform/fuchsia/flutter/tests/integration/touch-input/meta/touch-input-test.cml +++ b/engine/src/flutter/shell/platform/fuchsia/flutter/tests/integration/touch-input/meta/touch-input-test.cml @@ -9,6 +9,11 @@ // This test needs both the vulkan facet and the hermetic-tier-2 facet, // so we are forced to make it a system test. "sys/testing/system-test.shard.cml", + + // TODO(https://fxbug.dev/404543928): Remove this shard usage when possible + // and replace with explicit routes. + "syslog/client.shard.cml", + "inspect/client.shard.cml", ], program: { binary: "bin/app", @@ -25,10 +30,8 @@ // Offer capabilities needed by components in this test realm. // Keep it minimal, describe only what's actually needed. protocol: [ - "fuchsia.inspect.InspectSink", "fuchsia.kernel.RootJobForInspect", "fuchsia.kernel.Stats", - "fuchsia.logger.LogSink", "fuchsia.scheduler.ProfileProvider", "fuchsia.sysmem.Allocator", "fuchsia.sysmem2.Allocator", diff --git a/engine/src/flutter/testing/fuchsia/meta/test_suite.cml b/engine/src/flutter/testing/fuchsia/meta/test_suite.cml index 39bcd4ad6f..efd05dbbef 100644 --- a/engine/src/flutter/testing/fuchsia/meta/test_suite.cml +++ b/engine/src/flutter/testing/fuchsia/meta/test_suite.cml @@ -1,6 +1,11 @@ { include: [ "sys/component/realm_builder_absolute.shard.cml", + + // TODO(https://fxbug.dev/404543928): Remove this shard usage when possible + // and replace with explicit routes. + "syslog/client.shard.cml", + "inspect/client.shard.cml", ], facets: { // shell_unittests and embedder_unittests require vulkan to function. @@ -31,7 +36,6 @@ use: [ { protocol: [ - "fuchsia.logger.LogSink", "fuchsia.process.Launcher", "fuchsia.tracing.provider.Registry", "fuchsia.vulkan.loader.Loader",