re-structure clipboard handling for mux

The wayland changes rendered clipboard handling for remote multiplexers
broken, and this commit makes it work again.

It removes the clipboard concept from the the TerminalHost and
keeps it separated as the term::Clipboard concept.

The muxer now has plumbing for passing the Clipboard to its idea
of Windows and Tabs and this is hooked up at window creation and
domain attach time.

src/frontend/gui/termwindow.rs

@@ -65,7 +65,6 @@ pub struct TermWindow {
 struct Host<'a> {
     writer: &'a mut dyn std::io::Write,
     context: &'a dyn WindowOps,
-    clipboard: Arc<dyn term::Clipboard>,
 }
 
 impl<'a> term::TerminalHost for Host<'a> {
@@ -73,10 +72,6 @@ impl<'a> term::TerminalHost for Host<'a> {
         self.writer
     }
 
-    fn get_clipboard(&mut self) -> Fallible<Arc<dyn term::Clipboard>> {
-        Ok(Arc::clone(&self.clipboard))
-    }
-
     fn set_title(&mut self, title: &str) {
         self.context.set_title(title);
     }
@@ -221,9 +216,6 @@ impl WindowCallbacks for TermWindow {
                 &mut Host {
                     writer: &mut *tab.writer(),
                     context,
-                    clipboard: Arc::new(ClipboardHelper {
-                        window: self.window.as_ref().unwrap().clone(),
-                    }),
                 },
             )
             .ok();
@@ -477,6 +469,16 @@ impl TermWindow {
             },
         );
 
