fix: give clipboard/primary TARGETS unique atoms

The clipboard and primary selections previously used the same atom to
store TARGETS when queried for them by `handle_target_list`. When a
program requests both selections simultaneously, problems such as one
selection using the target list of the other, then deleting it and
requiring the second target to get the selection targets again could
occur. This was observed when using `wl-clip-persist` and could result
in selection desyncs.

This change gives a unique atom to each primary and clipboard selection
targets so they no longer compete over the same property. It also adds
updates to the `copy_from_x11` intergration to test for this issue.

src/xstate/mod.rs

@@ -998,15 +998,16 @@ xcb::atoms_struct! {
         motif_wm_hints => b"_MOTIF_WM_HINTS" only_if_exists = false,
         utf8_string => b"UTF8_STRING" only_if_exists = false,
         clipboard => b"CLIPBOARD" only_if_exists = false,
+        clipboard_targets => b"_clipboard_targets" only_if_exists = false,
         targets => b"TARGETS" only_if_exists = false,
         save_targets => b"SAVE_TARGETS" only_if_exists = false,
         multiple => b"MULTIPLE" only_if_exists = false,
         timestamp => b"TIMESTAMP" only_if_exists = false,
-        selection_reply => b"_selection_reply" only_if_exists = false,
         incr => b"INCR" only_if_exists = false,
         xsettings => b"_XSETTINGS_S0" only_if_exists = false,
         xsettings_settings => b"_XSETTINGS_SETTINGS" only_if_exists = false,
         primary => b"PRIMARY" only_if_exists = false,
+        primary_targets => b"_primary_targets" only_if_exists = false,
         moveresize => b"_NET_WM_MOVERESIZE" only_if_exists = false,
     }
 }

src/xstate/selection.rs

