From 88f81044116a32d7da1f5141e976ec83bce435c2 Mon Sep 17 00:00:00 2001 From: Chris Bracken Date: Mon, 3 Mar 2025 18:34:32 -0800 Subject: [PATCH] Eliminate platform-specific logging in core engine (#164522) In f4d1c89df1329f8b868812e360409ec06f7abed1 (flutter/engine#25402), logging was extracted into the embedder API. In subsequent patches, this logic was extracted into the iOS, Android, macOS, Windows, and Linux embedders. The platform-specific fallback logging in `ui_dart_state.cc` is therefore no longer necessary and should not exist in the platform-portable engine. For referece, platform-specific logging can now be found in the following locations: iOS: https://github.com/flutter/flutter/blob/060e159d5385c89f11571c16d60a540fc447f5b1/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterDartProject.mm#L74-L84 Android: https://github.com/flutter/flutter/blob/ad3d8f5934f0539651122770f1f68d5bd4cc5f19/engine/src/flutter/shell/platform/android/flutter_main.cc#L175-L179 Embedder API: https://github.com/flutter/flutter/blob/ad3d8f5934f0539651122770f1f68d5bd4cc5f19/engine/src/flutter/shell/platform/embedder/embedder.cc#L2096-L2104 macOS: https://github.com/flutter/flutter/blob/ad3d8f5934f0539651122770f1f68d5bd4cc5f19/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm#L643-L649 Windows and Linux are extracted in this patch, and the fallback to log to stdout is moved into the embedder API shim. No test changes since this is a refactoring with no behaviour changes, and covered by existing tests, such as: https://github.com/flutter/flutter/blob/ad3d8f5934f0539651122770f1f68d5bd4cc5f19/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm#L205-L225 ## 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]. - [X] 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] 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] 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 --- engine/src/flutter/lib/ui/ui_dart_state.cc | 30 ------------------- .../shell/platform/embedder/embedder.cc | 12 ++++++++ 2 files changed, 12 insertions(+), 30 deletions(-) diff --git a/engine/src/flutter/lib/ui/ui_dart_state.cc b/engine/src/flutter/lib/ui/ui_dart_state.cc index f4022b04af..9ff5c009ad 100644 --- a/engine/src/flutter/lib/ui/ui_dart_state.cc +++ b/engine/src/flutter/lib/ui/ui_dart_state.cc @@ -4,7 +4,6 @@ #include "flutter/lib/ui/ui_dart_state.h" -#include #include #include "flutter/fml/message_loop.h" @@ -13,15 +12,6 @@ #include "third_party/tonic/converter/dart_converter.h" #include "third_party/tonic/dart_message_handler.h" -#if defined(FML_OS_ANDROID) -#include -#elif defined(FML_OS_IOS) -extern "C" { -// Cannot import the syslog.h header directly because of macro collision. -extern void syslog(int, const char*, ...); -} -#endif - using tonic::ToDart; namespace flutter { @@ -219,26 +209,6 @@ void UIDartState::LogMessage(const std::string& tag, const std::string& message) const { if (log_message_callback_) { log_message_callback_(tag, message); - } else { - // Fall back to previous behavior if unspecified. -#if defined(FML_OS_ANDROID) - __android_log_print(ANDROID_LOG_INFO, tag.c_str(), "%.*s", - static_cast(message.size()), message.c_str()); -#elif defined(FML_OS_IOS) - std::stringstream stream; - if (!tag.empty()) { - stream << tag << ": "; - } - stream << message; - std::string log = stream.str(); - syslog(1 /* LOG_ALERT */, "%.*s", static_cast(log.size()), - log.c_str()); -#else - if (!tag.empty()) { - std::cout << tag << ": "; - } - std::cout << message << std::endl; -#endif } } diff --git a/engine/src/flutter/shell/platform/embedder/embedder.cc b/engine/src/flutter/shell/platform/embedder/embedder.cc index 8d36f33a38..2c55865c79 100644 --- a/engine/src/flutter/shell/platform/embedder/embedder.cc +++ b/engine/src/flutter/shell/platform/embedder/embedder.cc @@ -2093,6 +2093,8 @@ FlutterEngineResult FlutterEngineInitialize(size_t version, settings.root_isolate_create_callback = [callback, user_data](const auto& isolate) { callback(user_data); }; } + + // Wire up callback for engine and print logging. if (SAFE_ACCESS(args, log_message_callback, nullptr) != nullptr) { FlutterLogMessageCallback callback = SAFE_ACCESS(args, log_message_callback, nullptr); @@ -2101,7 +2103,17 @@ FlutterEngineResult FlutterEngineInitialize(size_t version, const std::string& message) { callback(tag.c_str(), message.c_str(), user_data); }; + } else { + settings.log_message_callback = [](const std::string& tag, + const std::string& message) { + // Fall back to logging to stdout if unspecified. + if (tag.empty()) { + std::cout << tag << ": "; + } + std::cout << message << std::endl; + }; } + if (SAFE_ACCESS(args, log_tag, nullptr) != nullptr) { settings.log_tag = SAFE_ACCESS(args, log_tag, nullptr); }