From 51e4e32986e100cd0c67d8938e4bdaebdc17d796 Mon Sep 17 00:00:00 2001 From: Loic Sharma Date: Mon, 21 Nov 2022 14:21:22 -0800 Subject: [PATCH] [Windows] Make the engine own PlatformHandler --- .../windows/flutter_windows_engine.cc | 2 + .../platform/windows/flutter_windows_engine.h | 4 + .../platform/windows/flutter_windows_view.cc | 2 - shell/platform/windows/flutter_windows_view.h | 4 - shell/platform/windows/platform_handler.cc | 31 ++- shell/platform/windows/platform_handler.h | 8 +- .../windows/platform_handler_unittests.cc | 193 ++++++++++++------ 7 files changed, 163 insertions(+), 81 deletions(-) diff --git a/shell/platform/windows/flutter_windows_engine.cc b/shell/platform/windows/flutter_windows_engine.cc index 1cd2fc408c330..795d83efea6fd 100644 --- a/shell/platform/windows/flutter_windows_engine.cc +++ b/shell/platform/windows/flutter_windows_engine.cc @@ -207,6 +207,8 @@ FlutterWindowsEngine::FlutterWindowsEngine( // Set up internal channels. // TODO: Replace this with an embedder.h API. See // https://github.com/flutter/flutter/issues/71099 + platform_handler_ = + std::make_unique(messenger_wrapper_.get(), this); settings_plugin_ = std::make_unique(messenger_wrapper_.get(), task_runner_.get()); } diff --git a/shell/platform/windows/flutter_windows_engine.h b/shell/platform/windows/flutter_windows_engine.h index 3b6fdf15e9185..d7f08036c552c 100644 --- a/shell/platform/windows/flutter_windows_engine.h +++ b/shell/platform/windows/flutter_windows_engine.h @@ -23,6 +23,7 @@ #include "flutter/shell/platform/windows/flutter_desktop_messenger.h" #include "flutter/shell/platform/windows/flutter_project_bundle.h" #include "flutter/shell/platform/windows/flutter_windows_texture_registrar.h" +#include "flutter/shell/platform/windows/platform_handler.h" #include "flutter/shell/platform/windows/public/flutter_windows.h" #include "flutter/shell/platform/windows/settings_plugin.h" #include "flutter/shell/platform/windows/task_runner.h" @@ -306,6 +307,9 @@ class FlutterWindowsEngine { // May be nullptr if ANGLE failed to initialize. std::unique_ptr surface_manager_; + // Handler for the flutter/platform channel. + std::unique_ptr platform_handler_; + // The settings plugin. std::unique_ptr settings_plugin_; diff --git a/shell/platform/windows/flutter_windows_view.cc b/shell/platform/windows/flutter_windows_view.cc index 0f04f57bf7ee6..a7b9a675d6b58 100644 --- a/shell/platform/windows/flutter_windows_view.cc +++ b/shell/platform/windows/flutter_windows_view.cc @@ -63,8 +63,6 @@ void FlutterWindowsView::SetEngine( // Set up the system channel handlers. auto internal_plugin_messenger = internal_plugin_registrar_->messenger(); InitializeKeyboard(); - platform_handler_ = - std::make_unique(internal_plugin_messenger, this); cursor_handler_ = std::make_unique(internal_plugin_messenger, binding_handler_.get()); diff --git a/shell/platform/windows/flutter_windows_view.h b/shell/platform/windows/flutter_windows_view.h index f2b38ed22e724..1c7d721e2f5eb 100644 --- a/shell/platform/windows/flutter_windows_view.h +++ b/shell/platform/windows/flutter_windows_view.h @@ -22,7 +22,6 @@ #include "flutter/shell/platform/windows/flutter_windows_engine.h" #include "flutter/shell/platform/windows/keyboard_handler_base.h" #include "flutter/shell/platform/windows/keyboard_key_embedder_handler.h" -#include "flutter/shell/platform/windows/platform_handler.h" #include "flutter/shell/platform/windows/public/flutter_windows.h" #include "flutter/shell/platform/windows/text_input_plugin.h" #include "flutter/shell/platform/windows/text_input_plugin_delegate.h" @@ -380,9 +379,6 @@ class FlutterWindowsView : public WindowBindingHandlerDelegate, // Handlers for text events from Windows. std::unique_ptr text_input_plugin_; - // Handler for the flutter/platform channel. - std::unique_ptr platform_handler_; - // Handler for cursor events. std::unique_ptr cursor_handler_; diff --git a/shell/platform/windows/platform_handler.cc b/shell/platform/windows/platform_handler.cc index c73677da1b1f8..ff1e02d564bb0 100644 --- a/shell/platform/windows/platform_handler.cc +++ b/shell/platform/windows/platform_handler.cc @@ -199,14 +199,14 @@ int ScopedClipboard::SetString(const std::wstring string) { PlatformHandler::PlatformHandler( BinaryMessenger* messenger, - FlutterWindowsView* view, + FlutterWindowsEngine* engine, std::optional()>> scoped_clipboard_provider) : channel_(std::make_unique>( messenger, kChannelName, &JsonMethodCodec::GetInstance())), - view_(view) { + engine_(engine) { channel_->SetMethodCallHandler( [this](const MethodCall& call, std::unique_ptr> result) { @@ -226,10 +226,17 @@ PlatformHandler::~PlatformHandler() = default; void PlatformHandler::GetPlainText( std::unique_ptr> result, std::string_view key) { + const FlutterWindowsView* view = engine_->view(); + if (view == nullptr) { + result->Error(kClipboardError, + "Clipboard is not available in Windows headless mode"); + return; + } + std::unique_ptr clipboard = scoped_clipboard_provider_(); - int open_result = clipboard->Open(std::get(*view_->GetRenderTarget())); + int open_result = clipboard->Open(std::get(*view->GetRenderTarget())); if (open_result != kErrorSuccess) { rapidjson::Document error_code; error_code.SetInt(open_result); @@ -262,11 +269,18 @@ void PlatformHandler::GetPlainText( void PlatformHandler::GetHasStrings( std::unique_ptr> result) { + const FlutterWindowsView* view = engine_->view(); + if (view == nullptr) { + result->Error(kClipboardError, + "Clipboard is not available in Windows headless mode"); + return; + } + std::unique_ptr clipboard = scoped_clipboard_provider_(); bool hasStrings; - int open_result = clipboard->Open(std::get(*view_->GetRenderTarget())); + int open_result = clipboard->Open(std::get(*view->GetRenderTarget())); if (open_result != kErrorSuccess) { // Swallow errors of type ERROR_ACCESS_DENIED. These happen when the app is // not in the foreground and GetHasStrings is irrelevant. @@ -293,10 +307,17 @@ void PlatformHandler::GetHasStrings( void PlatformHandler::SetPlainText( const std::string& text, std::unique_ptr> result) { + const FlutterWindowsView* view = engine_->view(); + if (view == nullptr) { + result->Error(kClipboardError, + "Clipboard is not available in Windows headless mode"); + return; + } + std::unique_ptr clipboard = scoped_clipboard_provider_(); - int open_result = clipboard->Open(std::get(*view_->GetRenderTarget())); + int open_result = clipboard->Open(std::get(*view->GetRenderTarget())); if (open_result != kErrorSuccess) { rapidjson::Document error_code; error_code.SetInt(open_result); diff --git a/shell/platform/windows/platform_handler.h b/shell/platform/windows/platform_handler.h index 4da0ab2180527..a8104fea043d6 100644 --- a/shell/platform/windows/platform_handler.h +++ b/shell/platform/windows/platform_handler.h @@ -18,7 +18,7 @@ namespace flutter { -class FlutterWindowsView; +class FlutterWindowsEngine; class ScopedClipboardInterface; // Handler for internal system channels. @@ -26,7 +26,7 @@ class PlatformHandler { public: explicit PlatformHandler( BinaryMessenger* messenger, - FlutterWindowsView* view, + FlutterWindowsEngine* engine, std::optional()>> scoped_clipboard_provider = std::nullopt); @@ -68,8 +68,8 @@ class PlatformHandler { // The MethodChannel used for communication with the Flutter engine. std::unique_ptr> channel_; - // A reference to the Flutter view. - FlutterWindowsView* view_; + // A reference to the Flutter engine. + FlutterWindowsEngine* engine_; // A scoped clipboard provider that can be passed in for mocking in tests. // Use this to acquire clipboard in each operation to avoid blocking clipboard diff --git a/shell/platform/windows/platform_handler_unittests.cc b/shell/platform/windows/platform_handler_unittests.cc index 604ffcb0679eb..9dbce5c8d7179 100644 --- a/shell/platform/windows/platform_handler_unittests.cc +++ b/shell/platform/windows/platform_handler_unittests.cc @@ -49,10 +49,10 @@ class MockPlatformHandler : public PlatformHandler { public: explicit MockPlatformHandler( BinaryMessenger* messenger, - FlutterWindowsView* view, + FlutterWindowsEngine* engine, std::optional()>> scoped_clipboard_provider = std::nullopt) - : PlatformHandler(messenger, view, scoped_clipboard_provider) {} + : PlatformHandler(messenger, engine, scoped_clipboard_provider) {} virtual ~MockPlatformHandler() = default; @@ -95,12 +95,40 @@ std::string SimulatePlatformMessage(TestBinaryMessenger* messenger, } // namespace -TEST(PlatformHandler, GetClipboardData) { - TestBinaryMessenger messenger; - FlutterWindowsView view( - std::make_unique>()); +class PlatformHandlerTest : public ::testing::Test { + protected: + FlutterWindowsEngine* engine() { return engine_.get(); } + + void use_headless_engine() { + // Set properties required to create the engine. + FlutterDesktopEngineProperties properties = {}; + properties.assets_path = L"C:\\foo\\flutter_assets"; + properties.icu_data_path = L"C:\\foo\\icudtl.dat"; + properties.aot_library_path = L"C:\\foo\\aot.so"; + FlutterProjectBundle project(properties); + + engine_ = std::make_unique(project); + } + + void use_engine_with_view() { + use_headless_engine(); - PlatformHandler platform_handler(&messenger, &view, []() { + auto window = std::make_unique>(); + view_ = std::make_unique(std::move(window)); + + engine_->SetView(view_.get()); + } + + private: + std::unique_ptr engine_; + std::unique_ptr view_; +}; + +TEST_F(PlatformHandlerTest, GetClipboardData) { + use_engine_with_view(); + + TestBinaryMessenger messenger; + PlatformHandler platform_handler(&messenger, engine(), []() { auto clipboard = std::make_unique(); EXPECT_CALL(*clipboard.get(), Open) @@ -120,11 +148,11 @@ TEST(PlatformHandler, GetClipboardData) { EXPECT_EQ(result, "[{\"text\":\"Hello world\"}]"); } -TEST(PlatformHandler, GetClipboardDataRejectsUnknownContentType) { +TEST_F(PlatformHandlerTest, GetClipboardDataRejectsUnknownContentType) { + use_engine_with_view(); + TestBinaryMessenger messenger; - FlutterWindowsView view( - std::make_unique>()); - PlatformHandler platform_handler(&messenger, &view); + PlatformHandler platform_handler(&messenger, engine()); // Requesting an unknown content type is an error. std::string result = SimulatePlatformMessage( @@ -133,12 +161,25 @@ TEST(PlatformHandler, GetClipboardDataRejectsUnknownContentType) { EXPECT_EQ(result, "[\"Clipboard error\",\"Unknown clipboard format\",null]"); } -TEST(PlatformHandler, GetClipboardDataReportsOpenFailure) { +TEST_F(PlatformHandlerTest, GetClipboardDataRequiresView) { + use_headless_engine(); + TestBinaryMessenger messenger; - FlutterWindowsView view( - std::make_unique>()); + PlatformHandler platform_handler(&messenger, engine()); + + std::string result = + SimulatePlatformMessage(&messenger, kClipboardGetDataMessage); + + EXPECT_EQ(result, + "[\"Clipboard error\",\"Clipboard is not available in " + "Windows headless mode\",null]"); +} - PlatformHandler platform_handler(&messenger, &view, []() { +TEST_F(PlatformHandlerTest, GetClipboardDataReportsOpenFailure) { + use_engine_with_view(); + + TestBinaryMessenger messenger; + PlatformHandler platform_handler(&messenger, engine(), []() { auto clipboard = std::make_unique(); EXPECT_CALL(*clipboard.get(), Open) @@ -154,12 +195,11 @@ TEST(PlatformHandler, GetClipboardDataReportsOpenFailure) { EXPECT_EQ(result, "[\"Clipboard error\",\"Unable to open clipboard\",1]"); } -TEST(PlatformHandler, GetClipboardDataReportsGetDataFailure) { - TestBinaryMessenger messenger; - FlutterWindowsView view( - std::make_unique>()); +TEST_F(PlatformHandlerTest, GetClipboardDataReportsGetDataFailure) { + use_engine_with_view(); - PlatformHandler platform_handler(&messenger, &view, []() { + TestBinaryMessenger messenger; + PlatformHandler platform_handler(&messenger, engine(), []() { auto clipboard = std::make_unique(); EXPECT_CALL(*clipboard.get(), Open) @@ -179,12 +219,11 @@ TEST(PlatformHandler, GetClipboardDataReportsGetDataFailure) { EXPECT_EQ(result, "[\"Clipboard error\",\"Unable to get clipboard data\",1]"); } -TEST(PlatformHandler, ClipboardHasStrings) { - TestBinaryMessenger messenger; - FlutterWindowsView view( - std::make_unique>()); +TEST_F(PlatformHandlerTest, ClipboardHasStrings) { + use_engine_with_view(); - PlatformHandler platform_handler(&messenger, &view, []() { + TestBinaryMessenger messenger; + PlatformHandler platform_handler(&messenger, engine(), []() { auto clipboard = std::make_unique(); EXPECT_CALL(*clipboard.get(), Open) @@ -201,12 +240,11 @@ TEST(PlatformHandler, ClipboardHasStrings) { EXPECT_EQ(result, "[{\"value\":true}]"); } -TEST(PlatformHandler, ClipboardHasStringsReturnsFalse) { - TestBinaryMessenger messenger; - FlutterWindowsView view( - std::make_unique>()); +TEST_F(PlatformHandlerTest, ClipboardHasStringsReturnsFalse) { + use_engine_with_view(); - PlatformHandler platform_handler(&messenger, &view, []() { + TestBinaryMessenger messenger; + PlatformHandler platform_handler(&messenger, engine(), []() { auto clipboard = std::make_unique(); EXPECT_CALL(*clipboard.get(), Open) @@ -223,11 +261,11 @@ TEST(PlatformHandler, ClipboardHasStringsReturnsFalse) { EXPECT_EQ(result, "[{\"value\":false}]"); } -TEST(PlatformHandler, ClipboardHasStringsRejectsUnknownContentType) { +TEST_F(PlatformHandlerTest, ClipboardHasStringsRejectsUnknownContentType) { + use_engine_with_view(); + TestBinaryMessenger messenger; - FlutterWindowsView view( - std::make_unique>()); - PlatformHandler platform_handler(&messenger, &view); + PlatformHandler platform_handler(&messenger, engine()); std::string result = SimulatePlatformMessage( &messenger, kClipboardHasStringsFakeContentTypeMessage); @@ -235,13 +273,26 @@ TEST(PlatformHandler, ClipboardHasStringsRejectsUnknownContentType) { EXPECT_EQ(result, "[\"Clipboard error\",\"Unknown clipboard format\",null]"); } -// Regression test for https://github.com/flutter/flutter/issues/95817. -TEST(PlatformHandler, ClipboardHasStringsIgnoresPermissionErrors) { +TEST_F(PlatformHandlerTest, ClipboardHasStringsRequiresView) { + use_headless_engine(); + TestBinaryMessenger messenger; - FlutterWindowsView view( - std::make_unique>()); + PlatformHandler platform_handler(&messenger, engine()); + + std::string result = + SimulatePlatformMessage(&messenger, kClipboardHasStringsMessage); + + EXPECT_EQ(result, + "[\"Clipboard error\",\"Clipboard is not available in Windows " + "headless mode\",null]"); +} - PlatformHandler platform_handler(&messenger, &view, []() { +// Regression test for https://github.com/flutter/flutter/issues/95817. +TEST_F(PlatformHandlerTest, ClipboardHasStringsIgnoresPermissionErrors) { + use_engine_with_view(); + + TestBinaryMessenger messenger; + PlatformHandler platform_handler(&messenger, engine(), []() { auto clipboard = std::make_unique(); EXPECT_CALL(*clipboard.get(), Open) @@ -257,12 +308,11 @@ TEST(PlatformHandler, ClipboardHasStringsIgnoresPermissionErrors) { EXPECT_EQ(result, "[{\"value\":false}]"); } -TEST(PlatformHandler, ClipboardHasStringsReportsErrors) { - TestBinaryMessenger messenger; - FlutterWindowsView view( - std::make_unique>()); +TEST_F(PlatformHandlerTest, ClipboardHasStringsReportsErrors) { + use_engine_with_view(); - PlatformHandler platform_handler(&messenger, &view, []() { + TestBinaryMessenger messenger; + PlatformHandler platform_handler(&messenger, engine(), []() { auto clipboard = std::make_unique(); EXPECT_CALL(*clipboard.get(), Open) @@ -278,12 +328,11 @@ TEST(PlatformHandler, ClipboardHasStringsReportsErrors) { EXPECT_EQ(result, "[\"Clipboard error\",\"Unable to open clipboard\",1]"); } -TEST(PlatformHandler, ClipboardSetData) { - TestBinaryMessenger messenger; - FlutterWindowsView view( - std::make_unique>()); +TEST_F(PlatformHandlerTest, ClipboardSetData) { + use_engine_with_view(); - PlatformHandler platform_handler(&messenger, &view, []() { + TestBinaryMessenger messenger; + PlatformHandler platform_handler(&messenger, engine(), []() { auto clipboard = std::make_unique(); EXPECT_CALL(*clipboard.get(), Open) @@ -305,11 +354,11 @@ TEST(PlatformHandler, ClipboardSetData) { EXPECT_EQ(result, "[null]"); } -TEST(PlatformHandler, ClipboardSetDataUnknownType) { +TEST_F(PlatformHandlerTest, ClipboardSetDataUnknownType) { + use_engine_with_view(); + TestBinaryMessenger messenger; - FlutterWindowsView view( - std::make_unique>()); - PlatformHandler platform_handler(&messenger, &view); + PlatformHandler platform_handler(&messenger, engine()); std::string result = SimulatePlatformMessage(&messenger, kClipboardSetDataUnknownTypeMessage); @@ -317,12 +366,25 @@ TEST(PlatformHandler, ClipboardSetDataUnknownType) { EXPECT_EQ(result, "[\"Clipboard error\",\"Unknown clipboard format\",null]"); } -TEST(PlatformHandler, ClipboardSetDataReportsOpenFailure) { +TEST_F(PlatformHandlerTest, ClipboardSetDataRequiresView) { + use_headless_engine(); + TestBinaryMessenger messenger; - FlutterWindowsView view( - std::make_unique>()); + PlatformHandler platform_handler(&messenger, engine()); - PlatformHandler platform_handler(&messenger, &view, []() { + std::string result = + SimulatePlatformMessage(&messenger, kClipboardSetDataMessage); + + EXPECT_EQ(result, + "[\"Clipboard error\",\"Clipboard is not available in Windows " + "headless mode\",null]"); +} + +TEST_F(PlatformHandlerTest, ClipboardSetDataReportsOpenFailure) { + use_engine_with_view(); + + TestBinaryMessenger messenger; + PlatformHandler platform_handler(&messenger, engine(), []() { auto clipboard = std::make_unique(); EXPECT_CALL(*clipboard.get(), Open) @@ -338,12 +400,11 @@ TEST(PlatformHandler, ClipboardSetDataReportsOpenFailure) { EXPECT_EQ(result, "[\"Clipboard error\",\"Unable to open clipboard\",1]"); } -TEST(PlatformHandler, ClipboardSetDataReportsSetDataFailure) { - TestBinaryMessenger messenger; - FlutterWindowsView view( - std::make_unique>()); +TEST_F(PlatformHandlerTest, ClipboardSetDataReportsSetDataFailure) { + use_engine_with_view(); - PlatformHandler platform_handler(&messenger, &view, []() { + TestBinaryMessenger messenger; + PlatformHandler platform_handler(&messenger, engine(), []() { auto clipboard = std::make_unique(); EXPECT_CALL(*clipboard.get(), Open) @@ -362,11 +423,11 @@ TEST(PlatformHandler, ClipboardSetDataReportsSetDataFailure) { EXPECT_EQ(result, "[\"Clipboard error\",\"Unable to set clipboard data\",1]"); } -TEST(PlatformHandler, PlaySystemSound) { +TEST_F(PlatformHandlerTest, PlaySystemSound) { + use_headless_engine(); + TestBinaryMessenger messenger; - FlutterWindowsView view( - std::make_unique>()); - MockPlatformHandler platform_handler(&messenger, &view); + MockPlatformHandler platform_handler(&messenger, engine()); EXPECT_CALL(platform_handler, SystemSoundPlay("SystemSoundType.alert", _)) .WillOnce([](const std::string& sound,