diff --git a/engine/src/flutter/shell/platform/windows/platform_handler_win32.cc b/engine/src/flutter/shell/platform/windows/platform_handler_win32.cc index 93d391a8f8f..6932e8e0dd7 100644 --- a/engine/src/flutter/shell/platform/windows/platform_handler_win32.cc +++ b/engine/src/flutter/shell/platform/windows/platform_handler_win32.cc @@ -191,12 +191,15 @@ std::unique_ptr PlatformHandler::Create( PlatformHandlerWin32::PlatformHandlerWin32( BinaryMessenger* messenger, FlutterWindowsView* view, - std::unique_ptr clipboard) + std::optional()>> + scoped_clipboard_provider) : PlatformHandler(messenger), view_(view) { - if (clipboard == nullptr) { - clipboard_ = std::make_unique(); + if (scoped_clipboard_provider.has_value()) { + scoped_clipboard_provider_ = scoped_clipboard_provider.value(); } else { - clipboard_ = std::move(clipboard); + scoped_clipboard_provider_ = []() { + return std::make_unique(); + }; } } @@ -205,18 +208,21 @@ PlatformHandlerWin32::~PlatformHandlerWin32() = default; void PlatformHandlerWin32::GetPlainText( std::unique_ptr> result, std::string_view key) { - int open_result = clipboard_->Open(std::get(*view_->GetRenderTarget())); + std::unique_ptr clipboard = + scoped_clipboard_provider_(); + + int open_result = clipboard->Open(std::get(*view_->GetRenderTarget())); if (open_result != kErrorSuccess) { rapidjson::Document error_code; error_code.SetInt(open_result); result->Error(kClipboardError, "Unable to open clipboard", error_code); return; } - if (!clipboard_->HasString()) { + if (!clipboard->HasString()) { result->Success(rapidjson::Document()); return; } - std::variant get_string_result = clipboard_->GetString(); + std::variant get_string_result = clipboard->GetString(); if (std::holds_alternative(get_string_result)) { rapidjson::Document error_code; error_code.SetInt(std::get(get_string_result)); @@ -238,8 +244,11 @@ void PlatformHandlerWin32::GetPlainText( void PlatformHandlerWin32::GetHasStrings( std::unique_ptr> result) { + 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. @@ -252,7 +261,7 @@ void PlatformHandlerWin32::GetHasStrings( } hasStrings = false; } else { - hasStrings = clipboard_->HasString(); + hasStrings = clipboard->HasString(); } rapidjson::Document document; @@ -266,14 +275,17 @@ void PlatformHandlerWin32::GetHasStrings( void PlatformHandlerWin32::SetPlainText( const std::string& text, std::unique_ptr> result) { - int open_result = clipboard_->Open(std::get(*view_->GetRenderTarget())); + std::unique_ptr clipboard = + scoped_clipboard_provider_(); + + int open_result = clipboard->Open(std::get(*view_->GetRenderTarget())); if (open_result != kErrorSuccess) { rapidjson::Document error_code; error_code.SetInt(open_result); result->Error(kClipboardError, "Unable to open clipboard", error_code); return; } - int set_result = clipboard_->SetString(fml::Utf8ToWideString(text)); + int set_result = clipboard->SetString(fml::Utf8ToWideString(text)); if (set_result != kErrorSuccess) { rapidjson::Document error_code; error_code.SetInt(set_result); diff --git a/engine/src/flutter/shell/platform/windows/platform_handler_win32.h b/engine/src/flutter/shell/platform/windows/platform_handler_win32.h index 62d64441584..86cf2a3adfe 100644 --- a/engine/src/flutter/shell/platform/windows/platform_handler_win32.h +++ b/engine/src/flutter/shell/platform/windows/platform_handler_win32.h @@ -48,7 +48,8 @@ class PlatformHandlerWin32 : public PlatformHandler { explicit PlatformHandlerWin32( BinaryMessenger* messenger, FlutterWindowsView* view, - std::unique_ptr clipboard = nullptr); + std::optional()>> + scoped_clipboard_provider = std::nullopt); virtual ~PlatformHandlerWin32(); @@ -74,8 +75,11 @@ class PlatformHandlerWin32 : public PlatformHandler { private: // A reference to the Flutter view. FlutterWindowsView* view_; - // A clipboard instance that can be passed in for mocking in tests. - std::unique_ptr clipboard_; + // 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 + // unnecessarily. See flutter/flutter#103205. + std::function()> + scoped_clipboard_provider_; }; } // namespace flutter diff --git a/engine/src/flutter/shell/platform/windows/platform_handler_win32_unittests.cc b/engine/src/flutter/shell/platform/windows/platform_handler_win32_unittests.cc index 9d350857640..6b1afabae8a 100644 --- a/engine/src/flutter/shell/platform/windows/platform_handler_win32_unittests.cc +++ b/engine/src/flutter/shell/platform/windows/platform_handler_win32_unittests.cc @@ -32,10 +32,34 @@ static constexpr int kArbitraryErrorCode = 1; } // namespace +// A test version of system clipboard. +class MockSystemClipboard { + public: + void OpenClipboard() { opened = true; } + void CloseClipboard() { opened = false; } + bool opened = false; +}; + +class TestPlatformHandlerWin32 : public PlatformHandlerWin32 { + public: + explicit TestPlatformHandlerWin32( + BinaryMessenger* messenger, + FlutterWindowsView* view, + std::optional()>> + scoped_clipboard_provider = std::nullopt) + : PlatformHandlerWin32(messenger, view, scoped_clipboard_provider) {} + + virtual ~TestPlatformHandlerWin32() = default; + + FRIEND_TEST(PlatformHandlerWin32, ReleaseClipboard); +}; + // A test version of the private ScopedClipboard. class TestScopedClipboard : public ScopedClipboardInterface { public: - TestScopedClipboard(int open_error, bool has_strings); + TestScopedClipboard(int open_error, + bool has_strings, + std::shared_ptr clipboard); ~TestScopedClipboard(); // Prevent copying. @@ -54,20 +78,28 @@ class TestScopedClipboard : public ScopedClipboardInterface { bool opened_ = false; bool has_strings_; int open_error_; + std::shared_ptr clipboard_; }; -TestScopedClipboard::TestScopedClipboard(int open_error, bool has_strings) { +TestScopedClipboard::TestScopedClipboard( + int open_error, + bool has_strings, + std::shared_ptr clipboard = nullptr) { open_error_ = open_error; has_strings_ = has_strings; -}; + clipboard_ = clipboard; +} TestScopedClipboard::~TestScopedClipboard() { - if (opened_) { - ::CloseClipboard(); + if ((!open_error_) && clipboard_ != nullptr) { + clipboard_->CloseClipboard(); } } int TestScopedClipboard::Open(HWND window) { + if ((!open_error_) && clipboard_ != nullptr) { + clipboard_->OpenClipboard(); + } return open_error_; } @@ -100,9 +132,9 @@ TEST(PlatformHandlerWin32, HasStringsAccessDeniedReturnsFalseWithoutError) { std::make_unique<::testing::NiceMock>()); // HasStrings will receive access denied on the clipboard, but will return // false without error. - PlatformHandlerWin32 platform_handler( - &messenger, &view, - std::make_unique(kAccessDeniedErrorCode, true)); + PlatformHandlerWin32 platform_handler(&messenger, &view, []() { + return std::make_unique(kAccessDeniedErrorCode, true); + }); auto args = std::make_unique(rapidjson::kStringType); auto& allocator = args->GetAllocator(); @@ -136,9 +168,9 @@ TEST(PlatformHandlerWin32, HasStringsSuccessWithStrings) { FlutterWindowsView view( std::make_unique<::testing::NiceMock>()); // HasStrings will succeed and return true. - PlatformHandlerWin32 platform_handler( - &messenger, &view, - std::make_unique(kErrorSuccess, true)); + PlatformHandlerWin32 platform_handler(&messenger, &view, []() { + return std::make_unique(kErrorSuccess, true); + }); auto args = std::make_unique(rapidjson::kStringType); auto& allocator = args->GetAllocator(); @@ -172,9 +204,9 @@ TEST(PlatformHandlerWin32, HasStringsSuccessWithoutStrings) { FlutterWindowsView view( std::make_unique<::testing::NiceMock>()); // HasStrings will succeed and return false. - PlatformHandlerWin32 platform_handler( - &messenger, &view, - std::make_unique(kErrorSuccess, false)); + PlatformHandlerWin32 platform_handler(&messenger, &view, []() { + return std::make_unique(kErrorSuccess, false); + }); auto args = std::make_unique(rapidjson::kStringType); auto& allocator = args->GetAllocator(); @@ -208,9 +240,9 @@ TEST(PlatformHandlerWin32, HasStringsError) { FlutterWindowsView view( std::make_unique<::testing::NiceMock>()); // HasStrings will fail. - PlatformHandlerWin32 platform_handler( - &messenger, &view, - std::make_unique(kArbitraryErrorCode, true)); + PlatformHandlerWin32 platform_handler(&messenger, &view, []() { + return std::make_unique(kArbitraryErrorCode, true); + }); auto args = std::make_unique(rapidjson::kStringType); auto& allocator = args->GetAllocator(); @@ -237,5 +269,28 @@ TEST(PlatformHandlerWin32, HasStringsError) { })); } +// Regression test for https://github.com/flutter/flutter/issues/103205. +TEST(PlatformHandlerWin32, ReleaseClipboard) { + auto system_clipboard = std::make_shared(); + + TestBinaryMessenger messenger; + FlutterWindowsView view( + std::make_unique<::testing::NiceMock>()); + TestPlatformHandlerWin32 platform_handler( + &messenger, &view, [system_clipboard]() { + return std::make_unique(kErrorSuccess, false, + system_clipboard); + }); + + platform_handler.GetPlainText(std::make_unique(), "text"); + ASSERT_FALSE(system_clipboard->opened); + + platform_handler.GetHasStrings(std::make_unique()); + ASSERT_FALSE(system_clipboard->opened); + + platform_handler.SetPlainText("", std::make_unique()); + ASSERT_FALSE(system_clipboard->opened); +} + } // namespace testing } // namespace flutter