X11: util design improvements (#534)

src/platform/linux/x11/dnd.rs

@@ -40,7 +40,7 @@ impl DndAtoms {
             b"text/uri-list\0".as_ptr() as *mut c_char,
             b"None\0".as_ptr() as *mut c_char,
         ];
-        let atoms = unsafe { util::get_atoms(xconn, &names) }?;
+        let atoms = unsafe { xconn.get_atoms(&names) }?;
         Ok(DndAtoms {
             aware: atoms[0],
             enter: atoms[1],
@@ -127,13 +127,12 @@ impl Dnd {
             DndState::Accepted => (1, self.atoms.action_private as c_long),
             DndState::Rejected => (0, self.atoms.none as c_long),
         };
-        util::send_client_msg(
+        self.xconn.send_client_msg(
-            &self.xconn,
             target_window,
             target_window,
             self.atoms.status,
             None,
-            (this_window as c_long, accepted, 0, 0, action),
+            [this_window as c_long, accepted, 0, 0, action],
         ).flush()
     }
 
@@ -147,13 +146,12 @@ impl Dnd {
             DndState::Accepted => (1, self.atoms.action_private as c_long),
             DndState::Rejected => (0, self.atoms.none as c_long),
         };
-        util::send_client_msg(
+        self.xconn.send_client_msg(
-            &self.xconn,
             target_window,
             target_window,
             self.atoms.finished,
             None,
-            (this_window as c_long, accepted, action, 0, 0),
+            [this_window as c_long, accepted, action, 0, 0],
         ).flush()
     }
 
@@ -161,8 +159,7 @@ impl Dnd {
         &self,
         source_window: c_ulong,
     ) -> Result<Vec<ffi::Atom>, util::GetPropertyError> {
-        util::get_property(
+        self.xconn.get_property(
-            &self.xconn,
             source_window,
             self.atoms.type_list,
             ffi::XA_ATOM,
@@ -184,8 +181,7 @@ impl Dnd {
         &self,
         window: c_ulong,
     ) -> Result<Vec<c_uchar>, util::GetPropertyError> {
-        util::get_property(
+        self.xconn.get_property(
-            &self.xconn,
             window,
             self.atoms.selection,
             self.atoms.uri_list,

src/platform/linux/x11/ime/input_method.rs

@@ -85,13 +85,11 @@ enum GetXimServersError {
 // modifiers, since we don't want a user who's looking at logs to ask "am I supposed to set
 // XMODIFIERS to `@server=ibus`?!?"
 unsafe fn get_xim_servers(xconn: &Arc<XConnection>) -> Result<Vec<String>, GetXimServersError> {
-    let servers_atom = util::get_atom(&xconn, b"XIM_SERVERS\0")
+    let servers_atom = xconn.get_atom_unchecked(b"XIM_SERVERS\0");
-        .map_err(GetXimServersError::XError)?;
 
     let root = (xconn.xlib.XDefaultRootWindow)(xconn.display);
 
-    let mut atoms: Vec<ffi::Atom> = util::get_property(
+    let mut atoms: Vec<ffi::Atom> = xconn.get_property(
-        &xconn,
         root,
         servers_atom,
         ffi::XA_ATOM,

src/platform/linux/x11/mod.rs

@@ -75,8 +75,7 @@ impl EventsLoop {
     pub fn new(display: Arc<XConnection>) -> EventsLoop {
         let root = unsafe { (display.xlib.XDefaultRootWindow)(display.display) };
 
-        let wm_delete_window = unsafe { util::get_atom(&display, b"WM_DELETE_WINDOW\0") }
+        let wm_delete_window = unsafe { display.get_atom_unchecked(b"WM_DELETE_WINDOW\0") };
-            .expect("Failed to call XInternAtom (WM_DELETE_WINDOW)");
 
         let dnd = Dnd::new(Arc::clone(&display))
             .expect("Failed to call XInternAtoms when initializing drag and drop");
@@ -130,7 +129,7 @@ impl EventsLoop {
             }
         }
 
-        util::update_cached_wm_info(&display, root);
+        display.update_cached_wm_info(root);
 
         let wakeup_dummy_window = unsafe {
             let (x, y, w, h) = (10, 10, 10, 10);
@@ -166,14 +165,8 @@ impl EventsLoop {
         };
 
         // Register for device hotplug events
-        unsafe {
+        // (The request buffer is flushed during `init_device`)
-            util::select_xinput_events(
+        result.display.select_xinput_events(root, ffi::XIAllDevices, ffi::XI_HierarchyChangedMask).queue();
-                &result.display,
-                root,
-                ffi::XIAllDevices,
-                ffi::XI_HierarchyChangedMask,
-            )
-        }.queue(); // The request buffer is flushed during init_device
 
         result.init_device(ffi::XIAllDevices);
 
@@ -484,11 +477,7 @@ impl EventsLoop {
                                         .unwrap()
                                         .inner_pos_to_outer(inner_x, inner_y)
                                 } else {
-                                    let extents = util::get_frame_extents_heuristic(
+                                    let extents = self.display.get_frame_extents_heuristic(window, self.root);
-                                        &self.display,
-                                        window,
-                                        self.root,
-                                    );
                                     let outer_pos = extents.inner_pos_to_outer(inner_x, inner_y);
                                     (*window_state_lock).frame_extents = Some(extents);
                                     outer_pos
@@ -513,7 +502,7 @@ impl EventsLoop {
                 // (which is almost all of them). Failing to correctly update WM info doesn't
                 // really have much impact, since on the WMs affected (xmonad, dwm, etc.) the only
                 // effect is that we waste some time trying to query unsupported properties.
-                util::update_cached_wm_info(&self.display, self.root);
+                self.display.update_cached_wm_info(self.root);
 
                 self.shared_state
                     .borrow()
@@ -614,7 +603,7 @@ impl EventsLoop {
 
                 if state == Pressed {
                     let written = if let Some(ic) = self.ime.borrow().get_context(window) {
-                        unsafe { util::lookup_utf8(&self.display, ic, xkev) }
+                        self.display.lookup_utf8(ic, xkev)
                     } else {
                         return;
                     };
@@ -841,13 +830,8 @@ impl EventsLoop {
                         // The mods field on this event isn't actually populated, so query the
                         // pointer device. In the future, we can likely remove this round-trip by
                         // relying on Xkb for modifier values.
-                        let modifiers = unsafe {
+                        let modifiers = self.display.query_pointer(xev.event, xev.deviceid)
-                            util::query_pointer(
+                            .expect("Failed to query pointer device").get_modifier_state();
-                                &self.display,
-                                xev.event,
-                                xev.deviceid,
-                            )
-                        }.expect("Failed to query pointer device").get_modifier_state();
 
                         callback(Event::WindowEvent { window_id, event: CursorMoved {
                             device_id,
@@ -1094,16 +1078,13 @@ impl EventsLoopProxy {
         // NOTE: This design is taken from the old `WindowProxy::wakeup` implementation. It
         // assumes that X11 is thread safe. Is this true?
         // (WARNING: it's probably not true)
-        unsafe {
+        display.send_client_msg(
-            util::send_client_msg(
-                &display,
             self.wakeup_dummy_window,
             self.wakeup_dummy_window,
             0,
             None,
-                (0, 0, 0, 0, 0),
+            [0, 0, 0, 0, 0],
-            )
+        ).flush().expect("Failed to call XSendEvent after wakeup");
-        }.flush().expect("Failed to call XSendEvent after wakeup");
 
         Ok(())
     }
@@ -1320,14 +1301,8 @@ impl Device {
                 | ffi::XI_RawButtonReleaseMask
                 | ffi::XI_RawKeyPressMask
                 | ffi::XI_RawKeyReleaseMask;
-            unsafe {
+            // The request buffer is flushed when we poll for events
-                util::select_xinput_events(
+            el.display.select_xinput_events(el.root, info.deviceid, mask).queue();
-                    &el.display,
-                    el.root,
-                    info.deviceid,
-                    mask,
-                )
-            }.queue(); // The request buffer is flushed when we poll for events
 
             // Identify scroll axes
             for class_ptr in Device::classes(info) {

src/platform/linux/x11/monitor.rs

@@ -66,7 +66,7 @@ impl MonitorId {
         repr: util::MonitorRepr,
         primary: bool,
     ) -> Self {
-        let (name, hidpi_factor) = unsafe { util::get_output_info(xconn, resources, &repr) };
+        let (name, hidpi_factor) = unsafe { xconn.get_output_info(resources, &repr) };
         let (dimensions, position) = unsafe { (repr.get_dimensions(), repr.get_position()) };
         let rect = util::Rect::new(position, dimensions);
         MonitorId {

src/platform/linux/x11/util/atom.rs

@@ -1,5 +1,6 @@
 use std::collections::HashMap;
 use std::ffi::{CStr, CString};
+use std::fmt::Debug;
 use std::os::raw::*;
 
 use parking_lot::Mutex;
@@ -12,44 +13,55 @@ lazy_static! {
     static ref ATOM_CACHE: Mutex<AtomCache> = Mutex::new(HashMap::with_capacity(2048));
 }
 
-pub unsafe fn get_atom(xconn: &Arc<XConnection>, name: &[u8]) -> Result<ffi::Atom, XError> {
+impl XConnection {
-    let name = CStr::from_bytes_with_nul_unchecked(name); // I trust you. Don't let me down.
+    pub fn get_atom<T: AsRef<CStr> + Debug>(&self, name: T) -> ffi::Atom {
+        let name = name.as_ref();
         let mut atom_cache_lock = ATOM_CACHE.lock();
         let cached_atom = (*atom_cache_lock).get(name).cloned();
         if let Some(atom) = cached_atom {
-        Ok(atom)
+            atom
         } else {
-        let atom = (xconn.xlib.XInternAtom)(
+            let atom = unsafe { (self.xlib.XInternAtom)(
-            xconn.display,
+                self.display,
                 name.as_ptr() as *const c_char,
                 ffi::False,
+            ) };
+            if atom == 0 {
+                let msg = format!(
+                    "`XInternAtom` failed, which really shouldn't happen. Atom: {:?}, Error: {:#?}",
+                    name,
+                    self.check_errors(),
                 );
+                panic!(msg);
+            }
             /*println!(
                 "XInternAtom name:{:?} atom:{:?}",
                 name,
                 atom,
             );*/
-        xconn.check_errors()?;
             (*atom_cache_lock).insert(name.to_owned(), atom);
-        Ok(atom)
+            atom
         }
     }
 
+    pub unsafe fn get_atom_unchecked(&self, name: &[u8]) -> ffi::Atom {
+        debug_assert!(CStr::from_bytes_with_nul(name).is_ok());
+        let name = CStr::from_bytes_with_nul_unchecked(name);
+        self.get_atom(name)
+    }
+
     // Note: this doesn't use caching, for the sake of simplicity.
     // If you're dealing with this many atoms, you'll usually want to cache them locally anyway.
-pub unsafe fn get_atoms(
+    pub unsafe fn get_atoms(&self,  names: &[*mut c_char]) -> Result<Vec<ffi::Atom>, XError> {
-    xconn: &Arc<XConnection>,
-    names: &[*mut c_char],
-) -> Result<Vec<ffi::Atom>, XError> {
         let mut atoms = Vec::with_capacity(names.len());
-    (xconn.xlib.XInternAtoms)(
+        (self.xlib.XInternAtoms)(
-        xconn.display,
+            self.display,
             names.as_ptr() as *mut _,
             names.len() as c_int,
             ffi::False,
             atoms.as_mut_ptr(),
         );
-    xconn.check_errors()?;
+        self.check_errors()?;
         atoms.set_len(names.len());
         /*println!(
             "XInternAtoms atoms:{:?}",
@@ -57,3 +69,4 @@ pub unsafe fn get_atoms(
         );*/
         Ok(atoms)
     }
+}

src/platform/linux/x11/util/client_msg.rs

@@ -0,0 +1,95 @@
+use super::*;
+
+pub type ClientMsgPayload = [c_long; 5];
+
+impl XConnection {
+    pub fn send_event<T: Into<ffi::XEvent>>(
+        &self,
+        target_window: c_ulong,
+        event_mask: Option<c_long>,
+        event: T,
+    ) -> Flusher {
+        let event_mask = event_mask.unwrap_or(ffi::NoEventMask);
+        unsafe {
+            (self.xlib.XSendEvent)(
+                self.display,
+                target_window,
+                ffi::False,
+                event_mask,
+                &mut event.into(),
+            );
+        }
+        Flusher::new(self)
+    }
+
+    pub fn send_client_msg(
+        &self,
+        window: c_ulong,        // The window this is "about"; not necessarily this window
+        target_window: c_ulong, // The window we're sending to
+        message_type: ffi::Atom,
+        event_mask: Option<c_long>,
+        data: ClientMsgPayload,
+    ) -> Flusher {
+        let mut event: ffi::XClientMessageEvent = unsafe { mem::uninitialized() };
+        event.type_ = ffi::ClientMessage;
+        event.display = self.display;
+        event.window = window;
+        event.message_type = message_type;
+        event.format = c_long::FORMAT as c_int;
+        event.data = unsafe { mem::transmute(data) };
+        self.send_event(target_window, event_mask, event)
+    }
+
+    // Prepare yourself for the ultimate in unsafety!
+    // You should favor `send_client_msg` whenever possible, but some protocols (i.e. startup notification) require you
+    // to send more than one message worth of data.
+    pub fn send_client_msg_multi<T: Formattable>(
+        &self,
+        window: c_ulong,        // The window this is "about"; not necessarily this window
+        target_window: c_ulong, // The window we're sending to
+        message_type: ffi::Atom,
+        event_mask: Option<c_long>,
+        data: &[T],
+    ) -> Flusher {
+        let format = T::FORMAT;
+        let size_of_t = mem::size_of::<T>();
+        debug_assert_eq!(size_of_t, format.get_actual_size());
+        let mut event: ffi::XClientMessageEvent = unsafe { mem::uninitialized() };
+        event.type_ = ffi::ClientMessage;
+        event.display = self.display;
+        event.window = window;
+        event.message_type = message_type;
+        event.format = format as c_int;
+
+        let t_per_payload = format.get_payload_size() / size_of_t;
+        assert!(t_per_payload > 0);
+        let payload_count = data.len() / t_per_payload;
+        let payload_remainder = data.len() % t_per_payload;
+        let payload_ptr = data.as_ptr() as *const ClientMsgPayload;
+
+        let mut payload_index = 0;
+        while payload_index < payload_count {
+            let payload = unsafe { payload_ptr.offset(payload_index as isize) };
+            payload_index += 1;
+            event.data = unsafe { mem::transmute(*payload) };
+            self.send_event(target_window, event_mask, &event).queue();
+        }
+
+        if payload_remainder > 0 {
+            let mut payload: ClientMsgPayload = [0; 5];
+            let t_payload = payload.as_mut_ptr() as *mut T;
+            let invalid_payload = unsafe { payload_ptr.offset(payload_index as isize) };
+            let invalid_t_payload = invalid_payload as *const T;
+            let mut t_index = 0;
+            while t_index < payload_remainder {
+                let valid_t = unsafe { invalid_t_payload.offset(t_index as isize) };
+                unsafe { (*t_payload.offset(t_index as isize)) = (*valid_t).clone() };
+                t_index += 1;
+            }
+            event.data = unsafe { mem::transmute(payload) };
+            self.send_event(target_window, event_mask, &event).queue();
+        }
+
+        Flusher::new(self)
+    }
+}

src/platform/linux/x11/util/format.rs

@@ -0,0 +1,58 @@
+use std::fmt::Debug;
+use std::mem;
+use std::os::raw::*;
+
+// This isn't actually the number of the bits in the format.
+// X11 does a match on this value to determine which type to call sizeof on.
+// Thus, we use 32 for c_long, since 32 maps to c_long which maps to 64.
+// ...if that sounds confusing, then you know why this enum is here.
+#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)]
+pub enum Format {
+    Char = 8,
+    Short = 16,
+    Long = 32,
+}
+
+impl Format {
+    pub fn from_format(format: usize) -> Option<Self> {
+        match format {
+            8 => Some(Format::Char),
+            16 => Some(Format::Short),
+            32 => Some(Format::Long),
+            _ => None,
+        }
+    }
+
+    pub fn is_same_size_as<T>(&self) -> bool {
+        mem::size_of::<T>() == self.get_actual_size()
+    }
+
+    pub fn get_actual_size(&self) -> usize {
+        match self {
+            &Format::Char => mem::size_of::<c_char>(),
+            &Format::Short => mem::size_of::<c_short>(),
+            &Format::Long => mem::size_of::<c_long>(),
+        }
+    }
+
+    pub fn get_payload_size(&self) -> usize {
+        match self {
+            // Due to the wonders of X11, half the space goes unused if you're not using longs (on 64-bit).
+            &Format::Char => mem::size_of::<c_char>() * 20,
+            &Format::Short => mem::size_of::<c_short>() * 10,
+            &Format::Long => mem::size_of::<c_long>() * 5,
+        }
+    }
+}
+
+pub trait Formattable: Debug + Clone + Copy + PartialEq + PartialOrd {
+    const FORMAT: Format;
+}
+
+// You might be surprised by the absence of c_int, but not as surprised as X11 would be by the presence of it.
+impl Formattable for c_char { const FORMAT: Format = Format::Char; }
+impl Formattable for c_uchar { const FORMAT: Format = Format::Char; }
+impl Formattable for c_short { const FORMAT: Format = Format::Short; }
+impl Formattable for c_ushort { const FORMAT: Format = Format::Short; }
+impl Formattable for c_long { const FORMAT: Format = Format::Long; }
+impl Formattable for c_ulong { const FORMAT: Format = Format::Long; }

src/platform/linux/x11/util/geometry.rs

@@ -42,30 +42,6 @@ pub struct TranslatedCoords {
     pub child: ffi::Window,
 }
 
-// This is adequate for get_inner_position
-pub unsafe fn translate_coords(
-    xconn: &Arc<XConnection>,
-    window: ffi::Window,
-    root: ffi::Window,
-) -> Result<TranslatedCoords, XError> {
-    let mut translated_coords: TranslatedCoords = mem::uninitialized();
-
-    (xconn.xlib.XTranslateCoordinates)(
-        xconn.display,
-        window,
-        root,
-        0,
-        0,
-        &mut translated_coords.x_rel_root,
-        &mut translated_coords.y_rel_root,
-        &mut translated_coords.child,
-    );
-
-    //println!("XTranslateCoordinates coords:{:?}", translated_coords);
-
-    xconn.check_errors().map(|_| translated_coords)
-}
-
 #[derive(Debug)]
 pub struct Geometry {
     pub root: ffi::Window,
@@ -86,30 +62,6 @@ pub struct Geometry {
     pub depth: c_uint,
 }
 
-// This is adequate for get_inner_size
-pub unsafe fn get_geometry(
-    xconn: &Arc<XConnection>,
-    window: ffi::Window,
-) -> Result<Geometry, XError> {
-    let mut geometry: Geometry = mem::uninitialized();
-
-    let _status = (xconn.xlib.XGetGeometry)(
-        xconn.display,
-        window,
-        &mut geometry.root,
-        &mut geometry.x_rel_parent,
-        &mut geometry.y_rel_parent,
-        &mut geometry.width,
-        &mut geometry.height,
-        &mut geometry.border,
-        &mut geometry.depth,
-    );
-
-    //println!("XGetGeometry geo:{:?}", geometry);
-
-    xconn.check_errors().map(|_| geometry)
-}
-
 #[derive(Debug, Clone)]
 pub struct FrameExtents {
     pub left: c_ulong,
@@ -128,109 +80,6 @@ impl FrameExtents {
     }
 }
 
-fn get_frame_extents(
-    xconn: &Arc<XConnection>,
-    window: ffi::Window,
-) -> Option<self::FrameExtents> {
-    let extents_atom = unsafe { self::get_atom(xconn, b"_NET_FRAME_EXTENTS\0") }
-        .expect("Failed to call XInternAtom (_NET_FRAME_EXTENTS)");
-
-    if !self::hint_is_supported(extents_atom) {
-        return None;
-    }
-
-    // Of the WMs tested, xmonad, i3, dwm, IceWM (1.3.x and earlier), and blackbox don't
-    // support this. As this is part of EWMH (Extended Window Manager Hints), it's likely to
-    // be unsupported by many smaller WMs.
-    let extents: Option<Vec<c_ulong>> = unsafe {
-        self::get_property(
-            xconn,
-            window,
-            extents_atom,
-            ffi::XA_CARDINAL,
-        )
-    }.ok();
-
-    extents.and_then(|extents| {
-        if extents.len() >= 4 {
-            Some(self::FrameExtents {
-                left: extents[0],
-                right: extents[1],
-                top: extents[2],
-                bottom: extents[3],
-            })
-        } else {
-            None
-        }
-    })
-}
-
-pub fn is_top_level(
-    xconn: &Arc<XConnection>,
-    window: ffi::Window,
-    root: ffi::Window,
-) -> Option<bool> {
-    let client_list_atom = unsafe { self::get_atom(xconn, b"_NET_CLIENT_LIST\0") }
-        .expect("Failed to call XInternAtom (_NET_CLIENT_LIST)");
-
-    if !self::hint_is_supported(client_list_atom) {
-        return None;
-    }
-
-    let client_list: Option<Vec<ffi::Window>> = unsafe {
-        self::get_property(
-            xconn,
-            root,
-            client_list_atom,
-            ffi::XA_WINDOW,
-        )
-    }.ok();
-
-    client_list.map(|client_list| client_list.contains(&window))
-}
-
-unsafe fn get_parent_window(
-    xconn: &Arc<XConnection>,
-    window: ffi::Window,
-) -> Result<ffi::Window, XError> {
-    let mut root: ffi::Window = mem::uninitialized();
-    let mut parent: ffi::Window = mem::uninitialized();
-    let mut children: *mut ffi::Window = ptr::null_mut();
-    let mut nchildren: c_uint = mem::uninitialized();
-
-    let _status = (xconn.xlib.XQueryTree)(
-        xconn.display,
-        window,
-        &mut root,
-        &mut parent,
-        &mut children,
-        &mut nchildren,
-    );
-
-    // The list of children isn't used
-    if children != ptr::null_mut() {
-        (xconn.xlib.XFree)(children as *mut _);
-    }
-
-    xconn.check_errors().map(|_| parent)
-}
-
-fn climb_hierarchy(
-    xconn: &Arc<XConnection>,
-    window: ffi::Window,
-    root: ffi::Window,
-) -> Result<ffi::Window, XError> {
-    let mut outer_window = window;
-    loop {
-        let candidate = unsafe { get_parent_window(xconn, outer_window) }?;
-        if candidate == root {
-            break;
-        }
-        outer_window = candidate;
-    }
-    Ok(outer_window)
-}
-
 #[derive(Debug, Clone, PartialEq)]
 pub enum FrameExtentsHeuristicPath {
     Supported,
@@ -266,11 +115,132 @@ impl FrameExtentsHeuristic {
     }
 }
 
-pub fn get_frame_extents_heuristic(
+impl XConnection {
-    xconn: &Arc<XConnection>,
+    // This is adequate for get_inner_position
-    window: ffi::Window,
+    pub fn translate_coords(&self, window: ffi::Window, root: ffi::Window) -> Result<TranslatedCoords, XError> {
-    root: ffi::Window,
+        let mut translated_coords: TranslatedCoords = unsafe { mem::uninitialized() };
-) -> FrameExtentsHeuristic {
+        unsafe {
+            (self.xlib.XTranslateCoordinates)(
+                self.display,
+                window,
+                root,
+                0,
+                0,
+                &mut translated_coords.x_rel_root,
+                &mut translated_coords.y_rel_root,
+                &mut translated_coords.child,
+            );
+        }
+        //println!("XTranslateCoordinates coords:{:?}", translated_coords);
+        self.check_errors().map(|_| translated_coords)
+    }
+
+    // This is adequate for get_inner_size
+    pub fn get_geometry(&self, window: ffi::Window) -> Result<Geometry, XError> {
+        let mut geometry: Geometry = unsafe { mem::uninitialized() };
+        let _status = unsafe {
+            (self.xlib.XGetGeometry)(
+                self.display,
+                window,
+                &mut geometry.root,
+                &mut geometry.x_rel_parent,
+                &mut geometry.y_rel_parent,
+                &mut geometry.width,
+                &mut geometry.height,
+                &mut geometry.border,
+                &mut geometry.depth,
+            )
+        };
+        //println!("XGetGeometry geo:{:?}", geometry);
+        self.check_errors().map(|_| geometry)
+    }
+
+    fn get_frame_extents(&self, window: ffi::Window) -> Option<FrameExtents> {
+        let extents_atom = unsafe { self.get_atom_unchecked(b"_NET_FRAME_EXTENTS\0") };
+
+        if !hint_is_supported(extents_atom) {
+            return None;
+        }
+
+        // Of the WMs tested, xmonad, i3, dwm, IceWM (1.3.x and earlier), and blackbox don't
+        // support this. As this is part of EWMH (Extended Window Manager Hints), it's likely to
+        // be unsupported by many smaller WMs.
+        let extents: Option<Vec<c_ulong>> = self.get_property(
+            window,
+            extents_atom,
+            ffi::XA_CARDINAL,
+        ).ok();
+
+        extents.and_then(|extents| {
+            if extents.len() >= 4 {
+                Some(FrameExtents {
+                    left: extents[0],
+                    right: extents[1],
+                    top: extents[2],
+                    bottom: extents[3],
+                })
+            } else {
+                None
+            }
+        })
+    }
+
+    pub fn is_top_level(&self, window: ffi::Window, root: ffi::Window) -> Option<bool> {
+        let client_list_atom = unsafe { self.get_atom_unchecked(b"_NET_CLIENT_LIST\0") };
+
+        if !hint_is_supported(client_list_atom) {
+            return None;
+        }
+
+        let client_list: Option<Vec<ffi::Window>> = self.get_property(
+            root,
+            client_list_atom,
+            ffi::XA_WINDOW,
+        ).ok();
+
+        client_list.map(|client_list| client_list.contains(&window))
+    }
+
+    fn get_parent_window(&self, window: ffi::Window) -> Result<ffi::Window, XError> {
+        let parent = unsafe {
+            let mut root: ffi::Window = mem::uninitialized();
+            let mut parent: ffi::Window = mem::uninitialized();
+            let mut children: *mut ffi::Window = ptr::null_mut();
+            let mut nchildren: c_uint = mem::uninitialized();
+
+            // What's filled into `parent` if `window` is the root window?
+            let _status = (self.xlib.XQueryTree)(
+                self.display,
+                window,
+                &mut root,
+                &mut parent,
+                &mut children,
+                &mut nchildren,
+            );
+
+            // The list of children isn't used
+            if children != ptr::null_mut() {
+                (self.xlib.XFree)(children as *mut _);
+            }
+
+            parent
+        };
+        self.check_errors().map(|_| parent)
+    }
+
+    fn climb_hierarchy(&self, window: ffi::Window, root: ffi::Window) -> Result<ffi::Window, XError> {
+        let mut outer_window = window;
+        loop {
+            let candidate = self.get_parent_window(outer_window)?;
+            if candidate == root {
+                break;
+            }
+            outer_window = candidate;
+        }
+        Ok(outer_window)
+    }
+
+    pub fn get_frame_extents_heuristic(&self, window: ffi::Window, root: ffi::Window) -> FrameExtentsHeuristic {
         use self::FrameExtentsHeuristicPath::*;
 
         // Position relative to root window.
@@ -278,8 +248,7 @@ pub fn get_frame_extents_heuristic(
         // isn't nested are outlined in the comments throghout this function, but in addition to
         // that, fullscreen windows often aren't nested.
         let (inner_y_rel_root, child) = {
-        let coords = unsafe { translate_coords(xconn, window, root) }
+            let coords = self.translate_coords(window, root).expect("Failed to translate window coordinates");
-            .expect("Failed to translate window coordinates");
             (
                 coords.y_rel_root,
                 coords.child,
@@ -287,8 +256,7 @@ pub fn get_frame_extents_heuristic(
         };
 
         let (width, height, border) = {
-        let inner_geometry = unsafe { get_geometry(xconn, window) }
+            let inner_geometry = self.get_geometry(window).expect("Failed to get inner window geometry");
-            .expect("Failed to get inner window geometry");
             (
                 inner_geometry.width,
                 inner_geometry.height,
@@ -301,10 +269,10 @@ pub fn get_frame_extents_heuristic(
         // when y is on the range [0, 2] and if the window has been unfocused since being
         // undecorated (or was undecorated upon construction), the first condition is true,
         // requiring us to rely on the second condition.
-    let nested = !(window == child || is_top_level(xconn, child, root) == Some(true));
+        let nested = !(window == child || self.is_top_level(child, root) == Some(true));
 
         // Hopefully the WM supports EWMH, allowing us to get exact info on the window frames.
-    if let Some(mut frame_extents) = get_frame_extents(xconn, window) {
+        if let Some(mut frame_extents) = self.get_frame_extents(window) {
             // Mutter/Muffin/Budgie and Marco preserve their decorated frame extents when
             // decorations are disabled, but since the window becomes un-nested, it's easy to
             // catch.
@@ -339,12 +307,9 @@ pub fn get_frame_extents_heuristic(
             // If the position value we have is for a nested window used as the client area, we'll
             // just climb up the hierarchy and get the geometry of the outermost window we're
             // nested in.
-        let outer_window = climb_hierarchy(xconn, window, root)
+            let outer_window = self.climb_hierarchy(window, root).expect("Failed to climb window hierarchy");
-            .expect("Failed to climb window hierarchy");
-
             let (outer_y, outer_width, outer_height) = {
-            let outer_geometry = unsafe { get_geometry(xconn, outer_window) }
+                let outer_geometry = self.get_geometry(outer_window).expect("Failed to get outer window geometry");
-                .expect("Failed to get outer window geometry");
                 (
                     outer_geometry.y_rel_parent,
                     outer_geometry.width,
@@ -383,3 +348,4 @@ pub fn get_frame_extents_heuristic(
             }
         }
     }
+}

src/platform/linux/x11/util/hint.rs

@@ -1,3 +1,5 @@
+use std::sync::Arc;
+
 use super::*;
 
 pub const MWM_HINTS_DECORATIONS: c_ulong = 2;
@@ -6,12 +8,12 @@ pub const MWM_HINTS_DECORATIONS: c_ulong = 2;
 pub enum StateOperation {
     Remove = 0, // _NET_WM_STATE_REMOVE
     Add = 1,    // _NET_WM_STATE_ADD
-    _Toggle = 2, // _NET_WM_STATE_TOGGLE
+    Toggle = 2, // _NET_WM_STATE_TOGGLE
 }
 
 impl From<bool> for StateOperation {
-    fn from(b: bool) -> Self {
+    fn from(op: bool) -> Self {
-        if b {
+        if op {
             StateOperation::Add
         } else {
             StateOperation::Remove
@@ -62,7 +64,30 @@ impl WindowType {
             &Dialog => b"_NET_WM_WINDOW_TYPE_DIALOG\0",
             &Normal => b"_NET_WM_WINDOW_TYPE_NORMAL\0",
         };
-        unsafe { get_atom(xconn, atom_name) }
+        unsafe { xconn.get_atom_unchecked(atom_name) }
-            .expect("Failed to get atom for `WindowType`")
+    }
+}
+
+impl XConnection {
+    pub fn get_wm_hints(&self, window: ffi::Window) -> Result<XSmartPointer<ffi::XWMHints>, XError> {
+        let wm_hints = unsafe { (self.xlib.XGetWMHints)(self.display, window) };
+        self.check_errors()?;
+        let wm_hints = if wm_hints.is_null() {
+            self.alloc_wm_hints()
+        } else {
+            XSmartPointer::new(self, wm_hints).unwrap()
+        };
+        Ok(wm_hints)
+    }
+
+    pub fn set_wm_hints(&self, window: ffi::Window, wm_hints: XSmartPointer<ffi::XWMHints>) -> Flusher {
+        unsafe {
+            (self.xlib.XSetWMHints)(
+                self.display,
+                window,
+                wm_hints.ptr,
+            );
+        }
+        Flusher::new(self)
     }
 }

src/platform/linux/x11/util/input.rs

@@ -1,44 +1,12 @@
+use std::str;
+
 use super::*;
 use events::ModifiersState;
 
-pub unsafe fn select_xinput_events(
+// A base buffer size of 1kB uses a negligible amount of RAM while preventing us from having to
-    xconn: &Arc<XConnection>,
+// re-allocate (and make another round-trip) in the *vast* majority of cases.
-    window: c_ulong,
+// To test if `lookup_utf8` works correctly, set this to 1.
-    device_id: c_int,
+const TEXT_BUFFER_SIZE: usize = 1024;
-    mask: i32,
-) -> Flusher {
-    let mut event_mask = ffi::XIEventMask {
-        deviceid: device_id,
-        mask: &mask as *const _ as *mut c_uchar,
-        mask_len: mem::size_of_val(&mask) as c_int,
-    };
-    (xconn.xinput2.XISelectEvents)(
-        xconn.display,
-        window,
-        &mut event_mask as *mut ffi::XIEventMask,
-        1, // number of masks to read from pointer above
-    );
-    Flusher::new(xconn)
-}
-
-#[allow(dead_code)]
-pub unsafe fn select_xkb_events(
-    xconn: &Arc<XConnection>,
-    device_id: c_uint,
-    mask: c_ulong,
-) -> Option<Flusher> {
-    let status = (xconn.xlib.XkbSelectEvents)(
-        xconn.display,
-        device_id,
-        mask,
-        mask,
-    );
-    if status == ffi::True {
-        Some(Flusher::new(xconn))
-    } else {
-        None
-    }
-}
 
 impl From<ffi::XIModifierState> for ModifiersState {
     fn from(mods: ffi::XIModifierState) -> Self {
@@ -53,17 +21,17 @@ impl From<ffi::XIModifierState> for ModifiersState {
 }
 
 pub struct PointerState<'a> {
-    xconn: &'a Arc<XConnection>,
+    xconn: &'a XConnection,
-    _root: ffi::Window,
+    root: ffi::Window,
-    _child: ffi::Window,
+    child: ffi::Window,
-    _root_x: c_double,
+    root_x: c_double,
-    _root_y: c_double,
+    root_y: c_double,
-    _win_x: c_double,
+    win_x: c_double,
-    _win_y: c_double,
+    win_y: c_double,
-    _buttons: ffi::XIButtonState,
+    buttons: ffi::XIButtonState,
     modifiers: ffi::XIModifierState,
-    _group: ffi::XIGroupState,
+    group: ffi::XIGroupState,
-    _relative_to_window: bool,
+    relative_to_window: bool,
 }
 
 impl<'a> PointerState<'a> {
@@ -74,107 +42,107 @@ impl<'a> PointerState<'a> {
 
 impl<'a> Drop for PointerState<'a> {
     fn drop(&mut self) {
+        if !self.buttons.mask.is_null() {
             unsafe {
                 // This is why you need to read the docs carefully...
-            (self.xconn.xlib.XFree)(self._buttons.mask as _);
+                (self.xconn.xlib.XFree)(self.buttons.mask as _);
+            }
         }
     }
 }
 
-pub unsafe fn query_pointer(
+impl XConnection {
-    xconn: &Arc<XConnection>,
+    pub fn select_xinput_events(&self, window: c_ulong, device_id: c_int, mask: i32) -> Flusher {
-    window: ffi::Window,
+        let mut event_mask = ffi::XIEventMask {
-    device_id: c_int,
+            deviceid: device_id,
-) -> Result<PointerState, XError> {
+            mask: &mask as *const _ as *mut c_uchar,
-    let mut root_return = mem::uninitialized();
+            mask_len: mem::size_of_val(&mask) as c_int,
-    let mut child_return = mem::uninitialized();
+        };
-    let mut root_x_return = mem::uninitialized();
+        unsafe {
-    let mut root_y_return = mem::uninitialized();
+            (self.xinput2.XISelectEvents)(
-    let mut win_x_return = mem::uninitialized();
+                self.display,
-    let mut win_y_return = mem::uninitialized();
+                window,
-    let mut buttons_return = mem::uninitialized();
+                &mut event_mask as *mut ffi::XIEventMask,
-    let mut modifiers_return = mem::uninitialized();
+                1, // number of masks to read from pointer above
-    let mut group_return = mem::uninitialized();
+            );
+        }
+        Flusher::new(self)
+    }
 
-    let relative_to_window = (xconn.xinput2.XIQueryPointer)(
+    #[allow(dead_code)]
-        xconn.display,
+    pub fn select_xkb_events(&self, device_id: c_uint, mask: c_ulong) -> Option<Flusher> {
+        let status = unsafe {
+            (self.xlib.XkbSelectEvents)(
+                self.display,
+                device_id,
+                mask,
+                mask,
+            )
+        };
+        if status == ffi::True {
+            Some(Flusher::new(self))
+        } else {
+            None
+        }
+    }
+
+    pub fn query_pointer(&self, window: ffi::Window, device_id: c_int) -> Result<PointerState, XError> {
+        unsafe {
+            let mut pointer_state: PointerState = mem::uninitialized();
+            pointer_state.xconn = self;
+            pointer_state.relative_to_window = (self.xinput2.XIQueryPointer)(
+                self.display,
                 device_id,
                 window,
-        &mut root_return,
+                &mut pointer_state.root,
-        &mut child_return,
+                &mut pointer_state.child,
-        &mut root_x_return,
+                &mut pointer_state.root_x,
-        &mut root_y_return,
+                &mut pointer_state.root_y,
-        &mut win_x_return,
+                &mut pointer_state.win_x,
-        &mut win_y_return,
+                &mut pointer_state.win_y,
-        &mut buttons_return,
+                &mut pointer_state.buttons,
-        &mut modifiers_return,
+                &mut pointer_state.modifiers,
-        &mut group_return,
+                &mut pointer_state.group,
             ) == ffi::True;
-
+            if let Err(err) = self.check_errors() {
-    xconn.check_errors()?;
+                // Running the destrutor would be bad news for us...
-
+                mem::forget(pointer_state);
-    Ok(PointerState {
+                Err(err)
-        xconn,
+            } else {
-        _root: root_return,
+                Ok(pointer_state)
-        _child: child_return,
+            }
-        _root_x: root_x_return,
+        }
-        _root_y: root_y_return,
-        _win_x: win_x_return,
-        _win_y: win_y_return,
-        _buttons: buttons_return,
-        modifiers: modifiers_return,
-        _group: group_return,
-        _relative_to_window: relative_to_window,
-    })
     }
 
-unsafe fn lookup_utf8_inner(
+    fn lookup_utf8_inner(
-    xconn: &Arc<XConnection>,
+        &self,
         ic: ffi::XIC,
         key_event: &mut ffi::XKeyEvent,
         buffer: &mut [u8],
     ) -> (ffi::KeySym, ffi::Status, c_int) {
         let mut keysym: ffi::KeySym = 0;
         let mut status: ffi::Status = 0;
-    let count = (xconn.xlib.Xutf8LookupString)(
+        let count = unsafe {
+            (self.xlib.Xutf8LookupString)(
                 ic,
                 key_event,
                 buffer.as_mut_ptr() as *mut c_char,
                 buffer.len() as c_int,
                 &mut keysym,
                 &mut status,
-    );
+            )
+        };
         (keysym, status, count)
     }
 
-// A base buffer size of 1kB uses a negligible amount of RAM while preventing us from having to
+    pub fn lookup_utf8(&self, ic: ffi::XIC, key_event: &mut ffi::XKeyEvent) -> String {
-// re-allocate (and make another round-trip) in the *vast* majority of cases.
+        let mut buffer: [u8; TEXT_BUFFER_SIZE] = unsafe { mem::uninitialized() };
-// To test if lookup_utf8 works correctly, set this to 1.
+        let (_, status, count) = self.lookup_utf8_inner(ic, key_event, &mut buffer);
-const TEXT_BUFFER_SIZE: usize = 1024;
-
-pub unsafe fn lookup_utf8(
-    xconn: &Arc<XConnection>,
-    ic: ffi::XIC,
-    key_event: &mut ffi::XKeyEvent,
-) -> String {
-    let mut buffer: [u8; TEXT_BUFFER_SIZE] = mem::uninitialized();
-    let (_, status, count) = lookup_utf8_inner(
-        xconn,
-        ic,
-        key_event,
-        &mut buffer,
-    );
-
         // The buffer overflowed, so we'll make a new one on the heap.
         if status == ffi::XBufferOverflow {
             let mut buffer = Vec::with_capacity(count as usize);
-        buffer.set_len(count as usize);
+            unsafe { buffer.set_len(count as usize) };
-        let (_, _, new_count) = lookup_utf8_inner(
+            let (_, _, new_count) = self.lookup_utf8_inner(ic, key_event, &mut buffer);
-            xconn,
-            ic,
-            key_event,
-            &mut buffer,
-        );
             debug_assert_eq!(count, new_count);
             str::from_utf8(&buffer[..count as usize])
                 .unwrap_or("")
@@ -185,3 +153,4 @@ pub unsafe fn lookup_utf8(
                 .to_string()
         }
     }
+}

src/platform/linux/x11/util/memory.rs

@@ -0,0 +1,62 @@
+use std::ops::{Deref, DerefMut};
+
+use super::*;
+
+pub struct XSmartPointer<'a, T> {
+    xconn: &'a XConnection,
+    pub ptr: *mut T,
+}
+
+impl<'a, T> XSmartPointer<'a, T> {
+    // You're responsible for only passing things to this that should be XFree'd.
+    // Returns None if ptr is null.
+    pub fn new(xconn: &'a XConnection, ptr: *mut T) -> Option<Self> {
+        if !ptr.is_null() {
+            Some(XSmartPointer {
+                xconn,
+                ptr,
+            })
+        } else {
+            None
+        }
+    }
+}
+
+impl<'a, T> Deref for XSmartPointer<'a, T> {
+    type Target = T;
+
+    fn deref(&self) -> &T {
+        unsafe { &*self.ptr }
+    }
+}
+
+impl<'a, T> DerefMut for XSmartPointer<'a, T> {
+    fn deref_mut(&mut self) -> &mut T {
+        unsafe { &mut *self.ptr }
+    }
+}
+
+impl<'a, T> Drop for XSmartPointer<'a, T> {
+    fn drop(&mut self) {
+        unsafe {
+            (self.xconn.xlib.XFree)(self.ptr as *mut _);
+        }
+    }
+}
+
+impl XConnection {
+    pub fn alloc_class_hint(&self) -> XSmartPointer<ffi::XClassHint> {
+        XSmartPointer::new(self, unsafe { (self.xlib.XAllocClassHint)() })
+            .expect("`XAllocClassHint` returned null; out of memory")
+    }
+
+    pub fn alloc_size_hints(&self) -> XSmartPointer<ffi::XSizeHints> {
+        XSmartPointer::new(self, unsafe { (self.xlib.XAllocSizeHints)() })
+            .expect("`XAllocSizeHints` returned null; out of memory")
+    }
+
+    pub fn alloc_wm_hints(&self) -> XSmartPointer<ffi::XWMHints> {
+        XSmartPointer::new(self, unsafe { (self.xlib.XAllocWMHints)() })
+            .expect("`XAllocWMHints` returned null; out of memory")
+    }
+}

src/platform/linux/x11/util/mod.rs

@@ -2,180 +2,80 @@
 // *results may vary
 
 mod atom;
+mod client_msg;
+mod format;
 mod geometry;
 mod hint;
 mod icon;
 mod input;
+mod memory;
 mod randr;
 mod window_property;
 mod wm;
 
 pub use self::atom::*;
+pub use self::client_msg::*;
+pub use self::format::*;
 pub use self::geometry::*;
 pub use self::hint::*;
 pub use self::icon::*;
 pub use self::input::*;
+pub use self::memory::*;
 pub use self::randr::*;
 pub use self::window_property::*;
 pub use self::wm::*;
 
 use std::mem;
 use std::ptr;
-use std::str;
-use std::sync::Arc;
-use std::ops::{Deref, DerefMut};
 use std::os::raw::*;
 
 use super::{ffi, XConnection, XError};
 
-// This isn't actually the number of the bits in the format.
-// X11 does a match on this value to determine which type to call sizeof on.
-// Thus, we use 32 for c_long, since 32 maps to c_long which maps to 64.
-// ...if that sounds confusing, then you know why this enum is here.
-#[derive(Debug, Copy, Clone)]
-pub enum Format {
-    Char = 8,
-    Short = 16,
-    Long = 32,
-}
-
-impl Format {
-    pub fn from_format(format: usize) -> Option<Self> {
-        match format {
-            8 => Some(Format::Char),
-            16 => Some(Format::Short),
-            32 => Some(Format::Long),
-            _ => None,
-        }
-    }
-
-    pub fn is_same_size_as<T>(&self) -> bool {
-        mem::size_of::<T>() == self.get_actual_size()
-    }
-
-    pub fn get_actual_size(&self) -> usize {
-        match self {
-            &Format::Char => mem::size_of::<c_char>(),
-            &Format::Short => mem::size_of::<c_short>(),
-            &Format::Long => mem::size_of::<c_long>(),
-        }
-    }
-}
-
-pub struct XSmartPointer<'a, T> {
-    xconn: &'a Arc<XConnection>,
-    pub ptr: *mut T,
-}
-
-impl<'a, T> XSmartPointer<'a, T> {
-    // You're responsible for only passing things to this that should be XFree'd.
-    // Returns None if ptr is null.
-    pub fn new(xconn: &'a Arc<XConnection>, ptr: *mut T) -> Option<Self> {
-        if !ptr.is_null() {
-            Some(XSmartPointer {
-                xconn,
-                ptr,
-            })
-        } else {
-            None
-        }
-    }
-}
-
-impl<'a, T> Deref for XSmartPointer<'a, T> {
-    type Target = T;
-
-    fn deref(&self) -> &T {
-        unsafe { &*self.ptr }
-    }
-}
-
-impl<'a, T> DerefMut for XSmartPointer<'a, T> {
-    fn deref_mut(&mut self) -> &mut T {
-        unsafe { &mut *self.ptr }
-    }
-}
-
-impl<'a, T> Drop for XSmartPointer<'a, T> {
-    fn drop(&mut self) {
-        unsafe {
-            (self.xconn.xlib.XFree)(self.ptr as *mut _);
-        }
-    }
-}
-
-// This is impoartant, so pay attention!
-// Xlib has an output buffer, and tries to hide the async nature of X from you.
-// This buffer contains the requests you make, and is flushed under various circumstances:
-// 1. XPending, XNextEvent, and XWindowEvent flush "as needed"
-// 2. XFlush explicitly flushes
-// 3. XSync flushes and blocks until all requests are responded to
-// 4. Calls that have a return dependent on a response (i.e. XGetWindowProperty) sync internally.
-//    When in doubt, check the X11 source; if a function calls _XReply, it flushes and waits.
-// All util functions that abstract an async function will return a Flusher.
-pub unsafe fn flush_requests(xconn: &Arc<XConnection>) -> Result<(), XError> {
-    (xconn.xlib.XFlush)(xconn.display);
-    //println!("XFlush");
-    // This isn't necessarily a useful time to check for errors (since our request hasn't
-    // necessarily been processed yet)
-    xconn.check_errors()
-}
-
-pub unsafe fn sync_with_server(xconn: &Arc<XConnection>) -> Result<(), XError> {
-    (xconn.xlib.XSync)(xconn.display, ffi::False);
-    //println!("XSync");
-    xconn.check_errors()
-}
-
 #[must_use = "This request was made asynchronously, and is still in the output buffer. You must explicitly choose to either `.flush()` (empty the output buffer, sending the request now) or `.queue()` (wait to send the request, allowing you to continue to add more requests without additional round-trips). For more information, see the documentation for `util::flush_requests`."]
 pub struct Flusher<'a> {
-    xconn: &'a Arc<XConnection>,
+    xconn: &'a XConnection,
 }
 
 impl<'a> Flusher<'a> {
-    pub fn new(xconn: &'a Arc<XConnection>) -> Self {
+    pub fn new(xconn: &'a XConnection) -> Self {
         Flusher { xconn }
     }
 
     // "I want this request sent now!"
     pub fn flush(self) -> Result<(), XError> {
-        unsafe { flush_requests(self.xconn) }
+        self.xconn.flush_requests()
+    }
+
+    // "I want the response now too!"
+    pub fn sync(self) -> Result<(), XError> {
+        self.xconn.sync_with_server()
     }
 
     // "I'm aware that this request hasn't been sent, and I'm okay with waiting."
     pub fn queue(self) {}
 }
 
-pub unsafe fn send_client_msg(
+impl XConnection {
-    xconn: &Arc<XConnection>,
+    // This is impoartant, so pay attention!
-    window: c_ulong,        // The window this is "about"; not necessarily this window
+    // Xlib has an output buffer, and tries to hide the async nature of X from you.
-    target_window: c_ulong, // The window we're sending to
+    // This buffer contains the requests you make, and is flushed under various circumstances:
-    message_type: ffi::Atom,
+    // 1. `XPending`, `XNextEvent`, and `XWindowEvent` flush "as needed"
-    event_mask: Option<c_long>,
+    // 2. `XFlush` explicitly flushes
-    data: (c_long, c_long, c_long, c_long, c_long),
+    // 3. `XSync` flushes and blocks until all requests are responded to
-) -> Flusher {
+    // 4. Calls that have a return dependent on a response (i.e. `XGetWindowProperty`) sync internally.
-    let mut event: ffi::XClientMessageEvent = mem::uninitialized();
+    //    When in doubt, check the X11 source; if a function calls `_XReply`, it flushes and waits.
-    event.type_ = ffi::ClientMessage;
+    // All util functions that abstract an async function will return a `Flusher`.
-    event.display = xconn.display;
+    pub fn flush_requests(&self) -> Result<(), XError> {
-    event.window = window;
+        unsafe { (self.xlib.XFlush)(self.display) };
-    event.message_type = message_type;
+        //println!("XFlush");
-    event.format = Format::Long as c_int;
+        // This isn't necessarily a useful time to check for errors (since our request hasn't
-    event.data = ffi::ClientMessageData::new();
+        // necessarily been processed yet)
-    event.data.set_long(0, data.0);
+        self.check_errors()
-    event.data.set_long(1, data.1);
+    }
-    event.data.set_long(2, data.2);
+
-    event.data.set_long(3, data.3);
+    pub fn sync_with_server(&self) -> Result<(), XError> {
-    event.data.set_long(4, data.4);
+        unsafe { (self.xlib.XSync)(self.display, ffi::False) };
-
+        //println!("XSync");
-    let event_mask = event_mask.unwrap_or(ffi::NoEventMask);
+        self.check_errors()
-
+    }
-    (xconn.xlib.XSendEvent)(
-        xconn.display,
-        target_window,
-        ffi::False,
-        event_mask,
-        &mut event.into(),
-    );
-
-    Flusher::new(xconn)
 }

src/platform/linux/x11/util/randr.rs

@@ -60,13 +60,10 @@ pub fn calc_dpi_factor(
     ((ppmm * (12.0 * 25.4 / 96.0)).round() / 12.0).max(1.0)
 }
 
-pub unsafe fn get_output_info(
+impl XConnection {
-    xconn: &Arc<XConnection>,
+    pub unsafe fn get_output_info(&self, resources: *mut XRRScreenResources, repr: &MonitorRepr) -> (String, f32) {
-    resources: *mut XRRScreenResources,
+        let output_info = (self.xrandr.XRRGetOutputInfo)(
-    repr: &MonitorRepr,
+            self.display,
-) -> (String, f32) {
-    let output_info = (xconn.xrandr.XRRGetOutputInfo)(
-        xconn.display,
             resources,
             repr.get_output(),
         );
@@ -79,6 +76,7 @@ pub unsafe fn get_output_info(
             repr.get_dimensions(),
             ((*output_info).mm_width as u64, (*output_info).mm_height as u64),
         ) as f32;
-    (xconn.xrandr.XRRFreeOutputInfo)(output_info);
+        (self.xrandr.XRRFreeOutputInfo)(output_info);
         (name, hidpi_factor)
     }
+}

src/platform/linux/x11/util/window_property.rs

@@ -1,5 +1,4 @@
 use std;
-use std::fmt::Debug;
 
 use super::*;
 
@@ -25,11 +24,19 @@ impl GetPropertyError {
 }
 
 // Number of 32-bit chunks to retrieve per iteration of get_property's inner loop.
-// To test if get_property works correctly, set this to 1.
+// To test if `get_property` works correctly, set this to 1.
 const PROPERTY_BUFFER_SIZE: c_long = 1024; // 4k of RAM ought to be enough for anyone!
 
-pub unsafe fn get_property<T: Debug + Clone>(
+#[derive(Debug)]
-    xconn: &Arc<XConnection>,
+pub enum PropMode {
+    Replace = ffi::PropModeReplace as isize,
+    Prepend = ffi::PropModePrepend as isize,
+    Append = ffi::PropModeAppend as isize,
+}
+
+impl XConnection {
+    pub fn get_property<T: Formattable>(
+        &self,
         window: c_ulong,
         property: ffi::Atom,
         property_type: ffi::Atom,
@@ -39,13 +46,14 @@ pub unsafe fn get_property<T: Debug + Clone>(
 
         let mut done = false;
         while !done {
+            unsafe {
                 let mut actual_type: ffi::Atom = mem::uninitialized();
                 let mut actual_format: c_int = mem::uninitialized();
                 let mut quantity_returned: c_ulong = mem::uninitialized();
                 let mut bytes_after: c_ulong = mem::uninitialized();
                 let mut buf: *mut c_uchar = ptr::null_mut();
-        (xconn.xlib.XGetWindowProperty)(
+                (self.xlib.XGetWindowProperty)(
-            xconn.display,
+                    self.display,
                     window,
                     property,
                     // This offset is in terms of 32-bit chunks.
@@ -63,7 +71,7 @@ pub unsafe fn get_property<T: Debug + Clone>(
                     &mut buf,
                 );
 
-        if let Err(e) = xconn.check_errors() {
+                if let Err(e) = self.check_errors() {
                     return Err(GetPropertyError::XError(e));
                 }
 
@@ -71,11 +79,7 @@ pub unsafe fn get_property<T: Debug + Clone>(
                     return Err(GetPropertyError::TypeMismatch(actual_type));
                 }
 
-        let format_mismatch = Format::from_format(actual_format as _)
+                let format_mismatch = Format::from_format(actual_format as _) != Some(T::FORMAT);
-            .map(|actual_format| !actual_format.is_same_size_as::<T>())
-            // This won't actually be reached; the XError condition above is triggered first.
-            .unwrap_or(true);
-
                 if format_mismatch {
                     return Err(GetPropertyError::FormatMismatch(actual_format));
                 }
@@ -97,67 +101,44 @@ pub unsafe fn get_property<T: Debug + Clone>(
                     );*/
                     data.extend_from_slice(&new_data);
                     // Fun fact: XGetWindowProperty allocates one extra byte at the end.
-            (xconn.xlib.XFree)(buf as _); // Don't try to access new_data after this.
+                    (self.xlib.XFree)(buf as _); // Don't try to access new_data after this.
                 } else {
                     return Err(GetPropertyError::NothingAllocated);
                 }
 
                 done = bytes_after == 0;
             }
+        }
 
         Ok(data)
     }
 
-#[derive(Debug)]
+    pub fn change_property<'a, T: Formattable>(
-pub enum PropMode {
+        &'a self,
-    Replace = ffi::PropModeReplace as isize,
-    _Prepend = ffi::PropModePrepend as isize,
-    _Append = ffi::PropModeAppend as isize,
-}
-
-#[derive(Debug, Clone)]
-pub struct InvalidFormat {
-    format_used: Format,
-    size_passed: usize,
-    size_expected: usize,
-}
-
-pub unsafe fn change_property<'a, T: Debug>(
-    xconn: &'a Arc<XConnection>,
         window: c_ulong,
         property: ffi::Atom,
         property_type: ffi::Atom,
-    format: Format,
         mode: PropMode,
         new_value: &[T],
     ) -> Flusher<'a> {
-    if !format.is_same_size_as::<T>() {
+        debug_assert_eq!(mem::size_of::<T>(), T::FORMAT.get_actual_size());
-        panic!(format!(
+        unsafe {
-            "[winit developer error] Incorrect usage of `util::change_property`: {:#?}",
+            (self.xlib.XChangeProperty)(
-            InvalidFormat {
+                self.display,
-                format_used: format,
-                size_passed: mem::size_of::<T>() * 8,
-                size_expected: format.get_actual_size() * 8,
-            },
-        ));
-    }
-
-    (xconn.xlib.XChangeProperty)(
-        xconn.display,
                 window,
                 property,
                 property_type,
-        format as c_int,
+                T::FORMAT as c_int,
                 mode as c_int,
                 new_value.as_ptr() as *const c_uchar,
                 new_value.len() as c_int,
             );
-
+        }
         /*println!(
             "XChangeProperty prop:{:?} val:{:?}",
             property,
             new_value,
         );*/
-
+        Flusher::new(self)
-    Flusher::new(xconn)
+    }
 }

src/platform/linux/x11/util/wm.rs

@@ -20,29 +20,24 @@ pub fn wm_name_is_one_of(names: &[&str]) -> bool {
     }
 }
 
-pub fn update_cached_wm_info(xconn: &Arc<XConnection>, root: ffi::Window) {
+impl XConnection {
-    *SUPPORTED_HINTS.lock() = self::get_supported_hints(xconn, root);
+    pub fn update_cached_wm_info(&self, root: ffi::Window) {
-    *WM_NAME.lock() = self::get_wm_name(xconn, root);
+        *SUPPORTED_HINTS.lock() = self.get_supported_hints(root);
+        *WM_NAME.lock() = self.get_wm_name(root);
     }
 
-fn get_supported_hints(xconn: &Arc<XConnection>, root: ffi::Window) -> Vec<ffi::Atom> {
+    fn get_supported_hints(&self, root: ffi::Window) -> Vec<ffi::Atom> {
-    let supported_atom = unsafe { self::get_atom(xconn, b"_NET_SUPPORTED\0") }
+        let supported_atom = unsafe { self.get_atom_unchecked(b"_NET_SUPPORTED\0") };
-        .expect("Failed to call XInternAtom (_NET_SUPPORTED)");
+        self.get_property(
-    unsafe {
-        self::get_property(
-            xconn,
             root,
             supported_atom,
             ffi::XA_ATOM,
-        )
+        ).unwrap_or_else(|_| Vec::with_capacity(0))
-    }.unwrap_or_else(|_| Vec::with_capacity(0))
     }
 
-fn get_wm_name(xconn: &Arc<XConnection>, root: ffi::Window) -> Option<String> {
+    fn get_wm_name(&self, root: ffi::Window) -> Option<String> {
-    let check_atom = unsafe { self::get_atom(xconn, b"_NET_SUPPORTING_WM_CHECK\0") }
+        let check_atom = unsafe { self.get_atom_unchecked(b"_NET_SUPPORTING_WM_CHECK\0") };
-        .expect("Failed to call XInternAtom (_NET_SUPPORTING_WM_CHECK)");
+        let wm_name_atom = unsafe { self.get_atom_unchecked(b"_NET_WM_NAME\0") };
-    let wm_name_atom = unsafe { self::get_atom(xconn, b"_NET_WM_NAME\0") }
-        .expect("Failed to call XInternAtom (_NET_WM_NAME)");
 
         // Mutter/Muffin/Budgie doesn't have _NET_SUPPORTING_WM_CHECK in its _NET_SUPPORTED, despite
         // it working and being supported. This has been reported upstream, but due to the
@@ -66,14 +61,11 @@ fn get_wm_name(xconn: &Arc<XConnection>, root: ffi::Window) -> Option<String> {
         // Querying this property on the root window will give us the ID of a child window created by
         // the WM.
         let root_window_wm_check = {
-        let result = unsafe {
+            let result = self.get_property(
-            self::get_property(
-                xconn,
                 root,
                 check_atom,
                 ffi::XA_WINDOW,
-            )
+            );
-        };
 
             let wm_check = result
                 .ok()
@@ -89,14 +81,11 @@ fn get_wm_name(xconn: &Arc<XConnection>, root: ffi::Window) -> Option<String> {
         // Querying the same property on the child window we were given, we should get this child
         // window's ID again.
         let child_window_wm_check = {
-        let result = unsafe {
+            let result = self.get_property(
-            self::get_property(
-                xconn,
                 root_window_wm_check,
                 check_atom,
                 ffi::XA_WINDOW,
-            )
+            );
-        };
 
             let wm_check = result
                 .ok()
@@ -116,17 +105,13 @@ fn get_wm_name(xconn: &Arc<XConnection>, root: ffi::Window) -> Option<String> {
 
         // All of that work gives us a window ID that we can get the WM name from.
         let wm_name = {
-        let utf8_string_atom = unsafe { self::get_atom(xconn, b"UTF8_STRING\0") }
+            let utf8_string_atom = unsafe { self.get_atom_unchecked(b"UTF8_STRING\0") };
-            .expect("Failed to call XInternAtom (UTF8_STRING)");
 
-        let result = unsafe {
+            let result = self.get_property(
-            self::get_property(
-                xconn,
                 root_window_wm_check,
                 wm_name_atom,
                 utf8_string_atom,
-            )
+            );
-        };
 
             // IceWM requires this. IceWM was also the only WM tested that returns a null-terminated
             // string. For more fun trivia, IceWM is also unique in including version and uname
@@ -141,14 +126,11 @@ fn get_wm_name(xconn: &Arc<XConnection>, root: ffi::Window) -> Option<String> {
             };
 
             if no_utf8 {
-            unsafe {
+                self.get_property(
-                self::get_property(
-                    xconn,
                     root_window_wm_check,
                     wm_name_atom,
                     ffi::XA_STRING,
                 )
-            }
             } else {
                 result
             }
@@ -156,3 +138,4 @@ fn get_wm_name(xconn: &Arc<XConnection>, root: ffi::Window) -> Option<String> {
 
         wm_name.and_then(|wm_name| String::from_utf8(wm_name).ok())
     }
+}

src/platform/linux/x11/window.rs

@@ -169,15 +169,12 @@ impl Window2 {
 
             // Enable drag and drop (TODO: extend API to make this toggleable)
             unsafe {
-                let dnd_aware_atom = util::get_atom(xconn, b"XdndAware\0")
+                let dnd_aware_atom = xconn.get_atom_unchecked(b"XdndAware\0");
-                    .expect("Failed to call XInternAtom (XdndAware)");
                 let version = &[5 as c_ulong]; // Latest version; hasn't changed since 2002
-                util::change_property(
+                xconn.change_property(
-                    xconn,
                     x_window.window,
                     dnd_aware_atom,
                     ffi::XA_ATOM,
-                    util::Format::Long,
                     util::PropMode::Replace,
                     version,
                 )
@@ -185,11 +182,6 @@ impl Window2 {
 
             // WM_CLASS must be set *before* mapping the window, as per ICCCM!
             {
-                let mut class_hints = {
-                    let class_hints = unsafe { (xconn.xlib.XAllocClassHint)() };
-                    util::XSmartPointer::new(xconn, class_hints)
-                }.expect("`XAllocClassHint` returned null; out of memory");
-
                 let (class, instance) = if let Some((instance, class)) = pl_attribs.class {
                     let instance = CString::new(instance.as_str())
                         .expect("`WM_CLASS` instance contained null byte");
@@ -216,14 +208,15 @@ impl Window2 {
                     (instance, class)
                 };
 
-                (*class_hints).res_name = class.as_ptr() as *mut c_char;
+                let mut class_hint = xconn.alloc_class_hint();
-                (*class_hints).res_class = instance.as_ptr() as *mut c_char;
+                (*class_hint).res_name = class.as_ptr() as *mut c_char;
+                (*class_hint).res_class = instance.as_ptr() as *mut c_char;
 
                 unsafe {
                     (xconn.xlib.XSetClassHint)(
                         xconn.display,
                         x_window.window,
-                        class_hints.ptr,
+                        class_hint.ptr,
                     );
                 }//.queue();
             }
@@ -241,10 +234,7 @@ impl Window2 {
 
             // set size hints
             {
-                let mut size_hints = {
+                let mut size_hints = xconn.alloc_size_hints();
-                    let size_hints = unsafe { (xconn.xlib.XAllocSizeHints)() };
-                    util::XSmartPointer::new(xconn, size_hints)
-                }.expect("XAllocSizeHints returned null; out of memory");
                 (*size_hints).flags = ffi::PSize;
                 (*size_hints).width = dimensions.0 as c_int;
                 (*size_hints).height = dimensions.1 as c_int;
@@ -330,14 +320,7 @@ impl Window2 {
                 }
                 mask
             };
-            unsafe {
+            xconn.select_xinput_events(x_window.window, ffi::XIAllMasterDevices, mask).queue();
-                util::select_xinput_events(
-                    xconn,
-                    x_window.window,
-                    ffi::XIAllMasterDevices,
-                    mask,
-                )
-            }.queue();
 
             // These properties must be set after mapping
             if window_attrs.maximized {
@@ -372,7 +355,7 @@ impl Window2 {
         }
 
         // We never want to give the user a broken window, since by then, it's too late to handle.
-        unsafe { util::sync_with_server(xconn) }
+        xconn.sync_with_server()
             .map(|_| window)
             .map_err(|x_err| OsError(
                 format!("X server returned error while building window: {:?}", x_err)
@@ -380,10 +363,8 @@ impl Window2 {
     }
 
     fn set_pid(xconn: &Arc<XConnection>, window: ffi::Window) -> Option<util::Flusher> {
-        let pid_atom = unsafe { util::get_atom(xconn, b"_NET_WM_PID\0") }
+        let pid_atom = unsafe { xconn.get_atom_unchecked(b"_NET_WM_PID\0") };
-            .expect("Failed to call XInternAtom (_NET_WM_PID)");
+        let client_machine_atom = unsafe { xconn.get_atom_unchecked(b"WM_CLIENT_MACHINE\0") };
-        let client_machine_atom = unsafe { util::get_atom(xconn, b"WM_CLIENT_MACHINE\0") }
-            .expect("Failed to call XInternAtom (WM_CLIENT_MACHINE)");
         unsafe {
             let (hostname, hostname_length) = {
                 // 64 would suffice for Linux, but 256 will be enough everywhere (as per SUSv2). For instance, this is
@@ -396,21 +377,17 @@ impl Window2 {
                 let hostname_length = libc::strlen(hostname.as_ptr());
                 (hostname, hostname_length as usize)
             };
-            util::change_property(
+            xconn.change_property(
-                xconn,
                 window,
                 pid_atom,
                 ffi::XA_CARDINAL,
-                util::Format::Long,
                 util::PropMode::Replace,
                 &[libc::getpid() as util::Cardinal],
             ).queue();
-            let flusher = util::change_property(
+            let flusher = xconn.change_property(
-                xconn,
                 window,
                 client_machine_atom,
                 ffi::XA_STRING,
-                util::Format::Char,
                 util::PropMode::Replace,
                 &hostname[0..hostname_length],
             );
@@ -423,50 +400,26 @@ impl Window2 {
         window: ffi::Window,
         window_type: util::WindowType,
     ) -> util::Flusher {
-        let hint_atom = unsafe { util::get_atom(xconn, b"_NET_WM_WINDOW_TYPE\0") }
+        let hint_atom = unsafe { xconn.get_atom_unchecked(b"_NET_WM_WINDOW_TYPE\0") };
-            .expect("Failed to call XInternAtom (_NET_WM_WINDOW_TYPE)");
         let window_type_atom = window_type.as_atom(xconn);
-        unsafe {
+        xconn.change_property(
-            util::change_property(
-                xconn,
             window,
             hint_atom,
             ffi::XA_ATOM,
-                util::Format::Long,
             util::PropMode::Replace,
             &[window_type_atom],
         )
     }
-    }
 
     pub fn set_urgent(&self, is_urgent: bool) {
         let xconn = &self.x.display;
-
+        let mut wm_hints = xconn.get_wm_hints(self.x.window).expect("`XGetWMHints` failed");
-        let mut wm_hints = {
-            let mut wm_hints = unsafe {
-                (xconn.xlib.XGetWMHints)(xconn.display, self.x.window)
-            };
-            xconn.check_errors().expect("`XGetWMHints` failed");
-            if wm_hints.is_null() {
-                wm_hints = unsafe { (xconn.xlib.XAllocWMHints)() };
-            }
-            util::XSmartPointer::new(xconn, wm_hints)
-        }.expect("`XAllocWMHints` returned null; out of memory");
-
         if is_urgent {
             (*wm_hints).flags |= ffi::XUrgencyHint;
         } else {
             (*wm_hints).flags &= !ffi::XUrgencyHint;
         }
-
+        xconn.set_wm_hints(self.x.window, wm_hints).flush().expect("Failed to set urgency hint");
-        unsafe {
-            (xconn.xlib.XSetWMHints)(
-                xconn.display,
-                self.x.window,
-                wm_hints.ptr,
-            );
-            util::flush_requests(xconn).expect("Failed to set urgency hint");
-        }
     }
 
     fn set_netwm(
@@ -476,33 +429,25 @@ impl Window2 {
         properties: (c_long, c_long, c_long, c_long),
         operation: util::StateOperation
     ) -> util::Flusher {
-        let state_atom = unsafe { util::get_atom(xconn, b"_NET_WM_STATE\0") }
+        let state_atom = unsafe { xconn.get_atom_unchecked(b"_NET_WM_STATE\0") };
-            .expect("Failed to call XInternAtom (_NET_WM_STATE)");
+        xconn.send_client_msg(
-
-        unsafe {
-            util::send_client_msg(
-                xconn,
             window,
             root,
             state_atom,
             Some(ffi::SubstructureRedirectMask | ffi::SubstructureNotifyMask),
-                (
+            [
                 operation as c_long,
                 properties.0,
                 properties.1,
                 properties.2,
                 properties.3,
+            ],
         )
-            )
-        }
     }
 
     fn set_fullscreen_hint(&self, fullscreen: bool) -> util::Flusher {
         let xconn = &self.x.display;
-
+        let fullscreen_atom = unsafe { xconn.get_atom_unchecked(b"_NET_WM_STATE_FULLSCREEN\0") };
-        let fullscreen_atom = unsafe { util::get_atom(xconn, b"_NET_WM_STATE_FULLSCREEN\0") }
-            .expect("Failed to call XInternAtom (_NET_WM_STATE_FULLSCREEN)");
-
         Window2::set_netwm(
             xconn,
             self.x.window,
@@ -548,12 +493,8 @@ impl Window2 {
 
     fn set_maximized_inner(&self, maximized: bool) -> util::Flusher {
         let xconn = &self.x.display;
-
+        let horz_atom = unsafe { xconn.get_atom_unchecked(b"_NET_WM_STATE_MAXIMIZED_HORZ\0") };
-        let horz_atom = unsafe { util::get_atom(xconn, b"_NET_WM_STATE_MAXIMIZED_HORZ\0") }
+        let vert_atom = unsafe { xconn.get_atom_unchecked(b"_NET_WM_STATE_MAXIMIZED_VERT\0") };
-            .expect("Failed to call XInternAtom (_NET_WM_STATE_MAXIMIZED_HORZ)");
-        let vert_atom = unsafe { util::get_atom(xconn, b"_NET_WM_STATE_MAXIMIZED_VERT\0") }
-            .expect("Failed to call XInternAtom (_NET_WM_STATE_MAXIMIZED_VERT)");
-
         Window2::set_netwm(
             xconn,
             self.x.window,
@@ -572,12 +513,8 @@ impl Window2 {
 
     fn set_title_inner(&self, title: &str) -> util::Flusher {
         let xconn = &self.x.display;
-
+        let wm_name_atom = unsafe { xconn.get_atom_unchecked(b"_NET_WM_NAME\0") };
-        let wm_name_atom = unsafe { util::get_atom(xconn, b"_NET_WM_NAME\0") }
+        let utf8_atom = unsafe { xconn.get_atom_unchecked(b"UTF8_STRING\0") };
-            .expect("Failed to call XInternAtom (_NET_WM_NAME)");
-        let utf8_atom = unsafe { util::get_atom(xconn, b"UTF8_STRING\0") }
-            .expect("Failed to call XInternAtom (UTF8_STRING)");
-
         let title = CString::new(title).expect("Window title contained null byte");
         unsafe {
             (xconn.xlib.XStoreName)(
@@ -585,13 +522,10 @@ impl Window2 {
                 self.x.window,
                 title.as_ptr() as *const c_char,
             );
-
+            xconn.change_property(
-            util::change_property(
-                xconn,
                 self.x.window,
                 wm_name_atom,
                 utf8_atom,
-                util::Format::Char,
                 util::PropMode::Replace,
                 title.as_bytes_with_nul(),
             )
@@ -606,17 +540,11 @@ impl Window2 {
 
     fn set_decorations_inner(&self, decorations: bool) -> util::Flusher {
         let xconn = &self.x.display;
-
+        let wm_hints = unsafe { xconn.get_atom_unchecked(b"_MOTIF_WM_HINTS\0") };
-        let wm_hints = unsafe { util::get_atom(xconn, b"_MOTIF_WM_HINTS\0") }
+        xconn.change_property(
-            .expect("Failed to call XInternAtom (_MOTIF_WM_HINTS)");
-
-        unsafe {
-            util::change_property(
-                xconn,
             self.x.window,
             wm_hints,
             wm_hints,
-                util::Format::Long,
             util::PropMode::Replace,
             &[
                 util::MWM_HINTS_DECORATIONS, // flags
@@ -627,7 +555,6 @@ impl Window2 {
             ],
         )
     }
-    }
 
     pub fn set_decorations(&self, decorations: bool) {
         self.set_decorations_inner(decorations)
@@ -638,10 +565,7 @@ impl Window2 {
 
     fn set_always_on_top_inner(&self, always_on_top: bool) -> util::Flusher {
         let xconn = &self.x.display;
-
+        let above_atom = unsafe { xconn.get_atom_unchecked(b"_NET_WM_STATE_ABOVE\0") };
-        let above_atom = unsafe { util::get_atom(xconn, b"_NET_WM_STATE_ABOVE\0") }
-            .expect("Failed to call XInternAtom (_NET_WM_STATE_ABOVE)");
-
         Window2::set_netwm(
             xconn,
             self.x.window,
@@ -659,43 +583,29 @@ impl Window2 {
 
     fn set_icon_inner(&self, icon: Icon) -> util::Flusher {
         let xconn = &self.x.display;
-
+        let icon_atom = unsafe { xconn.get_atom_unchecked(b"_NET_WM_ICON\0") };
-        let icon_atom = unsafe { util::get_atom(xconn, b"_NET_WM_ICON\0") }
-            .expect("Failed to call XInternAtom (_NET_WM_ICON)");
-
         let data = icon.to_cardinals();
-        unsafe {
+        xconn.change_property(
-            util::change_property(
-                xconn,
             self.x.window,
             icon_atom,
             ffi::XA_CARDINAL,
-                util::Format::Long,
             util::PropMode::Replace,
             data.as_slice(),
         )
     }
-    }
 
     fn unset_icon_inner(&self) -> util::Flusher {
         let xconn = &self.x.display;
-
+        let icon_atom = unsafe { xconn.get_atom_unchecked(b"_NET_WM_ICON\0") };
-        let icon_atom = unsafe { util::get_atom(xconn, b"_NET_WM_ICON\0") }
-            .expect("Failed to call XInternAtom (_NET_WM_ICON)");
-
         let empty_data: [util::Cardinal; 0] = [];
-        unsafe {
+        xconn.change_property(
-            util::change_property(
-                xconn,
             self.x.window,
             icon_atom,
             ffi::XA_CARDINAL,
-                util::Format::Long,
             util::PropMode::Replace,
             &empty_data,
         )
     }
-    }
 
     pub fn set_window_icon(&self, icon: Option<Icon>) {
         match icon {
@@ -707,7 +617,7 @@ impl Window2 {
     pub fn show(&self) {
         unsafe {
             (self.x.display.xlib.XMapRaised)(self.x.display.display, self.x.window);
-            util::flush_requests(&self.x.display)
+            self.x.display.flush_requests()
                 .expect("Failed to call XMapRaised");
         }
     }
@@ -715,17 +625,13 @@ impl Window2 {
     pub fn hide(&self) {
         unsafe {
             (self.x.display.xlib.XUnmapWindow)(self.x.display.display, self.x.window);
-            util::flush_requests(&self.x.display)
+            self.x.display.flush_requests()
                 .expect("Failed to call XUnmapWindow");
         }
     }
 
     fn update_cached_frame_extents(&self) {
-        let extents = util::get_frame_extents_heuristic(
+        let extents = self.x.display.get_frame_extents_heuristic(self.x.window, self.x.root);
-            &self.x.display,
-            self.x.window,
-            self.x.root,
-        );
         (*self.shared_state.lock()).frame_extents = Some(extents);
     }
 
@@ -748,7 +654,7 @@ impl Window2 {
 
     #[inline]
     pub fn get_inner_position(&self) -> Option<(i32, i32)> {
-        unsafe { util::translate_coords(&self.x.display, self.x.window, self.x.root )}
+        self.x.display.translate_coords(self.x.window, self.x.root )
             .ok()
             .map(|coords| (coords.x_rel_root, coords.y_rel_root))
     }
@@ -773,13 +679,13 @@ impl Window2 {
                 x as c_int,
                 y as c_int,
             );
-            util::flush_requests(&self.x.display)
+            self.x.display.flush_requests()
         }.expect("Failed to call XMoveWindow");
     }
 
     #[inline]
     pub fn get_inner_size(&self) -> Option<(u32, u32)> {
-        unsafe { util::get_geometry(&self.x.display, self.x.window) }
+        self.x.display.get_geometry(self.x.window)
             .ok()
             .map(|geo| (geo.width, geo.height))
     }
@@ -806,7 +712,7 @@ impl Window2 {
                 width as c_uint,
                 height as c_uint,
             );
-            util::flush_requests(&self.x.display)
+            self.x.display.flush_requests()
         }.expect("Failed to call XResizeWindow");
     }
 
@@ -815,14 +721,8 @@ impl Window2 {
     {
         let xconn = &self.x.display;
 
-        let size_hints = {
+        let size_hints = xconn.alloc_size_hints();
-            let size_hints = (xconn.xlib.XAllocSizeHints)();
-            util::XSmartPointer::new(&xconn, size_hints)
-                .expect("XAllocSizeHints returned null; out of memory")
-        };
-
         let mut flags: c_long = mem::uninitialized();
-
         (xconn.xlib.XGetWMNormalHints)(
             xconn.display,
             self.x.window,
@@ -838,7 +738,7 @@ impl Window2 {
             self.x.window,
             size_hints.ptr,
         );
-        util::flush_requests(xconn)?;
+        xconn.flush_requests()?;
 
         Ok(())
     }
@@ -994,7 +894,7 @@ impl Window2 {
             if cursor != 0 {
                 (self.x.display.xlib.XFreeCursor)(self.x.display.display, cursor);
             }
-            util::flush_requests(&self.x.display).expect("Failed to set or free the cursor");
+            self.x.display.flush_requests().expect("Failed to set or free the cursor");
         }
     }
 
@@ -1057,7 +957,7 @@ impl Window2 {
             Grab => {
                 unsafe {
                     (self.x.display.xlib.XUngrabPointer)(self.x.display.display, ffi::CurrentTime);
-                    util::flush_requests(&self.x.display).expect("Failed to call XUngrabPointer");
+                    self.x.display.flush_requests().expect("Failed to call XUngrabPointer");
                 }
             },
             Normal => {},
@@ -1123,7 +1023,7 @@ impl Window2 {
                 x,
                 y,
             );
-            util::flush_requests(&self.x.display).map_err(|_| ())
+            self.x.display.flush_requests().map_err(|_| ())
         }
     }