[Spice-commits] 5 commits - client/platform.h client/red_client.cpp client/red_client.h client/windows client/x11

Hans de Goede jwrdegoede at kemper.freedesktop.org
Fri Oct 8 01:17:35 PDT 2010


 client/platform.h           |    2 ++
 client/red_client.cpp       |   39 ++++++++++++++++++++++++++++++---------
 client/red_client.h         |   32 ++++++++++++++++++++++++++++++++
 client/windows/platform.cpp |    7 +++++++
 client/x11/platform.cpp     |   19 ++++++++++++++++---
 5 files changed, 87 insertions(+), 12 deletions(-)

New commits:
commit 1e34c14c64f1149fa5c30302d62442d7ce7c79b2
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Wed Oct 6 20:00:54 2010 +0200

    spicec: only send display-config if the agent can handle it

diff --git a/client/red_client.cpp b/client/red_client.cpp
index cf2ee35..61974b5 100644
--- a/client/red_client.cpp
+++ b/client/red_client.cpp
@@ -1013,7 +1013,8 @@ void RedClient::handle_agent_connected(RedPeer::InMessage* message)
         send_agent_monitors_config();
     }
 
-    if (!_agent_disp_config_sent) {
+    if (VD_AGENT_HAS_CAPABILITY(_agent_caps, _agent_caps_size,
+            VD_AGENT_CAP_DISPLAY_CONFIG) && !_agent_disp_config_sent) {
         send_agent_display_config();
     }
 }
@@ -1038,7 +1039,7 @@ void RedClient::on_agent_announce_capabilities(
     memcpy(_agent_caps, caps->caps, sizeof(_agent_caps[0]) * caps_size);
 
     if (VD_AGENT_HAS_CAPABILITY(caps->caps, caps_size,
-        VD_AGENT_DISPLAY_CONFIG)) {
+            VD_AGENT_CAP_DISPLAY_CONFIG)) {
         // not sending the color depth through send_agent_monitors_config, since
         // it applies only for attached screens.
         send_agent_display_config();
commit ab8bac36bd0d5a399b76c9266ff1c752e28f7bd7
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Wed Oct 6 19:52:49 2010 +0200

    spicec-x11: Drop annoying useless warning
    
    Every time an events comes past where the Window is None (which happens
    about once every 5 minutes or so), this annoying "invalid window" message
    gets printed. Remove it!

diff --git a/client/x11/platform.cpp b/client/x11/platform.cpp
index cb2431f..ccae877 100644
--- a/client/x11/platform.cpp
+++ b/client/x11/platform.cpp
@@ -271,7 +271,6 @@ void XEventHandler::on_event()
 
         XNextEvent(&_x_display, &event);
         if (event.xany.window == None) {
-            LOG_WARN("invalid window");
             continue;
         }
 
commit 9b00e93efb197bb215390d9f96204a9f20ffb262
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Wed Oct 6 18:54:31 2010 +0200

    spicec: don't send agent messages directly from ClipboardListener callbacks
    
    ClipboardListener callbacks can run from another thread then the
    main channel loop thread, where agent messages are normally dispatched from.
    
    So they may not send agent messages directly, instead they should post
    events to the main channel loop.

diff --git a/client/red_client.cpp b/client/red_client.cpp
index 0650e35..cf2ee35 100644
--- a/client/red_client.cpp
+++ b/client/red_client.cpp
@@ -101,6 +101,21 @@ void ClipboardRequestEvent::response(AbstractProcessLoop& events_loop)
         VD_AGENT_CLIPBOARD_REQUEST, sizeof(request), &request);
 }
 
+void ClipboardNotifyEvent::response(AbstractProcessLoop& events_loop)
+{
+    static_cast<RedClient*>(events_loop.get_owner())->send_agent_clipboard_notify_message(
+        _type, _data, _size);
+}
+
+void ClipboardReleaseEvent::response(AbstractProcessLoop& events_loop)
+{
+    if (Platform::get_clipboard_owner() != Platform::owner_client)
+        return;
+
+    static_cast<RedClient*>(events_loop.get_owner())->send_agent_clipboard_message(
+        VD_AGENT_CLIPBOARD_RELEASE, 0, NULL);
+}
+
 Migrate::Migrate(RedClient& client)
     : _client (client)
     , _running (false)
