diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index cc529b958e279..77997513f80b8 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -2271,6 +2271,7 @@ FILE: ../../../flutter/shell/platform/windows/platform_handler.h FILE: ../../../flutter/shell/platform/windows/platform_handler_unittests.cc FILE: ../../../flutter/shell/platform/windows/platform_handler_win32.cc FILE: ../../../flutter/shell/platform/windows/platform_handler_win32.h +FILE: ../../../flutter/shell/platform/windows/platform_handler_win32_unittests.cc FILE: ../../../flutter/shell/platform/windows/public/flutter_windows.h FILE: ../../../flutter/shell/platform/windows/sequential_id_generator.cc FILE: ../../../flutter/shell/platform/windows/sequential_id_generator.h diff --git a/shell/platform/windows/BUILD.gn b/shell/platform/windows/BUILD.gn index 1ac8628583c06..51157138fdd69 100644 --- a/shell/platform/windows/BUILD.gn +++ b/shell/platform/windows/BUILD.gn @@ -183,6 +183,7 @@ executable("flutter_windows_unittests") { "keyboard_key_handler_unittests.cc", "keyboard_win32_unittests.cc", "platform_handler_unittests.cc", + "platform_handler_win32_unittests.cc", "sequential_id_generator_unittests.cc", "settings_plugin_unittests.cc", "system_utils_unittests.cc", diff --git a/shell/platform/windows/platform_handler_win32.cc b/shell/platform/windows/platform_handler_win32.cc index 7f3b885748d88..dac80a0e5a9b5 100644 --- a/shell/platform/windows/platform_handler_win32.cc +++ b/shell/platform/windows/platform_handler_win32.cc @@ -14,6 +14,8 @@ #include "flutter/shell/platform/windows/flutter_windows_view.h" static constexpr char kValueKey[] = "value"; +static constexpr int kAccessDeniedErrorCode = 5; +static constexpr int kErrorSuccess = 0; namespace flutter { @@ -95,36 +97,22 @@ class ScopedGlobalLock { // A Clipboard wrapper that automatically closes the clipboard when it goes out // of scope. -class ScopedClipboard { +class ScopedClipboard : public ScopedClipboardInterface { public: ScopedClipboard(); - ~ScopedClipboard(); + virtual ~ScopedClipboard(); // Prevent copying. ScopedClipboard(ScopedClipboard const&) = delete; ScopedClipboard& operator=(ScopedClipboard const&) = delete; - // Attempts to open the clipboard for the given window, returning true if - // successful. - bool Open(HWND window); + int Open(HWND window) override; - // Returns true if there is string data available to get. - bool HasString(); + bool HasString() override; - // Returns string data from the clipboard. - // - // If getting a string fails, returns no value. Get error information with - // ::GetLastError(). - // - // Open(...) must have succeeded to call this method. - std::optional GetString(); + std::variant GetString() override; - // Sets the string content of the clipboard, returning true on success. - // - // On failure, get error information with ::GetLastError(). - // - // Open(...) must have succeeded to call this method. - bool SetString(const std::wstring string); + int SetString(const std::wstring string) override; private: bool opened_ = false; @@ -138,9 +126,14 @@ ScopedClipboard::~ScopedClipboard() { } } -bool ScopedClipboard::Open(HWND window) { +int ScopedClipboard::Open(HWND window) { opened_ = ::OpenClipboard(window); - return opened_; + + if (!opened_) { + return ::GetLastError(); + } + + return kErrorSuccess; } bool ScopedClipboard::HasString() { @@ -149,24 +142,25 @@ bool ScopedClipboard::HasString() { ::IsClipboardFormatAvailable(CF_TEXT); } -std::optional ScopedClipboard::GetString() { +std::variant ScopedClipboard::GetString() { assert(opened_); HANDLE data = ::GetClipboardData(CF_UNICODETEXT); if (data == nullptr) { - return std::nullopt; + return ::GetLastError(); } ScopedGlobalLock locked_data(data); + if (!locked_data.get()) { - return std::nullopt; + return ::GetLastError(); } - return std::optional(static_cast(locked_data.get())); + return static_cast(locked_data.get()); } -bool ScopedClipboard::SetString(const std::wstring string) { +int ScopedClipboard::SetString(const std::wstring string) { assert(opened_); if (!::EmptyClipboard()) { - return false; + return ::GetLastError(); } size_t null_terminated_byte_count = sizeof(decltype(string)::traits_type::char_type) * (string.size() + 1); @@ -174,15 +168,15 @@ bool ScopedClipboard::SetString(const std::wstring string) { null_terminated_byte_count); ScopedGlobalLock locked_memory(destination_memory.get()); if (!locked_memory.get()) { - return false; + return ::GetLastError(); } memcpy(locked_memory.get(), string.c_str(), null_terminated_byte_count); if (!::SetClipboardData(CF_UNICODETEXT, locked_memory.get())) { - return false; + return ::GetLastError(); } // The clipboard now owns the global memory. destination_memory.release(); - return true; + return kErrorSuccess; } } // namespace @@ -194,30 +188,38 @@ std::unique_ptr PlatformHandler::Create( return std::make_unique(messenger, view); } -PlatformHandlerWin32::PlatformHandlerWin32(BinaryMessenger* messenger, - FlutterWindowsView* view) - : PlatformHandler(messenger), view_(view) {} +PlatformHandlerWin32::PlatformHandlerWin32( + BinaryMessenger* messenger, + FlutterWindowsView* view, + std::unique_ptr clipboard) + : PlatformHandler(messenger), view_(view) { + if (clipboard == nullptr) { + clipboard_ = std::make_unique(); + } else { + clipboard_ = std::move(clipboard); + } +} PlatformHandlerWin32::~PlatformHandlerWin32() = default; void PlatformHandlerWin32::GetPlainText( std::unique_ptr> result, std::string_view key) { - ScopedClipboard clipboard; - if (!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(::GetLastError()); + 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::optional clipboard_string = clipboard.GetString(); - if (!clipboard_string) { + std::variant get_string_result = clipboard_->GetString(); + if (std::holds_alternative(get_string_result)) { rapidjson::Document error_code; - error_code.SetInt(::GetLastError()); + error_code.SetInt(std::get(get_string_result)); result->Error(kClipboardError, "Unable to get clipboard data", error_code); return; } @@ -227,42 +229,54 @@ void PlatformHandlerWin32::GetPlainText( rapidjson::Document::AllocatorType& allocator = document.GetAllocator(); document.AddMember( rapidjson::Value(key.data(), allocator), - rapidjson::Value(fml::WideStringToUtf8(*clipboard_string), allocator), + rapidjson::Value( + fml::WideStringToUtf8(std::get(get_string_result)), + allocator), allocator); result->Success(document); } void PlatformHandlerWin32::GetHasStrings( std::unique_ptr> result) { - ScopedClipboard clipboard; - if (!clipboard.Open(std::get(*view_->GetRenderTarget()))) { + bool hasStrings; + int open_result = clipboard_->Open(std::get(*view_->GetRenderTarget())); + if (open_result != kErrorSuccess) { rapidjson::Document error_code; - error_code.SetInt(::GetLastError()); - result->Error(kClipboardError, "Unable to open clipboard", error_code); - return; + error_code.SetInt(open_result); + // Swallow errors of type ERROR_ACCESS_DENIED. These happen when the app is + // not in the foreground and GetHasStrings is irrelevant. + // See https://github.com/flutter/flutter/issues/95817. + if (error_code != kAccessDeniedErrorCode) { + result->Error(kClipboardError, "Unable to open clipboard", error_code); + return; + } + hasStrings = false; + } else { + hasStrings = clipboard_->HasString(); } rapidjson::Document document; document.SetObject(); rapidjson::Document::AllocatorType& allocator = document.GetAllocator(); document.AddMember(rapidjson::Value(kValueKey, allocator), - rapidjson::Value(clipboard.HasString()), allocator); + rapidjson::Value(hasStrings), allocator); result->Success(document); } void PlatformHandlerWin32::SetPlainText( const std::string& text, std::unique_ptr> result) { - ScopedClipboard clipboard; - if (!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(::GetLastError()); + error_code.SetInt(open_result); result->Error(kClipboardError, "Unable to open clipboard", error_code); return; } - if (!clipboard.SetString(fml::Utf8ToWideString(text))) { + int set_result = clipboard_->SetString(fml::Utf8ToWideString(text)); + if (set_result != kErrorSuccess) { rapidjson::Document error_code; - error_code.SetInt(::GetLastError()); + error_code.SetInt(set_result); result->Error(kClipboardError, "Unable to set clipboard data", error_code); return; } diff --git a/shell/platform/windows/platform_handler_win32.h b/shell/platform/windows/platform_handler_win32.h index d69915d787c39..62d6444158462 100644 --- a/shell/platform/windows/platform_handler_win32.h +++ b/shell/platform/windows/platform_handler_win32.h @@ -15,11 +15,40 @@ namespace flutter { class FlutterWindowsView; +// A public interface for ScopedClipboard, so that it can be injected into +// PlatformHandlerWin32. +class ScopedClipboardInterface { + public: + virtual ~ScopedClipboardInterface(){}; + + // Attempts to open the clipboard for the given window, returning the error + // code in the case of failure and 0 otherwise. + virtual int Open(HWND window) = 0; + + // Returns true if there is string data available to get. + virtual bool HasString() = 0; + + // Returns string data from the clipboard. + // + // If getting a string fails, returns the error code. + // + // Open(...) must have succeeded to call this method. + virtual std::variant GetString() = 0; + + // Sets the string content of the clipboard, returning the error code on + // failure and 0 otherwise. + // + // Open(...) must have succeeded to call this method. + virtual int SetString(const std::wstring string) = 0; +}; + // Win32 implementation of PlatformHandler. class PlatformHandlerWin32 : public PlatformHandler { public: - explicit PlatformHandlerWin32(BinaryMessenger* messenger, - FlutterWindowsView* view); + explicit PlatformHandlerWin32( + BinaryMessenger* messenger, + FlutterWindowsView* view, + std::unique_ptr clipboard = nullptr); virtual ~PlatformHandlerWin32(); @@ -45,6 +74,8 @@ 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_; }; } // namespace flutter diff --git a/shell/platform/windows/platform_handler_win32_unittests.cc b/shell/platform/windows/platform_handler_win32_unittests.cc new file mode 100644 index 0000000000000..9d350857640f9 --- /dev/null +++ b/shell/platform/windows/platform_handler_win32_unittests.cc @@ -0,0 +1,241 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "flutter/shell/platform/windows/platform_handler_win32.h" + +#include + +#include "flutter/shell/platform/common/json_method_codec.h" +#include "flutter/shell/platform/windows/testing/mock_window_binding_handler.h" +#include "flutter/shell/platform/windows/testing/test_binary_messenger.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" +#include "rapidjson/document.h" + +namespace flutter { +namespace testing { + +namespace { +using ::testing::_; + +static constexpr char kChannelName[] = "flutter/platform"; + +static constexpr char kHasStringsClipboardMethod[] = "Clipboard.hasStrings"; + +static constexpr char kTextPlainFormat[] = "text/plain"; + +static constexpr char kValueKey[] = "value"; +static constexpr int kAccessDeniedErrorCode = 5; +static constexpr int kErrorSuccess = 0; +static constexpr int kArbitraryErrorCode = 1; + +} // namespace + +// A test version of the private ScopedClipboard. +class TestScopedClipboard : public ScopedClipboardInterface { + public: + TestScopedClipboard(int open_error, bool has_strings); + ~TestScopedClipboard(); + + // Prevent copying. + TestScopedClipboard(TestScopedClipboard const&) = delete; + TestScopedClipboard& operator=(TestScopedClipboard const&) = delete; + + int Open(HWND window) override; + + bool HasString() override; + + std::variant GetString() override; + + int SetString(const std::wstring string) override; + + private: + bool opened_ = false; + bool has_strings_; + int open_error_; +}; + +TestScopedClipboard::TestScopedClipboard(int open_error, bool has_strings) { + open_error_ = open_error; + has_strings_ = has_strings; +}; + +TestScopedClipboard::~TestScopedClipboard() { + if (opened_) { + ::CloseClipboard(); + } +} + +int TestScopedClipboard::Open(HWND window) { + return open_error_; +} + +bool TestScopedClipboard::HasString() { + return has_strings_; +} + +std::variant TestScopedClipboard::GetString() { + return -1; +} + +int TestScopedClipboard::SetString(const std::wstring string) { + return -1; +} + +class MockMethodResult : public MethodResult { + public: + MOCK_METHOD1(SuccessInternal, void(const rapidjson::Document*)); + MOCK_METHOD3(ErrorInternal, + void(const std::string&, + const std::string&, + const rapidjson::Document*)); + MOCK_METHOD0(NotImplementedInternal, void()); +}; + +// Regression test for https://github.com/flutter/flutter/issues/95817. +TEST(PlatformHandlerWin32, HasStringsAccessDeniedReturnsFalseWithoutError) { + TestBinaryMessenger messenger; + FlutterWindowsView view( + 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)); + + auto args = std::make_unique(rapidjson::kStringType); + auto& allocator = args->GetAllocator(); + args->SetString(kTextPlainFormat); + auto encoded = JsonMethodCodec::GetInstance().EncodeMethodCall( + MethodCall(kHasStringsClipboardMethod, + std::move(args))); + + MockMethodResult result; + rapidjson::Document document; + document.SetObject(); + rapidjson::Document::AllocatorType& document_allocator = + document.GetAllocator(); + document.AddMember(rapidjson::Value(kValueKey, document_allocator), + rapidjson::Value(false), document_allocator); + + EXPECT_CALL(result, SuccessInternal(_)) + .WillOnce([](const rapidjson::Document* document) { + ASSERT_FALSE((*document)[kValueKey].GetBool()); + }); + EXPECT_TRUE(messenger.SimulateEngineMessage( + kChannelName, encoded->data(), encoded->size(), + [&](const uint8_t* reply, size_t reply_size) { + JsonMethodCodec::GetInstance().DecodeAndProcessResponseEnvelope( + reply, reply_size, &result); + })); +} + +TEST(PlatformHandlerWin32, HasStringsSuccessWithStrings) { + TestBinaryMessenger messenger; + FlutterWindowsView view( + std::make_unique<::testing::NiceMock>()); + // HasStrings will succeed and return true. + PlatformHandlerWin32 platform_handler( + &messenger, &view, + std::make_unique(kErrorSuccess, true)); + + auto args = std::make_unique(rapidjson::kStringType); + auto& allocator = args->GetAllocator(); + args->SetString(kTextPlainFormat); + auto encoded = JsonMethodCodec::GetInstance().EncodeMethodCall( + MethodCall(kHasStringsClipboardMethod, + std::move(args))); + + MockMethodResult result; + rapidjson::Document document; + document.SetObject(); + rapidjson::Document::AllocatorType& document_allocator = + document.GetAllocator(); + document.AddMember(rapidjson::Value(kValueKey, document_allocator), + rapidjson::Value(false), document_allocator); + + EXPECT_CALL(result, SuccessInternal(_)) + .WillOnce([](const rapidjson::Document* document) { + ASSERT_TRUE((*document)[kValueKey].GetBool()); + }); + EXPECT_TRUE(messenger.SimulateEngineMessage( + kChannelName, encoded->data(), encoded->size(), + [&](const uint8_t* reply, size_t reply_size) { + JsonMethodCodec::GetInstance().DecodeAndProcessResponseEnvelope( + reply, reply_size, &result); + })); +} + +TEST(PlatformHandlerWin32, HasStringsSuccessWithoutStrings) { + TestBinaryMessenger messenger; + FlutterWindowsView view( + std::make_unique<::testing::NiceMock>()); + // HasStrings will succeed and return false. + PlatformHandlerWin32 platform_handler( + &messenger, &view, + std::make_unique(kErrorSuccess, false)); + + auto args = std::make_unique(rapidjson::kStringType); + auto& allocator = args->GetAllocator(); + args->SetString(kTextPlainFormat); + auto encoded = JsonMethodCodec::GetInstance().EncodeMethodCall( + MethodCall(kHasStringsClipboardMethod, + std::move(args))); + + MockMethodResult result; + rapidjson::Document document; + document.SetObject(); + rapidjson::Document::AllocatorType& document_allocator = + document.GetAllocator(); + document.AddMember(rapidjson::Value(kValueKey, document_allocator), + rapidjson::Value(false), document_allocator); + + EXPECT_CALL(result, SuccessInternal(_)) + .WillOnce([](const rapidjson::Document* document) { + ASSERT_FALSE((*document)[kValueKey].GetBool()); + }); + EXPECT_TRUE(messenger.SimulateEngineMessage( + kChannelName, encoded->data(), encoded->size(), + [&](const uint8_t* reply, size_t reply_size) { + JsonMethodCodec::GetInstance().DecodeAndProcessResponseEnvelope( + reply, reply_size, &result); + })); +} + +TEST(PlatformHandlerWin32, HasStringsError) { + TestBinaryMessenger messenger; + FlutterWindowsView view( + std::make_unique<::testing::NiceMock>()); + // HasStrings will fail. + PlatformHandlerWin32 platform_handler( + &messenger, &view, + std::make_unique(kArbitraryErrorCode, true)); + + auto args = std::make_unique(rapidjson::kStringType); + auto& allocator = args->GetAllocator(); + args->SetString(kTextPlainFormat); + auto encoded = JsonMethodCodec::GetInstance().EncodeMethodCall( + MethodCall(kHasStringsClipboardMethod, + std::move(args))); + + MockMethodResult result; + rapidjson::Document document; + document.SetObject(); + rapidjson::Document::AllocatorType& document_allocator = + document.GetAllocator(); + document.AddMember(rapidjson::Value(kValueKey, document_allocator), + rapidjson::Value(false), document_allocator); + + EXPECT_CALL(result, SuccessInternal(_)).Times(0); + EXPECT_CALL(result, ErrorInternal(_, _, _)).Times(1); + EXPECT_TRUE(messenger.SimulateEngineMessage( + kChannelName, encoded->data(), encoded->size(), + [&](const uint8_t* reply, size_t reply_size) { + JsonMethodCodec::GetInstance().DecodeAndProcessResponseEnvelope( + reply, reply_size, &result); + })); +} + +} // namespace testing +} // namespace flutter