Handle INCR selections properly

Closes #82

src/xstate/mod.rs

@@ -626,12 +626,15 @@ impl XState {
     }
 
     fn handle_property_change(
-        &self,
+        &mut self,
         event: x::PropertyNotifyEvent,
         server_state: &mut super::RealServerState,
     ) {
         if event.state() != x::Property::NewValue {
-            debug!("ignoring non newvalue for property {:?}", event.atom());
+            debug!(
+                "ignoring non newvalue for property {:?}",
+                get_atom_name(&self.connection, event.atom())
+            );
             return;
         }
 
@@ -663,7 +666,9 @@ impl XState {
                 server_state.set_win_class(window, class);
             }
             _ => {
-                if log::log_enabled!(log::Level::Debug) {
+                if !self.handle_selection_property_change(&event, server_state)
+                    && log::log_enabled!(log::Level::Debug)
+                {
                     debug!(
                         "changed property {:?} for {:?}",
                         get_atom_name(&self.connection, event.atom()),
@@ -698,6 +703,7 @@ xcb::atoms_struct! {
         pub multiple => b"MULTIPLE" only_if_exists = false,
         pub timestamp => b"TIMESTAMP" only_if_exists = false,
         pub selection_reply => b"_selection_reply" only_if_exists = false,
+        pub incr => b"INCR" only_if_exists = false,
     }
 }
 

src/xstate/selection.rs

@@ -32,6 +32,7 @@ impl MimeTypeData for SelectionTarget {
     fn data(&self) -> &[u8] {
         match &self.value {
             TargetValue::U8(v) => v,
+            TargetValue::U32(v) => unsafe { v.align_to().1 },
             other => {
                 warn!(
                     "Unexpectedly requesting data from mime type with data type {} - nothing will be copied",
@@ -43,13 +44,23 @@ impl MimeTypeData for SelectionTarget {
     }
 }
 
+enum PendingMimeDataType {
+    Standard,
+    Incremental(TargetValue),
+}
+
+struct PendingMimeData {
+    ty: PendingMimeDataType,
+    id: SelectionTargetId,
+    dest_property: x::Atom,
+}
+
 enum MimeTypes {
     Temporary {
         /// Temporary mime data, being built
         data: Vec<SelectionTarget>,
         /// Mime types we still need to receive feedback on
-        /// 2nd field is the destination property
+        to_grab: Vec<PendingMimeData>,
-        to_grab: Vec<(SelectionTargetId, x::Atom)>,
     },
     /// Done grabbing mime data
     Complete(Rc<Vec<SelectionTarget>>),
@@ -118,7 +129,7 @@ impl XState {
 
         self.selection_data.mime_types = MimeTypes::Complete(Rc::new(types));
         self.selection_data.foreign_data = Some(selection);
-        trace!("Clipboard set from Wayland");
+        debug!("Clipboard set from Wayland");
     }
 
     pub(crate) fn handle_selection_event(
@@ -157,19 +168,23 @@ impl XState {
                     return true;
                 }
 
+                trace!(
+                    "selection notify target: {}",
+                    get_atom_name(&self.connection, e.target())
+                );
                 match e.target() {
                     x if x == self.atoms.targets => self.handle_target_list(e.property()),
                     atom => self.handle_clipboard_data(atom),
                 }
 
-                if let MimeTypes::Temporary { data, to_grab } = &mut self.selection_data.mime_types
+                if let MimeTypes::Temporary { to_grab, .. } = &self.selection_data.mime_types {
-                {
                     if to_grab.is_empty() {
-                        let data = Rc::new(std::mem::take(data));
+                        let MimeTypes::Temporary { data, .. } =
-                        self.selection_data.mime_types = MimeTypes::Complete(data.clone());
+                            std::mem::take(&mut self.selection_data.mime_types)
-                        self.set_clipboard_owner(self.selection_data.clear_time.unwrap());
+                        else {
-                        server_state.set_copy_paste_source(data);
+                            unreachable!()
-                        trace!("Clipboard set from X11");
+                        };
+                        self.finish_mime_data(server_state, data);
                     }
                 }
             }
@@ -320,26 +335,27 @@ impl XState {
                         only_if_exists: false,
                     }))
                     .unwrap();
-                let dest = reply.atom();
+                let dest_property = reply.atom();
 
                 self.connection
                     .send_and_check_request(&x::ConvertSelection {
                         requestor: self.wm_window,
                         selection: self.atoms.clipboard,
                         target: target_atom,
-                        property: dest,
+                        property: dest_property,
                         time: self.selection_data.clear_time.as_ref().copied().unwrap(),
                     })
                     .unwrap();
 
                 let target_name = get_atom_name(&self.connection, target_atom);
-                (
+                PendingMimeData {
-                    SelectionTargetId {
+                    ty: PendingMimeDataType::Standard,
+                    id: SelectionTargetId {
                         name: target_name,
                         atom: target_atom,
                     },
-                    dest,
+                    dest_property,
-                )
+                }
             })
             .collect();
 
@@ -355,7 +371,10 @@ impl XState {
             return;
         };
 
-        let Some(idx) = to_grab.iter().position(|(id, _)| id.atom == atom) else {
+        let Some(idx) = to_grab
+            .iter()
+            .position(|PendingMimeData { id, .. }| id.atom == atom)
+        else {
             warn!(
                 "unexpected SelectionNotify type: {}",
                 get_atom_name(&self.connection, atom)
@@ -363,7 +382,11 @@ impl XState {
             return;
         };
 
-        let (id, dest) = to_grab.swap_remove(idx);
+        let PendingMimeData {
+            ty,
+            id,
+            dest_property,
+        } = to_grab.swap_remove(idx);
 
         let value = match atom {
             x if x == self.atoms.timestamp => TargetValue::U32(vec![self
@@ -373,36 +396,47 @@ impl XState {
                 .copied()
                 .unwrap()]),
             _ => {
-                let reply = self
+                let reply = get_property_any(&self.connection, self.wm_window, dest_property);
-                    .connection
-                    .wait_for_reply(self.connection.send_request(&x::GetProperty {
-                        delete: true,
-                        window: self.wm_window,
-                        property: dest,
-                        r#type: x::ATOM_ANY,
-                        long_offset: 0,
-                        long_length: u32::MAX,
-                    }))
-                    .unwrap();
 
-                match reply.format() {
+                trace!(
-                    8 => TargetValue::U8(reply.value().to_vec()),
+                    "got type {} for mime type {}",
-                    16 => TargetValue::U16(reply.value().to_vec()),
+                    get_atom_name(&self.connection, reply.r#type()),
-                    32 => TargetValue::U32(reply.value().to_vec()),
+                    get_atom_name(&self.connection, atom)
-                    other => {
+                );
-                        if log::log_enabled!(log::Level::Debug) {
+
-                            let target_name = &id.name;
+                match reply.r#type() {
-                            let ty = if reply.r#type() == x::ATOM_NONE {
+                    x if x == self.atoms.incr => {
-                                "None".to_string()
+                        assert!(matches!(ty, PendingMimeDataType::Standard));
-                            } else {
+                        debug!(
-                                get_atom_name(&self.connection, reply.r#type())
+                            "beginning incr process for {}",
-                            };
+                            get_atom_name(&self.connection, atom)
-                            let dest = get_atom_name(&self.connection, dest);
+                        );
-                            let value = reply.value::<u8>().to_vec();
+                        if let Some(data) =
-                            debug!("unexpected format: {other} (atom: {target_name}, type: {ty:?}, property: {dest}, value: {value:?})");
+                            begin_incr(&self.connection, self.wm_window, reply, id, dest_property)
+                        {
+                            to_grab.push(data);
                         }
                         return;
                     }
+                    _ => match reply.format() {
+                        8 => TargetValue::U8(reply.value().to_vec()),
+                        16 => TargetValue::U16(reply.value().to_vec()),
+                        32 => TargetValue::U32(reply.value().to_vec()),
+                        other => {
+                            if log::log_enabled!(log::Level::Debug) {
+                                let target_name = &id.name;
+                                let ty = if reply.r#type() == x::ATOM_NONE {
+                                    "None".to_string()
+                                } else {
+                                    get_atom_name(&self.connection, reply.r#type())
+                                };
+                                let dest = get_atom_name(&self.connection, dest_property);
+                                let value = reply.value::<u8>().to_vec();
+                                debug!("unexpected format: {other} (atom: {target_name}, type: {ty:?}, property: {dest}, value: {value:?})");
+                            }
+                            return;
+                        }
+                    },
                 }
             }
         };
@@ -410,4 +444,148 @@ impl XState {
         trace!("Selection data: {id:?} {value:?}");
         data.push(SelectionTarget { id, value });
     }
+
+    pub(super) fn handle_selection_property_change(
+        &mut self,
+        event: &x::PropertyNotifyEvent,
+        server_state: &mut RealServerState,
+    ) -> bool {
+        if event.window() != self.wm_window {
+            return false;
+        }
+
+        let MimeTypes::Temporary { data, to_grab } = &mut self.selection_data.mime_types else {
+            debug!("Got potential selection property change, but not awaiting mime data");
+            return false;
+        };
+
+        let Some(idx) = to_grab.iter().position(|p| {
+            matches!(p.ty, PendingMimeDataType::Incremental(_)) && p.dest_property == event.atom()
+        }) else {
+            debug!(
+                "Changed non selection property: {}",
+                get_atom_name(&self.connection, event.atom())
+            );
+            return false;
+        };
+
+        let pending = &mut to_grab[idx];
+        let reply = get_property_any(&self.connection, self.wm_window, pending.dest_property);
+
+        if reply.r#type() != pending.id.atom {
+            warn!(
+                "wrong getproperty type: {}",
+                get_atom_name(&self.connection, reply.r#type())
+            );
+            return false;
+        }
+
+        match reply.format() {
+            8 => {
+                let value: &[u8] = reply.value();
+                trace!("got incr data ({} bytes)", value.len());
+                if value.is_empty() {
+                    let pending = to_grab.swap_remove(idx);
+                    let PendingMimeDataType::Incremental(value) = pending.ty else {
+                        unreachable!()
+                    };
+                    let atom = pending.id.atom;
+                    data.push(SelectionTarget {
+                        id: pending.id,
+                        value,
+                    });
+                    trace!(
+                        "finalized incr for {}",
+                        get_atom_name(&self.connection, atom)
+                    );
+                } else {
+                    let PendingMimeDataType::Incremental(TargetValue::U8(data)) = &mut pending.ty
+                    else {
+                        unreachable!()
+                    };
+                    data.extend_from_slice(value);
+                    trace!("new incr len: {}", data.len());
+                }
+            }
+            other => {
+                warn!("Got unexpected format {other} for INCR data - copy/pasting with mime type {} will fail!", get_atom_name(&self.connection, reply.r#type()));
+                to_grab.swap_remove(idx);
+            }
+        }
+
+        if to_grab.is_empty() {
+            let MimeTypes::Temporary { data, .. } =
+                std::mem::take(&mut self.selection_data.mime_types)
+            else {
+                unreachable!()
+            };
+            self.finish_mime_data(server_state, data);
+        }
+
+        true
+    }
+
+    fn finish_mime_data(&mut self, server_state: &mut RealServerState, data: Vec<SelectionTarget>) {
+        self.connection
+            .send_and_check_request(&x::ChangeWindowAttributes {
+                window: self.wm_window,
+                value_list: &[x::Cw::EventMask(x::EventMask::empty())],
+            })
+            .unwrap();
+        let data = Rc::new(data);
+        self.selection_data.mime_types = MimeTypes::Complete(data.clone());
+        self.set_clipboard_owner(self.selection_data.clear_time.unwrap());
+        server_state.set_copy_paste_source(data);
+        debug!("Clipboard set from X11");
+    }
+}
+
+fn get_property_any(
+    connection: &xcb::Connection,
+    window: x::Window,
+    property: x::Atom,
+) -> x::GetPropertyReply {
+    connection
+        .wait_for_reply(connection.send_request(&x::GetProperty {
+            delete: true,
+            window,
+            property,
+            r#type: x::ATOM_ANY,
+            long_offset: 0,
+            long_length: u32::MAX,
+        }))
+        .unwrap()
+}
+fn begin_incr(
+    connection: &xcb::Connection,
+    window: x::Window,
+    reply: x::GetPropertyReply,
+    id: SelectionTargetId,
+    dest_property: x::Atom,
+) -> Option<PendingMimeData> {
+    let size = match reply.format() {
+        8 => reply.value::<u8>()[0] as usize,
+        16 => reply.value::<u16>()[0] as usize,
+        32 => reply.value::<u32>()[0] as usize,
+        other => {
+            warn!("unexpected incr format: {other}");
+            return None;
+        }
+    };
+
+    connection
+        .send_and_check_request(&x::ChangeWindowAttributes {
+            window,
+            value_list: &[x::Cw::EventMask(x::EventMask::PROPERTY_CHANGE)],
+        })
+        .unwrap();
+
+    // XXX: storing INCR property data in memory could significantly bloat memory depending on how
+    // much data is going to be stuck into the clipboard, but we'll cross that bridge when we get
+    // to it.
+    Some(PendingMimeData {
+        ty: PendingMimeDataType::Incremental(TargetValue::U8(Vec::with_capacity(size))),
+        id,
+        dest_property,
+    })
 }

tests/integration.rs

@@ -268,6 +268,7 @@ xcb::atoms_struct! {
         wm_check => b"_NET_SUPPORTING_WM_CHECK",
         mime1 => b"text/plain" only_if_exists = false,
         mime2 => b"blah/blah" only_if_exists = false,
+        incr => b"INCR",
     }
 }
 
@@ -383,6 +384,47 @@ impl Connection {
     {
         self.wait_for_reply(self.send_request(req)).unwrap()
     }
+
+    #[track_caller]
+    fn set_selection_owner(&self, window: x::Window) {
+        self.send_and_check_request(&x::SetSelectionOwner {
+            owner: window,
+            selection: self.atoms.clipboard,
+            time: x::CURRENT_TIME,
+        })
+        .unwrap();
+        let owner = self
+            .wait_for_reply(self.send_request(&x::GetSelectionOwner {
+                selection: self.atoms.clipboard,
+            }))
+            .unwrap();
+        assert_eq!(window, owner.owner());
+    }
+
+    #[track_caller]
+    fn send_selection_notify(&self, request: &x::SelectionRequestEvent) {
+        self.send_and_check_request(&x::SendEvent {
+            propagate: false,
+            destination: x::SendEventDest::Window(request.requestor()),
+            event_mask: x::EventMask::empty(),
+            event: &x::SelectionNotifyEvent::new(
+                request.time(),
+                request.requestor(),
+                request.selection(),
+                request.target(),
+                request.property(),
+            ),
+        })
+        .unwrap();
+    }
+
+    #[track_caller]
+    fn atom_name(&self, atom: x::Atom) -> String {
+        self.get_reply(&x::GetAtomName { atom })
+            .name()
+            .as_ascii()
+            .to_string()
+    }
 }
 
 #[test]
@@ -683,20 +725,7 @@ fn copy_from_x11() {
         .expect("No surface created");
     f.configure_and_verify_new_toplevel(&mut connection, window, surface);
 
-    // set data
+    connection.set_selection_owner(window);
-    connection
-        .send_and_check_request(&x::SetSelectionOwner {
-            owner: window,
-            selection: connection.atoms.clipboard,
-            time: x::CURRENT_TIME,
-        })
-        .unwrap();
-    let owner = connection
-        .wait_for_reply(connection.send_request(&x::GetSelectionOwner {
-            selection: connection.atoms.clipboard,
-        }))
-        .unwrap();
-    assert_eq!(window, owner.owner());
 
     // wait for requests to come through
     std::thread::sleep(std::time::Duration::from_millis(100));
@@ -712,20 +741,7 @@ fn copy_from_x11() {
         request.property(),
         &[connection.atoms.mime1, connection.atoms.mime2],
     );
-    connection
+    connection.send_selection_notify(&request);
-        .send_and_check_request(&x::SendEvent {
-            propagate: false,
-            destination: x::SendEventDest::Window(request.requestor()),
-            event_mask: x::EventMask::empty(),
-            event: &x::SelectionNotifyEvent::new(
-                request.time(),
-                request.requestor(),
-                request.selection(),
-                request.target(),
-                request.property(),
-            ),
-        })
-        .unwrap();
 
     connection.await_event();
     let mut mime_data = vec![
@@ -1197,3 +1213,116 @@ fn primary_output() {
     let reply = conn.get_reply(&xcb::randr::GetOutputPrimary { window: conn.root });
     assert_eq!(reply.output(), output3);
 }
+
+// TODO: these sleeps are horrible.
+#[test]
+fn incr_copy_from_x11() {
+    let mut f = Fixture::new();
+    let mut connection = Connection::new(&f.display);
+
+    let window = connection.new_window(connection.root, 0, 0, 20, 20, false);
+    f.map_as_toplevel(&mut connection, window);
+
+    connection.set_selection_owner(window);
+    std::thread::sleep(std::time::Duration::from_millis(10));
+    let request = match connection.poll_for_event().unwrap() {
+        Some(xcb::Event::X(x::Event::SelectionRequest(r))) => r,
+        other => panic!("Didn't get selection request event, instead got {other:?}"),
+    };
+    assert_eq!(request.target(), connection.atoms.targets);
+    connection.set_property(
+        request.requestor(),
+        x::ATOM_ATOM,
+        request.property(),
+        &[connection.atoms.targets, connection.atoms.mime1],
+    );
+    connection.send_selection_notify(&request);
+    connection.await_event();
+
+    let request = match connection.poll_for_event().unwrap() {
+        Some(xcb::Event::X(x::Event::SelectionRequest(r))) => r,
+        other => panic!("Didn't get selection request event, instead got {other:?}"),
+    };
+    assert_eq!(request.target(), connection.atoms.mime1);
+    let destination_property = request.property();
+
+    connection
+        .send_and_check_request(&x::ChangeWindowAttributes {
+            window: request.requestor(),
+            value_list: &[x::Cw::EventMask(x::EventMask::PROPERTY_CHANGE)],
+        })
+        .unwrap();
+    connection.set_property(
+        request.requestor(),
+        connection.atoms.incr,
+        destination_property,
+        &[3000u32],
+    );
+    connection.send_selection_notify(&request);
+    // skip NewValue
+    let notify = match connection.poll_for_event().unwrap() {
+        Some(xcb::Event::X(x::Event::PropertyNotify(p))) => p,
+        other => panic!("Didn't get property notify event, instead got {other:?}"),
+    };
+    assert_eq!(notify.atom(), request.property());
+    assert_eq!(notify.state(), x::Property::NewValue);
+
+    let data: Vec<u8> = std::iter::successors(Some(1u8), |n| Some(n.wrapping_add(1)))
+        .take(3000)
+        .collect();
+    for (idx, chunk) in data.chunks(500).enumerate() {
+        std::thread::sleep(std::time::Duration::from_millis(10));
+        let notify = match connection.poll_for_event().unwrap() {
+            Some(xcb::Event::X(x::Event::PropertyNotify(p))) => p,
+            other => panic!("Didn't get property notify event, instead got {other:?}"),
+        };
+        assert_eq!(notify.atom(), destination_property, "chunk {idx}");
+        assert_eq!(notify.state(), x::Property::Delete, "chunk {idx}");
+
+        connection.set_property(
+            request.requestor(),
+            connection.atoms.mime1,
+            destination_property,
+            chunk,
+        );
+        // skip NewValue
+        let notify = match connection.poll_for_event().unwrap() {
+            Some(xcb::Event::X(x::Event::PropertyNotify(p))) => p,
+            other => panic!("Didn't get property notify event, instead got {other:?}"),
+        };
+        assert_eq!(notify.atom(), destination_property, "chunk {idx}");
+        assert_eq!(notify.state(), x::Property::NewValue, "chunk {idx}");
+    }
+
+    std::thread::sleep(std::time::Duration::from_millis(10));
+    let notify = match connection.poll_for_event().unwrap() {
+        Some(xcb::Event::X(x::Event::PropertyNotify(p))) => p,
+        other => panic!("Didn't get property notify event, instead got {other:?}"),
+    };
+    assert_eq!(notify.atom(), destination_property);
+    assert_eq!(notify.state(), x::Property::Delete);
+    connection.set_property::<u8>(
+        request.requestor(),
+        connection.atoms.mime1,
+        destination_property,
+        &[],
+    );
+
+    std::thread::sleep(std::time::Duration::from_millis(100));
+    let owner = connection
+        .wait_for_reply(connection.send_request(&x::GetSelectionOwner {
+            selection: connection.atoms.clipboard,
+        }))
+        .unwrap();
+    assert_ne!(window, owner.owner());
+    f.wait_and_dispatch();
+
+    assert_eq!(f.testwl.data_source_mimes(), vec!["text/plain"]);
+    let wl_data = f.testwl.paste_data();
+    f.testwl.dispatch();
+    let mut wl_data = wl_data.resolve();
+    assert_eq!(wl_data.len(), 1);
+    let wl_data = wl_data.swap_remove(0);
+    assert_eq!(wl_data.mime_type, "text/plain");
+    assert_eq!(&wl_data.data, &data);
+}