Encapsulate the deferred window closing

This makes it a bit easier to tweak the closing behaviour.

src/wine-host/editor.cpp

@@ -112,6 +112,25 @@ xcb_window_t get_x11_handle(HWND win32_handle);
  */
 ATOM get_window_class();
 
+DeferredWindow::DeferredWindow(HWND window) : handle(window) {}
+
+DeferredWindow::~DeferredWindow() {
+    // XXX: I'm pretty sure this is a Wine bug (or, well, an unfortunate
+    //      interaction of Wine's behaviour and our embedding). If we don't
+    //      explicitly hide the window before sending a `WM_CLOSE` (in
+    //      `destroy_window_async()`), then we might get an X11 error because
+    //      the closing of the window will trigger an X11 event in Wine's X11drv
+    //      which then tries to interact with the no longer existing window.
+    //      Manually hiding the window seems to work around this.
+    // TODO: Check if we also have to do something special for
+    //       editor_double_embed (probalby not)
+    // TODO: Retest XEmbed after all of these changes
+    ShowWindow(handle, SW_MINIMIZE);
+
+    // The actual destroying will happen as part of the Win32 message loop
+    PostMessage(handle, WM_CLOSE, 0, 0);
+}
+
 Editor::Editor(const Configuration& config,
                const size_t parent_window_handle,
                std::optional<fu2::unique_function<void()>> timer_proc)
