From efe7683311e5de575bb830426a402db0cc8439ff Mon Sep 17 00:00:00 2001 From: stuartmorgan Date: Wed, 16 Sep 2020 12:47:21 -0400 Subject: [PATCH] Add an explicit API for font change notification (#21164) Originally font change notification was handled by forwarding WM_FONTCHANGE to the Flutter HWND, to avoid adding new API surface, but that's not a good solution in a multi-window scenario, and it would require a completely different solution for UWP. It also requires non-obvious plumbing in the runner. This replaces that with an explicit API, so that there's a clean and obvious way for the runner to trigger this event. --- .../windows/client_wrapper/flutter_engine.cc | 4 ++++ .../client_wrapper/flutter_engine_unittests.cc | 18 ++++++++++++++++++ .../include/flutter/flutter_engine.h | 5 +++++ .../testing/stub_flutter_windows_api.cc | 6 ++++++ .../testing/stub_flutter_windows_api.h | 3 +++ shell/platform/windows/flutter_windows.cc | 4 ++++ shell/platform/windows/flutter_windows_view.cc | 7 ------- shell/platform/windows/flutter_windows_view.h | 3 --- .../platform/windows/public/flutter_windows.h | 3 +++ .../windows/testing/mock_win32_window.h | 1 - .../testing/win32_flutter_window_test.cc | 7 ------- .../testing/win32_flutter_window_test.h | 5 ----- shell/platform/windows/win32_flutter_window.cc | 4 ---- shell/platform/windows/win32_flutter_window.h | 3 --- .../windows/win32_flutter_window_unittests.cc | 8 -------- shell/platform/windows/win32_window.cc | 3 --- shell/platform/windows/win32_window.h | 3 --- .../windows/window_binding_handler_delegate.h | 4 ---- 18 files changed, 43 insertions(+), 48 deletions(-) diff --git a/shell/platform/windows/client_wrapper/flutter_engine.cc b/shell/platform/windows/client_wrapper/flutter_engine.cc index 8bf94420646..affadb42dc9 100644 --- a/shell/platform/windows/client_wrapper/flutter_engine.cc +++ b/shell/platform/windows/client_wrapper/flutter_engine.cc @@ -64,6 +64,10 @@ std::chrono::nanoseconds FlutterEngine::ProcessMessages() { return std::chrono::nanoseconds(FlutterDesktopEngineProcessMessages(engine_)); } +void FlutterEngine::ReloadSystemFonts() { + FlutterDesktopEngineReloadSystemFonts(engine_); +} + FlutterDesktopPluginRegistrarRef FlutterEngine::GetRegistrarForPlugin( const std::string& plugin_name) { if (!engine_) { diff --git a/shell/platform/windows/client_wrapper/flutter_engine_unittests.cc b/shell/platform/windows/client_wrapper/flutter_engine_unittests.cc index 8fe83225b11..8fc43b7402d 100644 --- a/shell/platform/windows/client_wrapper/flutter_engine_unittests.cc +++ b/shell/platform/windows/client_wrapper/flutter_engine_unittests.cc @@ -38,16 +38,22 @@ class TestFlutterWindowsApi : public testing::StubFlutterWindowsApi { // |flutter::testing::StubFlutterWindowsApi| uint64_t EngineProcessMessages() override { return 99; } + // |flutter::testing::StubFlutterWindowsApi| + void EngineReloadSystemFonts() override { reload_fonts_called_ = true; } + bool create_called() { return create_called_; } bool run_called() { return run_called_; } bool destroy_called() { return destroy_called_; } + bool reload_fonts_called() { return reload_fonts_called_; } + private: bool create_called_ = false; bool run_called_ = false; bool destroy_called_ = false; + bool reload_fonts_called_ = false; }; } // namespace @@ -93,6 +99,18 @@ TEST(FlutterEngineTest, ProcessMessages) { EXPECT_EQ(next_event_time.count(), 99); } +TEST(FlutterEngineTest, ReloadFonts) { + testing::ScopedStubFlutterWindowsApi scoped_api_stub( + std::make_unique()); + auto test_api = static_cast(scoped_api_stub.stub()); + + FlutterEngine engine(DartProject(L"fake/project/path")); + engine.Run(); + + engine.ReloadSystemFonts(); + EXPECT_TRUE(test_api->reload_fonts_called()); +} + TEST(FlutterEngineTest, GetMessenger) { DartProject project(L"data"); testing::ScopedStubFlutterWindowsApi scoped_api_stub( diff --git a/shell/platform/windows/client_wrapper/include/flutter/flutter_engine.h b/shell/platform/windows/client_wrapper/include/flutter/flutter_engine.h index 4e832e0ddcf..ee48643dacd 100644 --- a/shell/platform/windows/client_wrapper/include/flutter/flutter_engine.h +++ b/shell/platform/windows/client_wrapper/include/flutter/flutter_engine.h @@ -53,6 +53,11 @@ class FlutterEngine : public PluginRegistry { // last return value from this function. std::chrono::nanoseconds ProcessMessages(); + // Tells the engine that the system font list has changed. Should be called + // by clients when OS-level font changes happen (e.g., WM_FONTCHANGE in a + // Win32 application). + void ReloadSystemFonts(); + // flutter::PluginRegistry: FlutterDesktopPluginRegistrarRef GetRegistrarForPlugin( const std::string& plugin_name) override; diff --git a/shell/platform/windows/client_wrapper/testing/stub_flutter_windows_api.cc b/shell/platform/windows/client_wrapper/testing/stub_flutter_windows_api.cc index 20613734f90..afe35b8f889 100644 --- a/shell/platform/windows/client_wrapper/testing/stub_flutter_windows_api.cc +++ b/shell/platform/windows/client_wrapper/testing/stub_flutter_windows_api.cc @@ -108,6 +108,12 @@ uint64_t FlutterDesktopEngineProcessMessages(FlutterDesktopEngineRef engine) { return 0; } +void FlutterDesktopEngineReloadSystemFonts(FlutterDesktopEngineRef engine) { + if (s_stub_implementation) { + s_stub_implementation->EngineReloadSystemFonts(); + } +} + FlutterDesktopPluginRegistrarRef FlutterDesktopEngineGetPluginRegistrar( FlutterDesktopEngineRef engine, const char* plugin_name) { diff --git a/shell/platform/windows/client_wrapper/testing/stub_flutter_windows_api.h b/shell/platform/windows/client_wrapper/testing/stub_flutter_windows_api.h index 532abff1967..b795eab1a6d 100644 --- a/shell/platform/windows/client_wrapper/testing/stub_flutter_windows_api.h +++ b/shell/platform/windows/client_wrapper/testing/stub_flutter_windows_api.h @@ -62,6 +62,9 @@ class StubFlutterWindowsApi { // Called for FlutterDesktopEngineProcessMessages. virtual uint64_t EngineProcessMessages() { return 0; } + // Called for FlutterDesktopEngineReloadSystemFonts. + virtual void EngineReloadSystemFonts() {} + // Called for FlutterDesktopViewGetHWND. virtual HWND ViewGetHWND() { return reinterpret_cast(1); } diff --git a/shell/platform/windows/flutter_windows.cc b/shell/platform/windows/flutter_windows.cc index f15cd7ab91d..d5963a21226 100644 --- a/shell/platform/windows/flutter_windows.cc +++ b/shell/platform/windows/flutter_windows.cc @@ -136,6 +136,10 @@ uint64_t FlutterDesktopEngineProcessMessages(FlutterDesktopEngineRef engine) { return EngineFromHandle(engine)->task_runner()->ProcessTasks().count(); } +void FlutterDesktopEngineReloadSystemFonts(FlutterDesktopEngineRef engine) { + FlutterEngineReloadSystemFonts(EngineFromHandle(engine)->engine()); +} + FlutterDesktopPluginRegistrarRef FlutterDesktopEngineGetPluginRegistrar( FlutterDesktopEngineRef engine, const char* plugin_name) { diff --git a/shell/platform/windows/flutter_windows_view.cc b/shell/platform/windows/flutter_windows_view.cc index 5736ce988f5..415f074a8e9 100644 --- a/shell/platform/windows/flutter_windows_view.cc +++ b/shell/platform/windows/flutter_windows_view.cc @@ -105,13 +105,6 @@ void FlutterWindowsView::OnScroll(double x, SendScroll(x, y, delta_x, delta_y, scroll_offset_multiplier); } -void FlutterWindowsView::OnFontChange() { - if (engine_->engine() == nullptr) { - return; - } - FlutterEngineReloadSystemFonts(engine_->engine()); -} - // Sends new size information to FlutterEngine. void FlutterWindowsView::SendWindowMetrics(size_t width, size_t height, diff --git a/shell/platform/windows/flutter_windows_view.h b/shell/platform/windows/flutter_windows_view.h index 5df86e47629..f4e1186c6e0 100644 --- a/shell/platform/windows/flutter_windows_view.h +++ b/shell/platform/windows/flutter_windows_view.h @@ -98,9 +98,6 @@ class FlutterWindowsView : public WindowBindingHandlerDelegate { double delta_y, int scroll_offset_multiplier) override; - // |WindowBindingHandlerDelegate| - void OnFontChange() override; - private: // Struct holding the mouse state. The engine doesn't keep track of which // mouse buttons have been pressed, so it's the embedding's responsibility. diff --git a/shell/platform/windows/public/flutter_windows.h b/shell/platform/windows/public/flutter_windows.h index a185afb8490..6862a3a21c5 100644 --- a/shell/platform/windows/public/flutter_windows.h +++ b/shell/platform/windows/public/flutter_windows.h @@ -143,6 +143,9 @@ FLUTTER_EXPORT bool FlutterDesktopEngineRun(FlutterDesktopEngineRef engine, FLUTTER_EXPORT uint64_t FlutterDesktopEngineProcessMessages(FlutterDesktopEngineRef engine); +FLUTTER_EXPORT void FlutterDesktopEngineReloadSystemFonts( + FlutterDesktopEngineRef engine); + // Returns the plugin registrar handle for the plugin with the given name. // // The name must be unique across the application. diff --git a/shell/platform/windows/testing/mock_win32_window.h b/shell/platform/windows/testing/mock_win32_window.h index 2fcb5d344f9..224aaf90905 100644 --- a/shell/platform/windows/testing/mock_win32_window.h +++ b/shell/platform/windows/testing/mock_win32_window.h @@ -38,7 +38,6 @@ class MockWin32Window : public Win32Window { MOCK_METHOD1(OnText, void(const std::u16string&)); MOCK_METHOD4(OnKey, void(int, int, int, char32_t)); MOCK_METHOD2(OnScroll, void(double, double)); - MOCK_METHOD0(OnFontChange, void()); }; } // namespace testing diff --git a/shell/platform/windows/testing/win32_flutter_window_test.cc b/shell/platform/windows/testing/win32_flutter_window_test.cc index b514a96a57f..3afad699072 100644 --- a/shell/platform/windows/testing/win32_flutter_window_test.cc +++ b/shell/platform/windows/testing/win32_flutter_window_test.cc @@ -12,12 +12,5 @@ Win32FlutterWindowTest::Win32FlutterWindowTest(int width, int height) Win32FlutterWindowTest::~Win32FlutterWindowTest() = default; -void Win32FlutterWindowTest::OnFontChange() { - on_font_change_called_ = true; -} - -bool Win32FlutterWindowTest::OnFontChangeWasCalled() { - return on_font_change_called_; -} } // namespace testing } // namespace flutter diff --git a/shell/platform/windows/testing/win32_flutter_window_test.h b/shell/platform/windows/testing/win32_flutter_window_test.h index 6da56d9383a..c48bda6311a 100644 --- a/shell/platform/windows/testing/win32_flutter_window_test.h +++ b/shell/platform/windows/testing/win32_flutter_window_test.h @@ -19,11 +19,6 @@ class Win32FlutterWindowTest : public Win32FlutterWindow { Win32FlutterWindowTest(Win32FlutterWindowTest const&) = delete; Win32FlutterWindowTest& operator=(Win32FlutterWindowTest const&) = delete; - // |Win32Window| - void OnFontChange() override; - - bool OnFontChangeWasCalled(); - private: bool on_font_change_called_ = false; }; diff --git a/shell/platform/windows/win32_flutter_window.cc b/shell/platform/windows/win32_flutter_window.cc index c79e0a5364d..5f4c92aa986 100644 --- a/shell/platform/windows/win32_flutter_window.cc +++ b/shell/platform/windows/win32_flutter_window.cc @@ -171,8 +171,4 @@ void Win32FlutterWindow::OnScroll(double delta_x, double delta_y) { kScrollOffsetMultiplier); } -void Win32FlutterWindow::OnFontChange() { - binding_handler_delegate_->OnFontChange(); -} - } // namespace flutter diff --git a/shell/platform/windows/win32_flutter_window.h b/shell/platform/windows/win32_flutter_window.h index b97d9b38b01..6d26bcf6bff 100644 --- a/shell/platform/windows/win32_flutter_window.h +++ b/shell/platform/windows/win32_flutter_window.h @@ -59,9 +59,6 @@ class Win32FlutterWindow : public Win32Window, public WindowBindingHandler { // |Win32Window| void OnScroll(double delta_x, double delta_y) override; - // |Win32Window| - void OnFontChange() override; - // |FlutterWindowBindingHandler| void SetView(WindowBindingHandlerDelegate* view) override; diff --git a/shell/platform/windows/win32_flutter_window_unittests.cc b/shell/platform/windows/win32_flutter_window_unittests.cc index ff3a5d81f8c..e03470dcd34 100644 --- a/shell/platform/windows/win32_flutter_window_unittests.cc +++ b/shell/platform/windows/win32_flutter_window_unittests.cc @@ -13,13 +13,5 @@ TEST(Win32FlutterWindowTest, CreateDestroy) { ASSERT_TRUE(TRUE); } -TEST(Win32FlutterWindowTest, CanFontChange) { - Win32FlutterWindowTest window(800, 600); - HWND hwnd = window.GetWindowHandle(); - LRESULT result = SendMessage(hwnd, WM_FONTCHANGE, NULL, NULL); - ASSERT_EQ(result, 0); - ASSERT_TRUE(window.OnFontChangeWasCalled()); -} - } // namespace testing } // namespace flutter diff --git a/shell/platform/windows/win32_window.cc b/shell/platform/windows/win32_window.cc index d284954786d..7ed1f7c0ccd 100644 --- a/shell/platform/windows/win32_window.cc +++ b/shell/platform/windows/win32_window.cc @@ -130,9 +130,6 @@ Win32Window::HandleMessage(UINT const message, current_height_ = height; HandleResize(width, height); break; - case WM_FONTCHANGE: - OnFontChange(); - break; case WM_MOUSEMOVE: TrackMouseLeaveEvent(window_handle_); diff --git a/shell/platform/windows/win32_window.h b/shell/platform/windows/win32_window.h index 85c392d364b..453bc0e9048 100644 --- a/shell/platform/windows/win32_window.h +++ b/shell/platform/windows/win32_window.h @@ -97,9 +97,6 @@ class Win32Window { // Called when mouse scrollwheel input occurs. virtual void OnScroll(double delta_x, double delta_y) = 0; - // Called when the system font change. - virtual void OnFontChange() = 0; - UINT GetCurrentDPI(); UINT GetCurrentWidth(); diff --git a/shell/platform/windows/window_binding_handler_delegate.h b/shell/platform/windows/window_binding_handler_delegate.h index 3ff8f7e889d..30bc5b3111b 100644 --- a/shell/platform/windows/window_binding_handler_delegate.h +++ b/shell/platform/windows/window_binding_handler_delegate.h @@ -50,10 +50,6 @@ class WindowBindingHandlerDelegate { double delta_x, double delta_y, int scroll_offset_multiplier) = 0; - - // Notifies delegate that backing window size has had system font change. - // Typically called by currently configured WindowBindingHandler - virtual void OnFontChange() = 0; }; } // namespace flutter