+        let clipboard: Arc<dyn term::Clipboard> = Arc::new(ClipboardHelper {
+            window: window.clone(),
+        });
+        tab.set_clipboard(&clipboard);
+        Mux::get()
+            .unwrap()
+            .get_window_mut(mux_window_id)
+            .unwrap()
+            .set_clipboard(&clipboard);
+
         if super::is_opengl_enabled() {
             window.enable_opengl(|any, window, maybe_ctx| {
                 let mut termwindow = any.downcast_mut::<TermWindow>().expect("to be TermWindow");
@@ -792,6 +794,11 @@ impl TermWindow {
         let tab = domain.spawn(size, None, self.mux_window_id)?;
         let tab_id = tab.tab_id();
 
+        let clipboard: Arc<dyn term::Clipboard> = Arc::new(ClipboardHelper {
+            window: self.window.as_ref().unwrap().clone(),
+        });
+        tab.set_clipboard(&clipboard);
+
         let len = {
             let window = mux
                 .get_window(self.mux_window_id)
@@ -819,8 +826,6 @@ impl TermWindow {
                 // self.toggle_full_screen(),
             }
             Copy => {
-                //    self.clipboard.set_contents(tab.selection_text())?;
-                log::error!("Copy pressed");
                 if let Some(text) = tab.selection_text() {
                     self.window.as_ref().unwrap().set_clipboard(text);
                 }

src/localtab.rs

@@ -4,9 +4,10 @@ use crate::mux::tab::{alloc_tab_id, Tab, TabId};
 use failure::Error;
 use portable_pty::{Child, MasterPty, PtySize};
 use std::cell::{RefCell, RefMut};
+use std::sync::Arc;
 use term::color::ColorPalette;
 use term::selection::SelectionRange;
-use term::{KeyCode, KeyModifiers, MouseEvent, Terminal, TerminalHost};
+use term::{Clipboard, KeyCode, KeyModifiers, MouseEvent, Terminal, TerminalHost};
 
 pub struct LocalTab {
     tab_id: TabId,
@@ -35,6 +36,10 @@ impl Tab for LocalTab {
         }
     }
 
+    fn set_clipboard(&self, clipboard: &Arc<dyn Clipboard>) {
+        self.terminal.borrow_mut().set_clipboard(clipboard);
+    }
+
     fn advance_bytes(&self, buf: &[u8], host: &mut dyn TerminalHost) {
         self.terminal.borrow_mut().advance_bytes(buf, host)
     }

src/mux/mod.rs

@@ -4,7 +4,7 @@ use crate::mux::window::{Window, WindowId};
 use crate::ratelim::RateLimiter;
 use crate::server::pollable::{pollable_channel, PollableReceiver, PollableSender};
 use domain::{Domain, DomainId};
-use failure::{bail, format_err, Error, Fallible};
+use failure::{format_err, Error, Fallible};
 use failure_derive::*;
 use log::{debug, error};
 use portable_pty::ExitStatus;
@@ -16,7 +16,6 @@ use std::rc::Rc;
 use std::sync::atomic::{AtomicUsize, Ordering};
 use std::sync::Arc;
 use std::thread;
-use term::terminal::Clipboard;
 use term::TerminalHost;
 use termwiz::hyperlink::Hyperlink;
 
@@ -119,10 +118,6 @@ impl<'a> TerminalHost for Host<'a> {
         }
     }
 
-    fn get_clipboard(&mut self) -> Fallible<Arc<dyn Clipboard>> {
-        bail!("peer requested clipboard; ignoring");
-    }
-
     fn set_title(&mut self, _title: &str) {}
 }
 

src/mux/tab.rs

@@ -9,7 +9,7 @@ use std::cell::RefMut;
 use std::sync::{Arc, Mutex};
 use term::color::ColorPalette;
 use term::selection::SelectionRange;
-use term::{KeyCode, KeyModifiers, MouseEvent, TerminalHost};
+use term::{Clipboard, KeyCode, KeyModifiers, MouseEvent, TerminalHost};
 
 static TAB_ID: ::std::sync::atomic::AtomicUsize = ::std::sync::atomic::AtomicUsize::new(0);
 pub type TabId = usize;
@@ -63,6 +63,8 @@ pub trait Tab: Downcast {
     fn palette(&self) -> ColorPalette;
     fn domain_id(&self) -> DomainId;
 
+    fn set_clipboard(&self, _clipboard: &Arc<dyn Clipboard>) {}
+
     /// Returns the selection range adjusted to the viewport
     /// (eg: it has been normalized and had clip_to_viewport called
     /// on it prior to being returned)

src/mux/window.rs

@@ -1,5 +1,7 @@
 use crate::mux::{Tab, TabId};
 use std::rc::Rc;
+use std::sync::Arc;
+use term::Clipboard;
 
 static WIN_ID: ::std::sync::atomic::AtomicUsize = ::std::sync::atomic::AtomicUsize::new(0);
 pub type WindowId = usize;
@@ -8,6 +10,7 @@ pub struct Window {
     id: WindowId,
     tabs: Vec<Rc<dyn Tab>>,
     active: usize,
+    clipboard: Option<Arc<dyn Clipboard>>,
 }
 
 impl Window {
@@ -16,9 +19,18 @@ impl Window {
             id: WIN_ID.fetch_add(1, ::std::sync::atomic::Ordering::Relaxed),
             tabs: vec![],
             active: 0,
+            clipboard: None,
         }
     }
 
+    pub fn set_clipboard(&mut self, clipboard: &Arc<dyn Clipboard>) {
+        self.clipboard.replace(Arc::clone(clipboard));
+    }
+
+    pub fn get_clipboard(&self) -> Option<&Arc<dyn Clipboard>> {
+        self.clipboard.as_ref()
+    }
+
     pub fn window_id(&self) -> WindowId {
         self.id
     }
@@ -27,6 +39,9 @@ impl Window {
         for t in &self.tabs {
             assert_ne!(t.tab_id(), tab.tab_id(), "tab already added to this window");
         }
+        if let Some(clip) = self.clipboard.as_ref() {
+            tab.set_clipboard(clip);
+        }
         self.tabs.push(Rc::clone(tab))
     }
 

src/server/listener.rs

@@ -547,13 +547,6 @@ impl<'a> term::TerminalHost for BufferedTerminalHost<'a> {
             .ok();
     }
 
-    fn get_clipboard(&mut self) -> Fallible<Arc<dyn Clipboard>> {
-        Ok(Arc::new(RemoteClipboard {
-            tab_id: self.tab_id,
-            sender: self.sender.clone(),
-        }))
-    }
-
     fn set_title(&mut self, title: &str) {
         self.title.replace(title.to_owned());
     }
@@ -761,26 +754,36 @@ impl<S: ReadAndWrite> ClientSession<S> {
                 })
             }
 
-            Pdu::Spawn(spawn) => Future::with_executor(executor(), move || {
-                let mux = Mux::get().unwrap();
-                let domain = mux.get_domain(spawn.domain_id).ok_or_else(|| {
-                    format_err!("domain {} not found on this server", spawn.domain_id)
-                })?;
-
-                let window_id = if let Some(window_id) = spawn.window_id {
-                    mux.get_window_mut(window_id).ok_or_else(|| {
-                        format_err!("window_id {} not found on this server", window_id)
+            Pdu::Spawn(spawn) => Future::with_executor(executor(), {
+                let sender = self.to_write_tx.clone();
+                move || {
+                    let mux = Mux::get().unwrap();
+                    let domain = mux.get_domain(spawn.domain_id).ok_or_else(|| {
+                        format_err!("domain {} not found on this server", spawn.domain_id)
                     })?;
-                    window_id
-                } else {
-                    mux.new_empty_window()
-                };
 
-                let tab = domain.spawn(spawn.size, spawn.command, window_id)?;
-                Ok(Pdu::SpawnResponse(SpawnResponse {
-                    tab_id: tab.tab_id(),
-                    window_id,
-                }))
+                    let window_id = if let Some(window_id) = spawn.window_id {
+                        mux.get_window_mut(window_id).ok_or_else(|| {
+                            format_err!("window_id {} not found on this server", window_id)
+                        })?;
+                        window_id
+                    } else {
+                        mux.new_empty_window()
+                    };
+
+                    let tab = domain.spawn(spawn.size, spawn.command, window_id)?;
+
+                    let clip: Arc<dyn Clipboard> = Arc::new(RemoteClipboard {
+                        tab_id: tab.tab_id(),
+                        sender,
+                    });
+                    tab.set_clipboard(&clip);
+
+                    Ok(Pdu::SpawnResponse(SpawnResponse {
+                        tab_id: tab.tab_id(),
+                        window_id,
+                    }))
+                }
             }),
 
             Pdu::GetTabRenderChanges(GetTabRenderChanges { tab_id, .. }) => {

src/server/tab.rs

@@ -20,7 +20,7 @@ use std::sync::{Arc, Mutex};
 use std::time::{Duration, Instant};
 use term::color::ColorPalette;
 use term::selection::SelectionRange;
-use term::{CursorPosition, Line};
+use term::{Clipboard, CursorPosition, Line};
 use term::{KeyCode, KeyModifiers, MouseButton, MouseEvent, MouseEventKind, TerminalHost};
 use termwiz::hyperlink::Hyperlink;
 use termwiz::input::KeyEvent;
@@ -132,6 +132,7 @@ pub struct ClientTab {
     writer: RefCell<TabWriter>,
     reader: Pipe,
     mouse: Arc<Mutex<MouseState>>,
+    clipboard: RefCell<Option<Arc<dyn Clipboard>>>,
 }
 
 impl ClientTab {
@@ -181,6 +182,7 @@ impl ClientTab {
             renderable: RefCell::new(render),
             writer: RefCell::new(writer),
             reader,
+            clipboard: RefCell::new(None),
         }
     }
 
@@ -195,7 +197,14 @@ impl ClientTab {
                     .apply_changes_to_surface(delta.sequence_no, delta.changes);
             }
             Pdu::SetClipboard(SetClipboard { clipboard, .. }) => {
-                log::error!("Ignoring SetClipboard request {:?}", clipboard);
+                match self.clipboard.borrow().as_ref() {
+                    Some(clip) => {
+                        clip.set_contents(clipboard)?;
+                    }
+                    None => {
+                        log::error!("ClientTab: Ignoring SetClipboard request {:?}", clipboard);
+                    }
+                }
             }
             Pdu::OpenURL(OpenURL { url, .. }) => {
                 // FIXME: ideally we'd have a provider that we can
@@ -225,6 +234,10 @@ impl Tab for ClientTab {
         self.renderable.borrow_mut()
     }
 
+    fn set_clipboard(&self, clipboard: &Arc<dyn Clipboard>) {
+        self.clipboard.borrow_mut().replace(Arc::clone(clipboard));
+    }
+
     fn get_title(&self) -> String {
         let renderable = self.renderable.borrow();
         let surface = &renderable.inner.borrow().surface;

term/src/terminal.rs

@@ -26,9 +26,6 @@ pub trait TerminalHost {
     /// slave end of the associated pty.
     fn writer(&mut self) -> &mut dyn std::io::Write;
 
-    /// Returns the clipboard manager
-    fn get_clipboard(&mut self) -> Fallible<Arc<dyn Clipboard>>;
-
     /// Change the title of the window
     fn set_title(&mut self, title: &str);
 

term/src/terminalstate.rs

@@ -3,7 +3,7 @@
 #![cfg_attr(feature = "cargo-clippy", allow(clippy::range_plus_one))]
 use super::*;
 use crate::color::ColorPalette;
-use failure::bail;
+use failure::{bail, Fallible};
 use image::{self, GenericImageView};
 use log::{debug, error};
 use ordered_float::NotNan;
@@ -218,6 +218,8 @@ pub struct TerminalState {
 
     pixel_width: usize,
     pixel_height: usize,
+
+    clipboard: Option<Arc<dyn Clipboard>>,
 }
 
 fn encode_modifiers(mods: KeyModifiers) -> u8 {
@@ -295,9 +297,14 @@ impl TerminalState {
             palette: ColorPalette::default(),
             pixel_height,
             pixel_width,
+            clipboard: None,
         }
     }
 
+    pub fn set_clipboard(&mut self, clipboard: &Arc<dyn Clipboard>) {
+        self.clipboard.replace(Arc::clone(clipboard));
+    }
+
     pub fn get_title(&self) -> &str {
         &self.title
     }
@@ -472,12 +479,23 @@ impl TerminalState {
         self.invalidate_hyperlinks();
     }
 
+    fn set_clipboard_contents(&self, text: Option<String>) -> Fallible<()> {
+        if let Some(clip) = self.clipboard.as_ref() {
+            clip.set_contents(text)?;
+        }
+        Ok(())
+    }
+
+    fn get_clipboard_contents(&self) -> Fallible<String> {
+        if let Some(clip) = self.clipboard.as_ref() {
+            clip.get_contents()
+        } else {
+            Ok(String::new())
+        }
+    }
+
     /// Single click prepares the start of a new selection
-    fn mouse_single_click_left(
-        &mut self,
-        event: MouseEvent,
-        host: &mut dyn TerminalHost,
-    ) -> Result<(), Error> {
+    fn mouse_single_click_left(&mut self, event: MouseEvent) -> Result<(), Error> {
         // Prepare to start a new selection.
         // We don't form the selection until the mouse drags.
         self.selection_range = None;
@@ -486,15 +504,11 @@ impl TerminalState {
             y: event.y as ScrollbackOrVisibleRowIndex
                 - self.viewport_offset as ScrollbackOrVisibleRowIndex,
         });
-        host.get_clipboard()?.set_contents(None)
+        self.set_clipboard_contents(None)
     }
 
     /// Double click to select a word on the current line
-    fn mouse_double_click_left(
-        &mut self,
-        event: MouseEvent,
-        host: &mut dyn TerminalHost,
-    ) -> Result<(), Error> {
+    fn mouse_double_click_left(&mut self, event: MouseEvent) -> Result<(), Error> {
         let y = event.y as ScrollbackOrVisibleRowIndex
             - self.viewport_offset as ScrollbackOrVisibleRowIndex;
         let idx = self.screen().scrollback_or_visible_row(y);
@@ -563,15 +577,11 @@ impl TerminalState {
             "finish 2click selection {:?} '{}'",
             self.selection_range, text
         );
-        host.get_clipboard()?.set_contents(Some(text))
+        self.set_clipboard_contents(Some(text))
     }
 
     /// triple click to select the current line
-    fn mouse_triple_click_left(
-        &mut self,
-        event: MouseEvent,
-        host: &mut dyn TerminalHost,
-    ) -> Result<(), Error> {
+    fn mouse_triple_click_left(&mut self, event: MouseEvent) -> Result<(), Error> {
         let y = event.y as ScrollbackOrVisibleRowIndex
             - self.viewport_offset as ScrollbackOrVisibleRowIndex;
         self.selection_start = Some(SelectionCoordinate { x: event.x, y });
@@ -588,31 +598,27 @@ impl TerminalState {
             "finish 3click selection {:?} '{}'",
             self.selection_range, text
         );
-        host.get_clipboard()?.set_contents(Some(text))
+        self.set_clipboard_contents(Some(text))
     }
 
-    fn mouse_press_left(
-        &mut self,
-        event: MouseEvent,
-        host: &mut dyn TerminalHost,
-    ) -> Result<(), Error> {
+    fn mouse_press_left(&mut self, event: MouseEvent) -> Result<(), Error> {
         self.current_mouse_button = MouseButton::Left;
         self.dirty_selection_lines();
         match self.last_mouse_click.as_ref() {
             Some(&LastMouseClick { streak: 1, .. }) => {
-                self.mouse_single_click_left(event, host)?;
+                self.mouse_single_click_left(event)?;
             }
             Some(&LastMouseClick { streak: 2, .. }) => {
-                self.mouse_double_click_left(event, host)?;
+                self.mouse_double_click_left(event)?;
             }
             Some(&LastMouseClick { streak: 3, .. }) => {
-                self.mouse_triple_click_left(event, host)?;
+                self.mouse_triple_click_left(event)?;
             }
             // otherwise, clear out the selection
             _ => {
                 self.selection_range = None;
                 self.selection_start = None;
-                host.get_clipboard()?.set_contents(None)?;
+                self.set_clipboard_contents(None)?;
             }
         }
 
@@ -635,7 +641,7 @@ impl TerminalState {
                     "finish drag selection {:?} '{}'",
                     self.selection_range, text
                 );
-                host.get_clipboard()?.set_contents(Some(text))?;
+                self.set_clipboard_contents(Some(text))?;
             } else if let Some(link) = self.current_highlight() {
                 // If the button release wasn't a drag, consider
                 // whether it was a click on a hyperlink
@@ -711,7 +717,7 @@ impl TerminalState {
                     format!("\x1b[<{};{};{}M", button, event.x + 1, event.y + 1).as_bytes(),
                 )?;
             } else if event.button == MouseButton::Middle {
-                let clip = host.get_clipboard()?.get_contents()?;
+                let clip = self.get_clipboard_contents()?;
                 self.send_paste(&clip, host.writer())?
             }
         }
@@ -800,7 +806,7 @@ impl TerminalState {
                     },
                     _,
                 ) => {
-                    return self.mouse_press_left(event, host);
+                    return self.mouse_press_left(event);
                 }
                 (
                     MouseEvent {
@@ -2205,19 +2211,13 @@ impl<'a> Performer<'a> {
             }
 
             OperatingSystemCommand::ClearSelection(_) => {
-                if let Ok(clip) = self.host.get_clipboard() {
-                    clip.set_contents(None).ok();
-                }
+                self.set_clipboard_contents(None).ok();
             }
             OperatingSystemCommand::QuerySelection(_) => {}
             OperatingSystemCommand::SetSelection(_, selection_data) => {
-                if let Ok(clip) = self.host.get_clipboard() {
-                    match clip.set_contents(Some(selection_data)) {
-                        Ok(_) => (),
-                        Err(err) => {
-                            error!("failed to set clipboard in response to OSC 52: {:?}", err)
-                        }
-                    }
+                match self.set_clipboard_contents(Some(selection_data)) {
+                    Ok(_) => (),
+                    Err(err) => error!("failed to set clipboard in response to OSC 52: {:?}", err),
                 }
             }
             OperatingSystemCommand::ITermProprietary(iterm) => match iterm {

term/src/test/mod.rs

@@ -16,14 +16,12 @@ use termwiz::escape::{OneBased, OperatingSystemCommand, CSI};
 
 struct TestHost {
     title: String,
-    clip: Arc<dyn Clipboard>,
 }
 
 impl TestHost {
     fn new() -> Self {
         Self {
             title: String::new(),
-            clip: Arc::new(LocalClip::new()),
         }
     }
 }
@@ -70,10 +68,6 @@ impl TerminalHost for TestHost {
         self.title = title.into();
     }
 
-    fn get_clipboard(&mut self) -> Fallible<Arc<dyn Clipboard>> {
-        Ok(Arc::clone(&self.clip))
-    }
-
     fn writer(&mut self) -> &mut dyn std::io::Write {
         self
     }
@@ -84,6 +78,7 @@ impl TerminalHost for TestHost {
 struct TestTerm {
     term: Terminal,
     host: TestHost,
+    clip: Arc<dyn Clipboard>,
 }
 
 #[derive(Debug)]
@@ -98,15 +93,20 @@ impl TerminalConfiguration for TestTermConfig {
 
 impl TestTerm {
     fn new(height: usize, width: usize, scrollback: usize) -> Self {
+        let mut term = Terminal::new(
+            height,
+            width,
+            height * 16,
+            width * 8,
+            Arc::new(TestTermConfig { scrollback }),
+        );
+        let clip: Arc<dyn Clipboard> = Arc::new(LocalClip::new());
+        term.set_clipboard(&clip);
+
         Self {
-            term: Terminal::new(
-                height,
-                width,
-                height * 16,
-                width * 8,
-                Arc::new(TestTermConfig { scrollback }),
-            ),
+            term,
             host: TestHost::new(),
+            clip,
         }
     }
 
@@ -171,7 +171,7 @@ impl TestTerm {
     }
 
     fn get_clipboard(&self) -> Option<String> {
-        self.host.clip.get_contents().ok()
+        self.clip.get_contents().ok()
     }
 
     /// Inject n_times clicks of the button at the specified coordinates