@@ -123,7 +142,7 @@ Editor::Editor(const Configuration& config,
       // be drawn without any decorations (making resizes behave as you'd
       // expect) and also causes mouse coordinates to be relative to the window
       // itself.
-      win32_handle(CreateWindowEx(WS_EX_TOOLWINDOW,
+      win32_window(CreateWindowEx(WS_EX_TOOLWINDOW,
                                   reinterpret_cast<LPCSTR>(get_window_class()),
                                   "yabridge plugin",
                                   WS_POPUP,
@@ -134,16 +153,15 @@ Editor::Editor(const Configuration& config,
                                   nullptr,
                                   nullptr,
                                   GetModuleHandle(nullptr),
-                                  this),
+                                  this)),
-                   destroy_window_async),
       // If `config.editor_double_embed` is set, then we'll also create a child
-      // window in `win32_child_handle`. If we do this before calling
+      // window in `win32_child_window`. If we do this before calling
-      // `ShowWindow()` on `win32_handle` we'll run into X11 errors.
+      // `ShowWindow()` on `win32_window` we'll run into X11 errors.
-      win32_child_handle(std::nullopt),
+      win32_child_window(std::nullopt),
       idle_timer(
           timer_proc
               ? Win32Timer(
-                    win32_handle.get(),
+                    win32_window.handle,
                     idle_timer_id,
                     std::chrono::duration_cast<std::chrono::milliseconds>(
                         config.event_loop_interval())
@@ -151,7 +169,7 @@ Editor::Editor(const Configuration& config,
               : Win32Timer()),
       idle_timer_proc(std::move(timer_proc)),
       parent_window(parent_window_handle),
-      wine_window(get_x11_handle(win32_handle.get())),
+      wine_window(get_x11_handle(win32_window.handle)),
       topmost_window(
           find_ancestor_windows(*x11_connection, parent_window).back()) {
     xcb_generic_error_t* error;
@@ -251,27 +269,17 @@ Editor::Editor(const Configuration& config,
 
         // If we're using the double embedding option, then the child window
         // should only be created after the parent window is visible
-        ShowWindow(win32_handle.get(), SW_SHOWNORMAL);
+        ShowWindow(win32_window.handle, SW_SHOWNORMAL);
         if (config.editor_double_embed) {
-            // FIXME: This emits `-Wignored-attributes` as of Wine 5.22
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wignored-attributes"
             // As explained above, we can't do this directly in the initializer
             // list
-            win32_child_handle =
+            win32_child_window.emplace(CreateWindowEx(
-                std::unique_ptr<std::remove_pointer_t<HWND>,
+                WS_EX_TOOLWINDOW, reinterpret_cast<LPCSTR>(get_window_class()),
-                                decltype(&destroy_window_async)>(
+                "yabridge plugin child", WS_CHILD, CW_USEDEFAULT, CW_USEDEFAULT,
-                    CreateWindowEx(WS_EX_TOOLWINDOW,
+                client_area.width, client_area.height, win32_window.handle,
-                                   reinterpret_cast<LPCSTR>(get_window_class()),
+                nullptr, GetModuleHandle(nullptr), this));
-                                   "yabridge plugin child", WS_CHILD,
-                                   CW_USEDEFAULT, CW_USEDEFAULT,
-                                   client_area.width, client_area.height,
-                                   win32_handle.get(), nullptr,
-                                   GetModuleHandle(nullptr), this),
-                    destroy_window_async);
-#pragma GCC diagnostic pop
 
-            ShowWindow(win32_child_handle->get(), SW_SHOWNORMAL);
+            ShowWindow(win32_child_window->handle, SW_SHOWNORMAL);
         }
 
         // HACK: I can't seem to figure why the initial reparent would fail on
@@ -295,26 +303,14 @@ Editor::~Editor() {
 
     xcb_reparent_window(x11_connection.get(), wine_window, root, 0, 0);
     xcb_flush(x11_connection.get());
-
-    // XXX: I'm pretty sure this is a Wine bug (or, well, an unfortunate
-    //      interaction of Wine's behaviour and our embedding). If we don't
-    //      explicitly hide the window before sending a `WM_CLOSE` (in
-    //      `destroy_window_async()`), then we might get an X11 error because
-    //      the closing of the window will trigger an X11 event in Wine's X11drv
-    //      which then tries to interact with the no longer existing window.
-    //      Manually hiding the window seems to work around this.
-    // TODO: Check if we also have to do something special for
-    //       editor_double_embed (probalby not)
-    // TODO: Retest XEmbed after all of these changes
-    ShowWindow(win32_handle.get(), SW_MINIMIZE);
 }
 
 HWND Editor::get_win32_handle() const {
     // FIXME: The double embed and XEmbed options don't work together right now
-    if (win32_child_handle && !use_xembed) {
+    if (win32_child_window && !use_xembed) {
-        return win32_child_handle->get();
+        return win32_child_window->handle;
     } else {
-        return win32_handle.get();
+        return win32_window.handle;
     }
 }
 
@@ -483,10 +479,6 @@ void Editor::maybe_run_timer_proc() {
     }
 }
 
-void Editor::destroy_window_async(HWND window_handle) {
-    PostMessage(window_handle, WM_CLOSE, 0, 0);
-}
-
 bool Editor::is_wine_window_active() const {
     if (!supports_ewmh_active_window()) {
         return false;
@@ -589,7 +581,7 @@ void Editor::do_xembed() const {
     xcb_map_window(x11_connection.get(), wine_window);
     xcb_flush(x11_connection.get());
 
-    ShowWindow(win32_handle.get(), SW_SHOWNORMAL);
+    ShowWindow(win32_window.handle, SW_SHOWNORMAL);
 }
 
 LRESULT CALLBACK window_proc(HWND handle,

src/wine-host/editor.h

@@ -55,6 +55,38 @@ struct Size {
     uint16_t height;
 };
 
+/**
+ * A RAII wrapper around windows created using `CreateWindow()` that will post a
+ * `WM_CLOSE` message to the window's message loop so it can clean itself up
+ * later. Directly calling `DestroyWindow()` might hang for a second or two, so
+ * deferring this increases responsiveness.
+ *
+ * This is essentially an alternative around `std::unique_ptr` with a non-static
+ * custom deleter.
+ *
+ * FIXME: It seems like there's a bug in Wine's X11Drv that _sometimes_ causes
+ *        the window to get deleted twice resulting in an Xlib error inside of
+ *        Wine.
+ */
+class DeferredWindow {
+   public:
+    /**
+     * Manage a window so that it will be asynchronously closed when this object
+     * gets dropped.
+     *
+     * @param window A `HWND` obtained through a call to `CreateWindowEx`
+     */
+    DeferredWindow(HWND window);
+
+    /**
+     * Post a `WM_CLOSE` message to the `handle`'s message queue as described
+     * above.
+     */
+    ~DeferredWindow();
+
+    const HWND handle;
+};
+
 /**
  * A wrapper around the win32 windowing API to create and destroy editor
  * windows. We can embed this window into the window provided by the host, and a
@@ -89,7 +121,7 @@ class Editor {
      *   plugins to periodically call `effEditIdle` from the message loop
      *   thread, even when the GUI is blocked.
      *
-     * @see win32_handle
+     * @see win32_window
      */
     Editor(
         const Configuration& config,
@@ -154,13 +186,6 @@ class Editor {
     const bool use_xembed;
 
    private:
-    /**
-     * Post a message to this window's message queue to clean up the window.
-     * This way we don't have to wait for the window to actually fully close
-     * before returning.
-     */
-    static void destroy_window_async(HWND window_handle);
-
     /**
      * Returns `true` if the currently active window (as per
      * `_NET_ACTIVE_WINDOW`) contains `wine_window`. If the window manager does
@@ -205,31 +230,21 @@ class Editor {
      */
     const Size client_area;
 
-    // FIXME: This emits `-Wignored-attributes` as of Wine 5.22
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wignored-attributes"
-
     /**
      * The handle for the window created through Wine that the plugin uses to
      * embed itself in.
      */
-    std::unique_ptr<std::remove_pointer_t<HWND>,
+    DeferredWindow win32_window;
-                    decltype(&destroy_window_async)>
-        win32_handle;
 
     /**
-     * A child window embedded inside of `win32_handle`. This is only used if
+     * A child window embedded inside of `win32_window`. This is only used if
      * the `editor_double_embed` option is enabled. It can be used as a
      * workaround for plugins that rely on their parent window's screen
      * coordinates instead of their own (see the 'Editor hosting modes' section
      * of the readme for more details). The plugin should then embed itself
      * within this child window.
      */
-    std::optional<std::unique_ptr<std::remove_pointer_t<HWND>,
+    std::optional<DeferredWindow> win32_child_window;
-                                  decltype(&destroy_window_async)>>
-        win32_child_handle;
-
-#pragma GCC diagnostic pop
 
     /**
      * A timer we'll use to periodically run `idle_timer_proc`, if set. Thisi is
@@ -253,7 +268,7 @@ class Editor {
      */
     const xcb_window_t parent_window;
     /**
-     * The X11 window handle of the window belonging to  `win32_handle`.
+     * The X11 window handle of the window belonging to  `win32_window`.
      */
     const xcb_window_t wine_window;
     /**