Fix Windows clipboard handling (flutter#17706)

Fixes several bugs in the clipboard code, and makes some structural
improvements:
- Adds scoped wrappers for clipboard open/close and global lock/unlock,
  to prevent missing cleanup, fixing at least one case where the lock
  was not released.
- Adds the relevant window handle to the clipboard calls, since the docs
  suggest that some operations won't work without one.
- Adds a missing clear step to setting the clipboard data.
- Switches from TEXT to UNICODETEXT to handle non-ASCII text correctly.
  - To enable that, adds UTF-16/-8 conversion utilities built on the
    Win32 APIs (rather than the deprecated std::codecvt functions, as
    have been previously used in the engine).
- Fixes handling of getting data when the clipboard is empty, correctly
  returning null.
- Passes more errors back through the method channel, with details, for
  easier debugging of future issues.

Fixes flutter/flutter#54226

Author: stuartmorgan-g

ci/licenses_golden/licenses_flutter

@@ -1182,6 +1182,9 @@ FILE: ../../../flutter/shell/platform/windows/keyboard_hook_handler.h
 FILE: ../../../flutter/shell/platform/windows/platform_handler.cc
 FILE: ../../../flutter/shell/platform/windows/platform_handler.h
 FILE: ../../../flutter/shell/platform/windows/public/flutter_windows.h
+FILE: ../../../flutter/shell/platform/windows/string_conversion.cc
+FILE: ../../../flutter/shell/platform/windows/string_conversion.h
+FILE: ../../../flutter/shell/platform/windows/string_conversion_unittests.cc
 FILE: ../../../flutter/shell/platform/windows/text_input_plugin.cc
 FILE: ../../../flutter/shell/platform/windows/text_input_plugin.h
 FILE: ../../../flutter/shell/platform/windows/win32_flutter_window.cc

shell/platform/windows/BUILD.gn

@@ -49,6 +49,8 @@ source_set("flutter_windows_source") {
     "keyboard_hook_handler.h",
     "platform_handler.cc",
     "platform_handler.h",
+    "string_conversion.cc",
+    "string_conversion.h",
     "text_input_plugin.cc",
     "text_input_plugin.h",
     "win32_flutter_window.cc",
@@ -108,6 +110,7 @@ executable("flutter_windows_unittests") {
 
   sources = [
     "dpi_utils_unittests.cc",
+    "string_conversion_unittests.cc",
     "testing/win32_flutter_window_test.cc",
     "testing/win32_flutter_window_test.h",
     "testing/win32_window_test.cc",

shell/platform/windows/platform_handler.cc

@@ -7,8 +7,11 @@
 #include <windows.h>
 
 #include <iostream>
+#include <optional>
 
 #include "flutter/shell/platform/common/cpp/json_method_codec.h"
+#include "flutter/shell/platform/windows/string_conversion.h"
+#include "flutter/shell/platform/windows/win32_flutter_window.h"
 
 static constexpr char kChannelName[] = "flutter/platform";
 
@@ -18,11 +21,182 @@ static constexpr char kSetClipboardDataMethod[] = "Clipboard.setData";
 static constexpr char kTextPlainFormat[] = "text/plain";
 static constexpr char kTextKey[] = "text";
 
-static constexpr char kUnknownClipboardFormatError[] =
-    "Unknown clipboard format error";
+static constexpr char kClipboardError[] = "Clipboard error";
+static constexpr char kUnknownClipboardFormatMessage[] =
+    "Unknown clipboard format";
 
 namespace flutter {
 
+namespace {
+
+// A scoped wrapper for GlobalAlloc/GlobalFree.
+class ScopedGlobalMemory {
+ public:
+  // Allocates |bytes| bytes of global memory with the given flags.
+  ScopedGlobalMemory(unsigned int flags, size_t bytes) {
+    memory_ = ::GlobalAlloc(flags, bytes);
+    if (!memory_) {
+      std::cerr << "Unable to allocate global memory: " << ::GetLastError();
+    }
+  }
+
+  ~ScopedGlobalMemory() {
+    if (memory_) {
+      if (::GlobalFree(memory_) != nullptr) {
+        std::cerr << "Failed to free global allocation: " << ::GetLastError();
+      }
+    }
+  }
+
+  // Prevent copying.
+  ScopedGlobalMemory(ScopedGlobalMemory const&) = delete;
+  ScopedGlobalMemory& operator=(ScopedGlobalMemory const&) = delete;
+
+  // Returns the memory pointer, which will be nullptr if allocation failed.
+  void* get() { return memory_; }
+
+  void* release() {
+    void* memory = memory_;
+    memory_ = nullptr;
+    return memory;
+  }
+
+ private:
+  HGLOBAL memory_;
+};
+
+// A scoped wrapper for GlobalLock/GlobalUnlock.
+class ScopedGlobalLock {
+ public:
+  // Attempts to acquire a global lock on |memory| for the life of this object.
+  ScopedGlobalLock(HGLOBAL memory) {
+    source_ = memory;
+    if (memory) {
+      locked_memory_ = ::GlobalLock(memory);
+      if (!locked_memory_) {
+        std::cerr << "Unable to acquire global lock: " << ::GetLastError();
+      }
+    }
+  }
+
+  ~ScopedGlobalLock() {
+    if (locked_memory_) {
+      if (!::GlobalUnlock(source_)) {
+        DWORD error = ::GetLastError();
+        if (error != NO_ERROR) {
+          std::cerr << "Unable to release global lock: " << ::GetLastError();
+        }
+      }
+    }
+  }
+
+  // Prevent copying.
+  ScopedGlobalLock(ScopedGlobalLock const&) = delete;
+  ScopedGlobalLock& operator=(ScopedGlobalLock const&) = delete;
+
+  // Returns the locked memory pointer, which will be nullptr if acquiring the
+  // lock failed.
+  void* get() { return locked_memory_; }
+
+ private:
+  HGLOBAL source_;
+  void* locked_memory_;
+};
+
+// A Clipboard wrapper that automatically closes the clipboard when it goes out
+// of scope.
+class ScopedClipboard {
+ public:
+  ScopedClipboard();
+  ~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);
+
+  // Returns true if there is string data available to get.
+  bool HasString();
+
+  // 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<std::wstring> GetString();
+
+  // 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);
+
+ private:
+  bool opened_ = false;
+};
+
+ScopedClipboard::ScopedClipboard() {}
+
+ScopedClipboard::~ScopedClipboard() {
+  if (opened_) {
+    ::CloseClipboard();
+  }
+}
+
+bool ScopedClipboard::Open(HWND window) {
+  opened_ = ::OpenClipboard(window);
+  return opened_;
+}
+
+bool ScopedClipboard::HasString() {
+  // Allow either plain text format, since getting data will auto-interpolate.
+  return ::IsClipboardFormatAvailable(CF_UNICODETEXT) ||
+         ::IsClipboardFormatAvailable(CF_TEXT);
+}
+
+std::optional<std::wstring> ScopedClipboard::GetString() {
+  assert(opened_);
+
+  HANDLE data = ::GetClipboardData(CF_UNICODETEXT);
+  if (data == nullptr) {
+    return std::nullopt;
+  }
+  ScopedGlobalLock locked_data(data);
+  if (!locked_data.get()) {
+    return std::nullopt;
+  }
+  return std::optional<std::wstring>(static_cast<wchar_t*>(locked_data.get()));
+}
+
+bool ScopedClipboard::SetString(const std::wstring string) {
+  assert(opened_);
+  if (!::EmptyClipboard()) {
+    return false;
+  }
+  size_t null_terminated_byte_count =
+      sizeof(decltype(string)::traits_type::char_type) * (string.size() + 1);
+  ScopedGlobalMemory destination_memory(GMEM_MOVEABLE,
+                                        null_terminated_byte_count);
+  ScopedGlobalLock locked_memory(destination_memory.get());
+  if (!locked_memory.get()) {
+    return false;
+  }
+  memcpy(locked_memory.get(), string.c_str(), null_terminated_byte_count);
+  if (!::SetClipboardData(CF_UNICODETEXT, locked_memory.get())) {
+    return false;
+  }
+  // The clipboard now owns the global memory.
+  destination_memory.release();
+  return true;
+}
+
+}  // namespace
+
 PlatformHandler::PlatformHandler(flutter::BinaryMessenger* messenger,
                                  Win32FlutterWindow* window)
     : channel_(std::make_unique<flutter::MethodChannel<rapidjson::Document>>(
@@ -47,76 +221,65 @@ void PlatformHandler::HandleMethodCall(
     const rapidjson::Value& format = method_call.arguments()[0];
 
     if (strcmp(format.GetString(), kTextPlainFormat) != 0) {
-      result->Error(kUnknownClipboardFormatError,
-                    "Windows clipboard API only supports text.");
+      result->Error(kClipboardError, kUnknownClipboardFormatMessage);
       return;
     }
 
-    auto clipboardData = GetClipboardString();
-
-    if (clipboardData.empty()) {
-      result->Error(kUnknownClipboardFormatError,
-                    "Failed to retrieve clipboard data from win32 api.");
+    ScopedClipboard clipboard;
+    if (!clipboard.Open(window_->GetWindowHandle())) {
+      rapidjson::Document error_code;
+      error_code.SetInt(::GetLastError());
+      result->Error(kClipboardError, "Unable to open clipboard", &error_code);
       return;
     }
+    if (!clipboard.HasString()) {
+      rapidjson::Document null;
+      result->Success(&null);
+      return;
+    }
+    std::optional<std::wstring> clipboard_string = clipboard.GetString();
+    if (!clipboard_string) {
+      rapidjson::Document error_code;
+      error_code.SetInt(::GetLastError());
+      result->Error(kClipboardError, "Unable to get clipboard data",
+                    &error_code);
+      return;
+    }
+
     rapidjson::Document document;
     document.SetObject();
     rapidjson::Document::AllocatorType& allocator = document.GetAllocator();
-    document.AddMember(rapidjson::Value(kTextKey, allocator),
-                       rapidjson::Value(clipboardData, allocator), allocator);
+    document.AddMember(
+        rapidjson::Value(kTextKey, allocator),
+        rapidjson::Value(Utf8FromUtf16(*clipboard_string), allocator),
+        allocator);
     result->Success(&document);
-
   } else if (method.compare(kSetClipboardDataMethod) == 0) {
     const rapidjson::Value& document = *method_call.arguments();
     rapidjson::Value::ConstMemberIterator itr = document.FindMember(kTextKey);
     if (itr == document.MemberEnd()) {
-      result->Error(kUnknownClipboardFormatError,
-                    "Missing text to store on clipboard.");
+      result->Error(kClipboardError, kUnknownClipboardFormatMessage);
+      return;
+    }
+
+    ScopedClipboard clipboard;
+    if (!clipboard.Open(window_->GetWindowHandle())) {
+      rapidjson::Document error_code;
+      error_code.SetInt(::GetLastError());
+      result->Error(kClipboardError, "Unable to open clipboard", &error_code);
+      return;
+    }
+    if (!clipboard.SetString(Utf16FromUtf8(itr->value.GetString()))) {
+      rapidjson::Document error_code;
+      error_code.SetInt(::GetLastError());
+      result->Error(kClipboardError, "Unable to set clipboard data",
+                    &error_code);
       return;
     }
-    SetClipboardString(std::string(itr->value.GetString()));
     result->Success();
   } else {
     result->NotImplemented();
   }
 }
 
-std::string PlatformHandler::GetClipboardString() {
-  if (!OpenClipboard(nullptr)) {
-    return nullptr;
-  }
-
-  HANDLE data = GetClipboardData(CF_TEXT);
-  if (data == nullptr) {
-    CloseClipboard();
-    return nullptr;
-  }
-
-  const char* clipboardData = static_cast<char*>(GlobalLock(data));
-
-  if (clipboardData == nullptr) {
-    CloseClipboard();
-    return nullptr;
-  }
-
-  auto result = std::string(clipboardData);
-  GlobalUnlock(data);
-  CloseClipboard();
-  return result;
-}
-
-void PlatformHandler::SetClipboardString(std::string data) {
-  if (!OpenClipboard(nullptr)) {
-    return;
-  }
-
-  auto htext = GlobalAlloc(GMEM_MOVEABLE, data.size());
-
-  memcpy(GlobalLock(htext), data.c_str(), data.size());
-
-  SetClipboardData(CF_TEXT, htext);
-
-  CloseClipboard();
-}
-
 }  // namespace flutter

shell/platform/windows/platform_handler.h

@@ -29,9 +29,6 @@ class PlatformHandler {
   // The MethodChannel used for communication with the Flutter engine.
   std::unique_ptr<flutter::MethodChannel<rapidjson::Document>> channel_;
 
-  static std::string GetClipboardString();
-  static void SetClipboardString(std::string data);
-
   // A reference to the win32 window.
   Win32FlutterWindow* window_;
 };