@@ -233,6 +233,7 @@ enum CurrentSelection<T: SelectionType> {
 struct SelectionData<T: SelectionType> {
     last_selection_timestamp: u32,
     atom: x::Atom,
+    targets_atom: x::Atom,
     current_selection: Option<CurrentSelection<T>>,
 }
 
@@ -269,10 +270,11 @@ trait SelectionDataImpl {
 }
 
 impl<T: SelectionType> SelectionData<T> {
-    fn new(atom: x::Atom) -> Self {
+    fn new(atom: x::Atom, targets_atom: x::Atom) -> Self {
         Self {
             last_selection_timestamp: x::CURRENT_TIME,
             atom,
+            targets_atom,
             current_selection: None,
         }
     }
@@ -336,7 +338,7 @@ impl<T: SelectionType> SelectionDataImpl for SelectionData<T> {
             requestor: wm_window,
             selection: self.atom,
             target: atoms.targets,
-            property: atoms.selection_reply,
+            property: self.targets_atom,
             time: timestamp,
         }) {
             Ok(_) => {
@@ -571,8 +573,8 @@ impl SelectionState {
             .expect("Couldn't create window for selections");
         Self {
             target_window,
-            clipboard: SelectionData::new(atoms.clipboard),
-            primary: SelectionData::new(atoms.primary),
+            clipboard: SelectionData::new(atoms.clipboard, atoms.clipboard_targets),
+            primary: SelectionData::new(atoms.primary, atoms.primary_targets),
         }
     }
 }

tests/integration.rs

@@ -994,17 +994,21 @@ fn copy_from_x11() {
 
     let window = connection.new_window(connection.root, 0, 0, 20, 20, false);
     f.map_as_toplevel(&mut connection, window);
+
+    connection.set_selection_owner(window, connection.atoms.primary);
     connection.set_selection_owner(window, connection.atoms.clipboard);
 
-    let request = connection.await_selection_request();
-    assert_eq!(request.target(), connection.atoms.targets);
-    connection.set_property(
-        request.requestor(),
-        x::ATOM_ATOM,
-        request.property(),
-        &[connection.atoms.mime1, connection.atoms.mime2],
-    );
-    connection.send_selection_notify(&request);
+    for _ in [connection.atoms.primary, connection.atoms.clipboard] {
+        let request = connection.await_selection_request();
+        assert_eq!(request.target(), connection.atoms.targets);
+        connection.set_property(
+            request.requestor(),
+            x::ATOM_ATOM,
+            request.property(),
+            &[connection.atoms.mime1, connection.atoms.mime2],
+        );
+        connection.send_selection_notify(&request);
+    }
     f.wait_and_dispatch();
 
     struct MimeData {
@@ -1028,21 +1032,37 @@ fn copy_from_x11() {
         },
     ];
 
-    let advertised_mimes = f.testwl.data_source_mimes();
+    // When requesting both primary and clipboard simultaneously, the first to be requested erases
+    // its TARGETS (which is the correct behavior of a GetProperty request), but the second still
+    // tried to use this data and would come up empty.
+    let primary_mimes = f.testwl.primary_source_mimes();
+    let clipboard_mimes = f.testwl.data_source_mimes();
     assert_eq!(
-        advertised_mimes.len(),
+        primary_mimes.len(),
         mimes_truth.len(),
-        "Wrong number of advertised mimes: {advertised_mimes:?}"
+        "Wrong number of advertised primary mimes: {primary_mimes:?}"
+    );
+    assert_eq!(
+        clipboard_mimes.len(),
+        mimes_truth.len(),
+        "Wrong number of advertised clipboard mimes: {clipboard_mimes:?}"
     );
     for MimeData { data, .. } in &mimes_truth {
         assert!(
-            advertised_mimes.contains(&data.mime_type),
+            primary_mimes.contains(&data.mime_type),
+            "Missing mime type {}",
+            data.mime_type
+        );
+        assert!(
+            clipboard_mimes.contains(&data.mime_type),
             "Missing mime type {}",
             data.mime_type
         );
     }
 
-    let data = f.testwl.clipboard_paste_data(|mime, _| {
+    // Type annotations hint the compiler to use HRTBs (needed since this closure is reused).
+    // See: https://users.rust-lang.org/t/implementation-of-fnonce-is-not-general-enough/68294/3
+    let mut send_data_for_mime = |mime: &str, _: &mut testwl::Server| {
         let request = connection.await_selection_request();
         let data = mimes_truth
             .iter()
@@ -1056,29 +1076,35 @@ fn copy_from_x11() {
         );
         connection.send_selection_notify(&request);
         true
-    });
-    let mut found_mimes = Vec::new();
-    for testwl::PasteData { mime_type, data } in data {
-        match &mime_type {
-            x if x == "text/plain" => {
-                assert_eq!(&data, b"hello world");
-            }
-            x if x == "blah/blah" => {
-                assert_eq!(&data, &[1, 2, 3, 4]);
-            }
-            other => panic!("unexpected mime type: {other} ({data:?})"),
-        }
-        found_mimes.push(mime_type);
-    }
+    };
 
-    assert!(
-        found_mimes.contains(&"text/plain".to_string()),
-        "Didn't get mime data for text/plain"
-    );
-    assert!(
-        found_mimes.contains(&"blah/blah".to_string()),
-        "Didn't get mime data for blah/blah"
-    );
+    let clipboard_data = f.testwl.clipboard_paste_data(&mut send_data_for_mime);
+    let primary_data = f.testwl.primary_paste_data(&mut send_data_for_mime);
+
+    for data in [primary_data, clipboard_data] {
+        let mut found_mimes = Vec::new();
+        for testwl::PasteData { mime_type, data } in data {
+            match &mime_type {
+                x if x == "text/plain" => {
+                    assert_eq!(&data, b"hello world");
+                }
+                x if x == "blah/blah" => {
+                    assert_eq!(&data, &[1, 2, 3, 4]);
+                }
+                other => panic!("unexpected mime type: {other} ({data:?})"),
+            }
+            found_mimes.push(mime_type);
+        }
+
+        assert!(
+            found_mimes.contains(&"text/plain".to_string()),
+            "Didn't get mime data for text/plain"
+        );
+        assert!(
+            found_mimes.contains(&"blah/blah".to_string()),
+            "Didn't get mime data for blah/blah"
+        );
+    }
 }
 
 #[test]