From 9f207a3fc6db6f121d2f247860f7f634e8438295 Mon Sep 17 00:00:00 2001 From: stuartmorgan Date: Mon, 23 Nov 2020 15:58:45 -0800 Subject: [PATCH] Add initial settings message to Windows embedding (#22323) Sends the flutter/settings update message to the engine after starting it. For now values other than 24-hour time preference are hard-coded, but dark mode support can be added later. Fixes https://github.com/flutter/flutter/issues/65590 --- .../test_utils/proc_table_replacement.h | 4 +- .../windows/flutter_windows_engine.cc | 23 +++++- .../platform/windows/flutter_windows_engine.h | 11 ++- .../flutter_windows_engine_unittests.cc | 73 ++++++++++++++++++- shell/platform/windows/system_utils.h | 6 ++ .../windows/system_utils_unittests.cc | 17 +++++ shell/platform/windows/system_utils_win32.cc | 16 ++++ testing/run_tests.py | 11 ++- 8 files changed, 153 insertions(+), 8 deletions(-) diff --git a/shell/platform/embedder/test_utils/proc_table_replacement.h b/shell/platform/embedder/test_utils/proc_table_replacement.h index f04299dcbed7f7..6981ee97b42ae4 100644 --- a/shell/platform/embedder/test_utils/proc_table_replacement.h +++ b/shell/platform/embedder/test_utils/proc_table_replacement.h @@ -8,7 +8,9 @@ // FlutterEngineProcTable entries (by using statics) to facilitate mocking in // tests of code built on top of the embedder API. // -// This should *ONLY* be used in unit tests as it is leaky by design. +// This should *ONLY* be used in unit tests as it is leaky by design. Because it +// uses statics for the lambdas, tests using this macro are generally not safe +// to run multiple times (e.g., using gtest_repeat). // // |proc| should be the name of an entry in FlutterEngineProcTable, such as // "initialize". |mock_impl| should be a lamba that replaces its implementation, diff --git a/shell/platform/windows/flutter_windows_engine.cc b/shell/platform/windows/flutter_windows_engine.cc index 2641e98c2e9794..6fdee48b1e9b47 100644 --- a/shell/platform/windows/flutter_windows_engine.cc +++ b/shell/platform/windows/flutter_windows_engine.cc @@ -8,10 +8,14 @@ #include #include +#include "flutter/shell/platform/common/cpp/client_wrapper/binary_messenger_impl.h" +#include "flutter/shell/platform/common/cpp/client_wrapper/include/flutter/basic_message_channel.h" +#include "flutter/shell/platform/common/cpp/json_message_codec.h" #include "flutter/shell/platform/common/cpp/path_utils.h" #include "flutter/shell/platform/windows/flutter_windows_view.h" #include "flutter/shell/platform/windows/string_conversion.h" #include "flutter/shell/platform/windows/system_utils.h" +#include "third_party/rapidjson/include/rapidjson/document.h" namespace flutter { @@ -114,10 +118,19 @@ FlutterWindowsEngine::FlutterWindowsEngine(const FlutterProjectBundle& project) plugin_registrar_ = std::make_unique(); plugin_registrar_->engine = this; + messenger_wrapper_ = std::make_unique(messenger_.get()); message_dispatcher_ = std::make_unique(messenger_.get()); window_proc_delegate_manager_ = std::make_unique(); + + // Set up internal channels. + // TODO: Replace this with an embedder.h API. See + // https://github.com/flutter/flutter/issues/71099 + settings_channel_ = + std::make_unique>( + messenger_wrapper_.get(), "flutter/settings", + &JsonMessageCodec::GetInstance()); } FlutterWindowsEngine::~FlutterWindowsEngine() { @@ -327,7 +340,15 @@ void FlutterWindowsEngine::SendSystemSettings() { embedder_api_.UpdateLocales(engine_, flutter_locale_list.data(), flutter_locale_list.size()); - // TODO: Send 'flutter/settings' channel settings here as well. + rapidjson::Document settings(rapidjson::kObjectType); + auto& allocator = settings.GetAllocator(); + settings.AddMember("alwaysUse24HourFormat", + Prefer24HourTime(GetUserTimeFormat()), allocator); + settings.AddMember("textScaleFactor", 1.0, allocator); + // TODO: Implement dark mode support. + // https://github.com/flutter/flutter/issues/54612 + settings.AddMember("platformBrightness", "light", allocator); + settings_channel_->Send(settings); } } // namespace flutter diff --git a/shell/platform/windows/flutter_windows_engine.h b/shell/platform/windows/flutter_windows_engine.h index 045cea0df4bc53..58a0a3a671211a 100644 --- a/shell/platform/windows/flutter_windows_engine.h +++ b/shell/platform/windows/flutter_windows_engine.h @@ -10,6 +10,8 @@ #include #include +#include "flutter/shell/platform/common/cpp/client_wrapper/binary_messenger_impl.h" +#include "flutter/shell/platform/common/cpp/client_wrapper/include/flutter/basic_message_channel.h" #include "flutter/shell/platform/common/cpp/incoming_message_dispatcher.h" #include "flutter/shell/platform/embedder/embedder.h" #include "flutter/shell/platform/windows/flutter_project_bundle.h" @@ -17,6 +19,7 @@ #include "flutter/shell/platform/windows/win32_task_runner.h" #include "flutter/shell/platform/windows/win32_window_proc_delegate_manager.h" #include "flutter/shell/platform/windows/window_state.h" +#include "third_party/rapidjson/include/rapidjson/document.h" namespace flutter { @@ -134,16 +137,22 @@ class FlutterWindowsEngine { // The plugin messenger handle given to API clients. std::unique_ptr messenger_; + // A wrapper around messenger_ for interacting with client_wrapper-level APIs. + std::unique_ptr messenger_wrapper_; + // Message dispatch manager for messages from engine_. std::unique_ptr message_dispatcher_; // The plugin registrar handle given to API clients. std::unique_ptr plugin_registrar_; + // The MethodChannel used for communication with the Flutter engine. + std::unique_ptr> settings_channel_; + // A callback to be called when the engine (and thus the plugin registrar) // is being destroyed. FlutterDesktopOnPluginRegistrarDestroyed - plugin_registrar_destruction_callback_; + plugin_registrar_destruction_callback_ = nullptr; // The manager for WindowProc delegate registration and callbacks. std::unique_ptr window_proc_delegate_manager_; diff --git a/shell/platform/windows/flutter_windows_engine_unittests.cc b/shell/platform/windows/flutter_windows_engine_unittests.cc index 18401ea11ee54f..5837e323302001 100644 --- a/shell/platform/windows/flutter_windows_engine_unittests.cc +++ b/shell/platform/windows/flutter_windows_engine_unittests.cc @@ -20,10 +20,81 @@ std::unique_ptr GetTestEngine() { properties.icu_data_path = L"C:\\foo\\icudtl.dat"; properties.aot_library_path = L"C:\\foo\\aot.so"; FlutterProjectBundle project(properties); - return std::make_unique(project); + auto engine = std::make_unique(project); + + EngineEmbedderApiModifier modifier(engine.get()); + // Force the non-AOT path unless overridden by the test. + modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; }; + + return engine; } } // namespace +TEST(FlutterWindowsEngine, RunDoesExpectedInitialization) { + std::unique_ptr engine = GetTestEngine(); + EngineEmbedderApiModifier modifier(engine.get()); + + // The engine should be run with expected configuration values. + bool run_called = false; + modifier.embedder_api().Run = MOCK_ENGINE_PROC( + Run, ([&run_called, engine_instance = engine.get()]( + size_t version, const FlutterRendererConfig* config, + const FlutterProjectArgs* args, void* user_data, + FLUTTER_API_SYMBOL(FlutterEngine) * engine_out) { + run_called = true; + *engine_out = reinterpret_cast(1); + + EXPECT_EQ(version, FLUTTER_ENGINE_VERSION); + EXPECT_NE(config, nullptr); + EXPECT_EQ(user_data, engine_instance); + // Spot-check arguments. + EXPECT_STREQ(args->assets_path, "C:\\foo\\flutter_assets"); + EXPECT_STREQ(args->icu_data_path, "C:\\foo\\icudtl.dat"); + EXPECT_EQ(args->dart_entrypoint_argc, 0); + EXPECT_NE(args->platform_message_callback, nullptr); + EXPECT_NE(args->custom_task_runners, nullptr); + EXPECT_EQ(args->custom_dart_entrypoint, nullptr); + + return kSuccess; + })); + + // It should send locale info. + bool update_locales_called = false; + modifier.embedder_api().UpdateLocales = MOCK_ENGINE_PROC( + UpdateLocales, + ([&update_locales_called](auto engine, const FlutterLocale** locales, + size_t locales_count) { + update_locales_called = true; + + EXPECT_GT(locales_count, 0); + EXPECT_NE(locales, nullptr); + + return kSuccess; + })); + + // And it should send initial settings info. + bool settings_message_sent = false; + modifier.embedder_api().SendPlatformMessage = MOCK_ENGINE_PROC( + SendPlatformMessage, + ([&settings_message_sent](auto engine, auto message) { + if (std::string(message->channel) == std::string("flutter/settings")) { + settings_message_sent = true; + } + + return kSuccess; + })); + + engine->RunWithEntrypoint(nullptr); + + EXPECT_TRUE(run_called); + EXPECT_TRUE(update_locales_called); + EXPECT_TRUE(settings_message_sent); + + // Ensure that deallocation doesn't call the actual Shutdown with the bogus + // engine pointer that the overridden Run returned. + modifier.embedder_api().Shutdown = [](auto engine) { return kSuccess; }; +} + TEST(FlutterWindowsEngine, SendPlatformMessageWithoutResponse) { std::unique_ptr engine = GetTestEngine(); EngineEmbedderApiModifier modifier(engine.get()); diff --git a/shell/platform/windows/system_utils.h b/shell/platform/windows/system_utils.h index 2585d4e8d5241f..60137d964bbc3b 100644 --- a/shell/platform/windows/system_utils.h +++ b/shell/platform/windows/system_utils.h @@ -31,6 +31,12 @@ std::vector GetPreferredLanguages(); // Parses a Windows language name into its components. LanguageInfo ParseLanguageName(std::wstring language_name); +// Returns the user's system time format string. +std::wstring GetUserTimeFormat(); + +// Returns true if the time_format is set to use 24 hour time. +bool Prefer24HourTime(std::wstring time_format); + } // namespace flutter #endif // FLUTTER_SHELL_PLATFORM_WINDOWS_SYSTEM_UTILS_H_ diff --git a/shell/platform/windows/system_utils_unittests.cc b/shell/platform/windows/system_utils_unittests.cc index 148ee8589007cc..6fecfbcc1134c8 100644 --- a/shell/platform/windows/system_utils_unittests.cc +++ b/shell/platform/windows/system_utils_unittests.cc @@ -71,5 +71,22 @@ TEST(SystemUtils, ParseLanguageNameWithThreeCharacterLanguage) { EXPECT_TRUE(info.script.empty()); } +TEST(SystemUtils, GetUserTimeFormat) { + // The value varies based on machine; just ensure that something is returned. + EXPECT_FALSE(GetUserTimeFormat().empty()); +} + +TEST(SystemUtils, Prefer24HourTimeHandlesEmptyFormat) { + EXPECT_FALSE(Prefer24HourTime(L"")); +} + +TEST(SystemUtils, Prefer24HourTimeHandles12Hour) { + EXPECT_FALSE(Prefer24HourTime(L"h:mm:ss tt")); +} + +TEST(SystemUtils, Prefer24HourTimeHandles24Hour) { + EXPECT_TRUE(Prefer24HourTime(L"HH:mm:ss")); +} + } // namespace testing } // namespace flutter diff --git a/shell/platform/windows/system_utils_win32.cc b/shell/platform/windows/system_utils_win32.cc index a198523bf536bf..53f9c215329933 100644 --- a/shell/platform/windows/system_utils_win32.cc +++ b/shell/platform/windows/system_utils_win32.cc @@ -90,4 +90,20 @@ LanguageInfo ParseLanguageName(std::wstring language_name) { return info; } +std::wstring GetUserTimeFormat() { + // Rather than do the call-allocate-call-free dance, just use a sufficiently + // large buffer to handle any reasonable time format string. + const int kBufferSize = 100; + wchar_t buffer[kBufferSize]; + if (::GetLocaleInfoEx(LOCALE_NAME_USER_DEFAULT, LOCALE_STIMEFORMAT, buffer, + kBufferSize) == 0) { + return std::wstring(); + } + return std::wstring(buffer, kBufferSize); +} + +bool Prefer24HourTime(std::wstring time_format) { + return time_format.find(L"H") != std::wstring::npos; +} + } // namespace flutter diff --git a/testing/run_tests.py b/testing/run_tests.py index 8bdd114f43988b..726b533798cdcc 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -97,8 +97,11 @@ def RunEngineExecutable(build_dir, executable_name, filter, flags=[], cwd=buildr def RunCCTests(build_dir, filter): print("Running Engine Unit-tests.") - shuffle_flags = [ + # Not all of the engine unit tests are designed to be run more than once. + non_repeatable_shuffle_flags = [ "--gtest_shuffle", + ] + shuffle_flags = non_repeatable_shuffle_flags + [ "--gtest_repeat=2", ] @@ -115,7 +118,7 @@ def RunCCTests(build_dir, filter): RunEngineExecutable(build_dir, 'embedder_unittests', filter, shuffle_flags) RunEngineExecutable(build_dir, 'embedder_proctable_unittests', filter, shuffle_flags) else: - RunEngineExecutable(build_dir, 'flutter_windows_unittests', filter, shuffle_flags) + RunEngineExecutable(build_dir, 'flutter_windows_unittests', filter, non_repeatable_shuffle_flags) RunEngineExecutable(build_dir, 'client_wrapper_windows_unittests', filter, shuffle_flags) @@ -150,14 +153,14 @@ def RunCCTests(build_dir, filter): # These unit-tests are Objective-C and can only run on Darwin. if IsMac(): RunEngineExecutable(build_dir, 'flutter_channels_unittests', filter, shuffle_flags) - RunEngineExecutable(build_dir, 'flutter_desktop_darwin_unittests', filter, shuffle_flags) + RunEngineExecutable(build_dir, 'flutter_desktop_darwin_unittests', filter, non_repeatable_shuffle_flags) # https://github.com/flutter/flutter/issues/36296 if IsLinux(): RunEngineExecutable(build_dir, 'txt_unittests', filter, shuffle_flags) if IsLinux(): - RunEngineExecutable(build_dir, 'flutter_linux_unittests', filter, shuffle_flags) + RunEngineExecutable(build_dir, 'flutter_linux_unittests', filter, non_repeatable_shuffle_flags) def RunEngineBenchmarks(build_dir, filter):