@@ -861,6 +876,18 @@ void RedClient::on_clipboard_request(uint32_t type)
 
 void RedClient::on_clipboard_notify(uint32_t type, uint8_t* data, int32_t size)
 {
+    AutoRef<ClipboardNotifyEvent> event(new ClipboardNotifyEvent(type, data, size));
+    get_process_loop().push_event(*event);
+}
+
+void RedClient::on_clipboard_release()
+{
+    AutoRef<ClipboardReleaseEvent> event(new ClipboardReleaseEvent());
+    get_process_loop().push_event(*event);
+}
+
+void RedClient::send_agent_clipboard_notify_message(uint32_t type, uint8_t *data, uint32_t size)
+{
     ASSERT(size && data);
     if (!_agent_connected) {
         return;
@@ -892,12 +919,6 @@ void RedClient::on_clipboard_notify(uint32_t type, uint8_t* data, int32_t size)
     }
 }
 
-void RedClient::on_clipboard_release()
-{
-    if (Platform::get_clipboard_owner() == Platform::owner_client)
-        send_agent_clipboard_message(VD_AGENT_CLIPBOARD_RELEASE, 0, NULL);
-}
-
 void RedClient::set_mouse_mode(uint32_t supported_modes, uint32_t current_mode)
 {
     if (current_mode != _mouse_mode) {
diff --git a/client/red_client.h b/client/red_client.h
index 15f0617..dd64682 100644
--- a/client/red_client.h
+++ b/client/red_client.h
@@ -173,6 +173,35 @@ private:
     uint32_t _type;
 };
 
+class ClipboardNotifyEvent : public Event {
+public:
+    ClipboardNotifyEvent(uint32_t type, uint8_t *data, uint32_t size)
+    {
+        _type = type;
+        _data = new uint8_t [size];
+        memcpy(_data, data, size);
+        _size = size;
+    }
+    ~ClipboardNotifyEvent()
+    {
+        delete[] _data;
+    }
+
+    virtual void response(AbstractProcessLoop& events_loop);
+
+private:
+    uint32_t _type;
+    uint8_t *_data;
+    uint32_t _size;
+};
+
+class ClipboardReleaseEvent : public Event {
+public:
+    ClipboardReleaseEvent() {}
+    virtual void response(AbstractProcessLoop& events_loop);
+};
+
+
 class RedClient: public RedChannel,
                  public Platform::ClipboardListener {
 public:
@@ -180,6 +209,8 @@ public:
     friend class Migrate;
     friend class ClipboardGrabEvent;
     friend class ClipboardRequestEvent;
+    friend class ClipboardNotifyEvent;
+    friend class ClipboardReleaseEvent;
 
     RedClient(Application& application);
     ~RedClient();
@@ -265,6 +296,7 @@ private:
                                         uint32_t msg_size);
     void do_send_agent_clipboard();
     void send_agent_clipboard_message(uint32_t message_type, uint32_t size = 0, void* data = NULL);
+    void send_agent_clipboard_notify_message(uint32_t type, uint8_t *data, uint32_t size);
 
     ChannelFactory* find_factory(uint32_t type);
     void create_channel(uint32_t type, uint32_t id);
commit dddb6ad48ada0ddd5327ee8ab70b87540d5150a5
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Wed Oct 6 16:14:06 2010 +0200

    spicec-x11: Remove a race window in selection ownership release code
    
    Well almost remove it, it was possible that another x11 app would acquire
    selection ownership, and we would receive a release message from the
    agent before having processed the xselection ownership change event.
    
    Then we would set the selection owner to none, overriding the new owner.
    As the comment in the patch indicates there still is a minute window left
    where something similar can happen after this patch. Nothing we can do
    about that (I blame the libX11 selection API).

diff --git a/client/x11/platform.cpp b/client/x11/platform.cpp
index 8f0665c..cb2431f 100644
--- a/client/x11/platform.cpp
+++ b/client/x11/platform.cpp
@@ -3466,7 +3466,15 @@ void Platform::on_clipboard_release()
 {
     XEvent event;
 
+    if (XGetSelectionOwner(x_display, clipboard_prop) != platform_win) {
+        LOG_INFO("Platform::on_clipboard_release() called while not selection owner");
+        return;
+    }
+    /* Note there is a small race window here where another x11 app could
+       acquire selection ownership and we kick it off again, nothing we
+       can do about that :( */
     XSetSelectionOwner(x_display, clipboard_prop, None, CurrentTime);
+
     /* Make sure we process the XFixesSetSelectionOwnerNotify event caused
        by this, so we don't end up changing the clipboard owner to none, after
        it has already been re-owned because this event is still pending. */
commit 7b84db7a7484b58fdde21029ef374baeaa0a4b51
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Wed Oct 6 12:37:05 2010 +0200

    spicec: Move setting of clipboard_owner to guest to platform code
    
    Atleast under x11 there is a race condition when setting the clipboard
    owner to guest from the RedClient code rather then doing it in Platform.
    
    After the XSetSelectionOwner() in Platform::on_clipboard_grab(), which runs
    from the main message loop thread, the x11 event thread can receive a
    SelectionRequest event from another x11 app, before the RedClient code
    has set the clipboard owner, which will trigger the owner != guest
    check in the SelectionRequest event handling code.
    
    By moving the setting of the owner in to Platform::on_clipboard_grab() it
    gets protected by the clipboard lock, which closes this tiny race.

diff --git a/client/platform.h b/client/platform.h
index af4a0f6..a9a1715 100644
--- a/client/platform.h
+++ b/client/platform.h
@@ -132,6 +132,8 @@ public:
     static int  get_clipboard_owner() { return _clipboard_owner; }
 
 private:
+    static void set_clipboard_owner_unlocked(int new_owner);
+
     static int _clipboard_owner;
 };
 
diff --git a/client/red_client.cpp b/client/red_client.cpp
index 8e7dfe0..0650e35 100644
--- a/client/red_client.cpp
+++ b/client/red_client.cpp
@@ -1118,7 +1118,6 @@ void RedClient::dispatch_agent_message(VDAgentMessage* msg, void* data)
     case VD_AGENT_CLIPBOARD_GRAB:
         Platform::on_clipboard_grab((uint32_t *)data,
                                       msg->size / sizeof(uint32_t));
-        Platform::set_clipboard_owner(Platform::owner_guest);
         break;
     case VD_AGENT_CLIPBOARD_REQUEST:
         if (Platform::get_clipboard_owner() != Platform::owner_client) {
diff --git a/client/windows/platform.cpp b/client/windows/platform.cpp
index 580a40a..075d269 100644
--- a/client/windows/platform.cpp
+++ b/client/windows/platform.cpp
@@ -859,6 +859,11 @@ void WinPlatform::exit_modal_loop()
 
 int Platform::_clipboard_owner = Platform::owner_none;
 
+void Platform::set_clipboard_owner_unlocked(int new_owner)
+{
+    set_clipboard_owner(new_owner);
+}
+
 void Platform::set_clipboard_owner(int new_owner)
 {
     if (new_owner == owner_none) {
@@ -885,6 +890,8 @@ bool Platform::on_clipboard_grab(uint32_t *types, uint32_t type_count)
     EmptyClipboard();
     SetClipboardData(format, NULL);
     CloseClipboard();
+
+    set_clipboard_owner(owner_guest);
     return true;
 }
 
diff --git a/client/x11/platform.cpp b/client/x11/platform.cpp
index 29b5f75..8f0665c 100644
--- a/client/x11/platform.cpp
+++ b/client/x11/platform.cpp
@@ -3355,6 +3355,8 @@ bool Platform::on_clipboard_grab(uint32_t *types, uint32_t type_count)
 
     XSetSelectionOwner(x_display, clipboard_prop, platform_win, CurrentTime);
     XFlush(x_display);
+
+    set_clipboard_owner_unlocked(owner_guest);
     return true;
 }
 
@@ -3362,12 +3364,16 @@ int Platform::_clipboard_owner = Platform::owner_none;
 
 void Platform::set_clipboard_owner(int new_owner)
 {
+        Lock lock(clipboard_lock);
+        set_clipboard_owner_unlocked(new_owner);
+}
+
+void Platform::set_clipboard_owner_unlocked(int new_owner)
+{
     const char * const owner_str[] = { "none", "guest", "client" };
 
     /* Clear pending requests and clipboard data */
     {
-        Lock lock(clipboard_lock);
-
         if (next_selection_request) {
             LOG_INFO("selection requests pending upon clipboard owner change, clearing");
             while (next_selection_request)


More information about the Spice-commits mailing list