Clipboard improvements. Still not working right. Nedit

doesn't work at all but at least now there's a timeout to
prevent synergy from hanging waiting on a reply.

client/CXWindowsSecondaryScreen.cpp

@@ -89,6 +89,20 @@ void					CXWindowsSecondaryScreen::run()
 								xevent.xselectionrequest.property,
 								xevent.xselectionrequest.time);
 			}
+			else {
+				// unknown window.  return failure.
+				CDisplayLock display(this);
+				XEvent event;
+				event.xselection.type      = SelectionNotify;
+				event.xselection.display   = display;
+				event.xselection.requestor = xevent.xselectionrequest.requestor;
+				event.xselection.selection = xevent.xselectionrequest.selection;
+				event.xselection.target    = xevent.xselectionrequest.target;
+				event.xselection.property  = None;
+				event.xselection.time      = xevent.xselectionrequest.time;
+				XSendEvent(display, xevent.xselectionrequest.requestor,
+								False, 0, &event);
+			}
 			break;
 
 		  case PropertyNotify:
@@ -269,14 +283,12 @@ void					CXWindowsSecondaryScreen::mouseWheel(SInt32)
 void					CXWindowsSecondaryScreen::setClipboard(
 								ClipboardID id, const IClipboard* clipboard)
 {
-	// FIXME -- don't use CurrentTime
+	setDisplayClipboard(id, clipboard, m_window, getCurrentTime(m_window));
-	setDisplayClipboard(id, clipboard, m_window, CurrentTime);
 }
 
 void					CXWindowsSecondaryScreen::grabClipboard(ClipboardID id)
 {
-	// FIXME -- don't use CurrentTime
+	setDisplayClipboard(id, NULL, m_window, getCurrentTime(m_window));
-	setDisplayClipboard(id, NULL, m_window, CurrentTime);
 }
 
 void					CXWindowsSecondaryScreen::getSize(
@@ -293,8 +305,7 @@ SInt32					CXWindowsSecondaryScreen::getJumpZoneSize() const
 void					CXWindowsSecondaryScreen::getClipboard(
 								ClipboardID id, IClipboard* clipboard) const
 {
-	// FIXME -- don't use CurrentTime
+	getDisplayClipboard(id, clipboard, m_window, getCurrentTime(m_window));
-	getDisplayClipboard(id, clipboard, m_window, CurrentTime);
 }
 
 void					CXWindowsSecondaryScreen::onOpenDisplay()

server/CXWindowsPrimaryScreen.cpp

@@ -163,6 +163,20 @@ void					CXWindowsPrimaryScreen::run()
 								xevent.xselectionrequest.property,
 								xevent.xselectionrequest.time);
 			}
+			else {
+				// unknown window.  return failure.
+				CDisplayLock display(this);
+				XEvent event;
+				event.xselection.type      = SelectionNotify;
+				event.xselection.display   = display;
+				event.xselection.requestor = xevent.xselectionrequest.requestor;
+				event.xselection.selection = xevent.xselectionrequest.selection;
+				event.xselection.target    = xevent.xselectionrequest.target;
+				event.xselection.property  = None;
+				event.xselection.time      = xevent.xselectionrequest.time;
+				XSendEvent(display, xevent.xselectionrequest.requestor,
+								False, 0, &event);
+			}
 			break;
 
 		  case PropertyNotify:
@@ -326,14 +340,12 @@ void					CXWindowsPrimaryScreen::warpCursorNoLock(
 void					CXWindowsPrimaryScreen::setClipboard(
 								ClipboardID id, const IClipboard* clipboard)
 {
-	// FIXME -- don't use CurrentTime
+	setDisplayClipboard(id, clipboard, m_window, getCurrentTime(m_window));
-	setDisplayClipboard(id, clipboard, m_window, CurrentTime);
 }
 
 void					CXWindowsPrimaryScreen::grabClipboard(ClipboardID id)
 {
-	// FIXME -- don't use CurrentTime
+	setDisplayClipboard(id, NULL, m_window, getCurrentTime(m_window));
-	setDisplayClipboard(id, NULL, m_window, CurrentTime);
 }
 
 void					CXWindowsPrimaryScreen::getSize(
@@ -350,8 +362,7 @@ SInt32					CXWindowsPrimaryScreen::getJumpZoneSize() const
 void					CXWindowsPrimaryScreen::getClipboard(
 								ClipboardID id, IClipboard* clipboard) const
 {
-	// FIXME -- don't use CurrentTime
+	getDisplayClipboard(id, clipboard, m_window, getCurrentTime(m_window));
-	getDisplayClipboard(id, clipboard, m_window, CurrentTime);
 }
 
 void					CXWindowsPrimaryScreen::onOpenDisplay()
@@ -371,7 +382,7 @@ void					CXWindowsPrimaryScreen::onOpenDisplay()
 	attr.event_mask            = PointerMotionMask |// PointerMotionHintMask |
 								 ButtonPressMask | ButtonReleaseMask |
 								 KeyPressMask | KeyReleaseMask |
-								 KeymapStateMask;
+								 KeymapStateMask | PropertyChangeMask;
 	attr.do_not_propagate_mask = 0;
 	attr.override_redirect     = True;
 	attr.cursor                = createBlankCursor();

synergy/CXWindowsScreen.cpp

@@ -4,6 +4,7 @@
 #include "TMethodJob.h"
 #include "CLog.h"
 #include "CString.h"
+#include "CStopwatch.h"
 #include <string.h>
 #include <assert.h>
 #include <X11/X.h>
@@ -64,6 +65,7 @@ void					CXWindowsScreen::openDisplay()
 	m_atomString       = XInternAtom(m_display, "STRING", False);
 	m_atomText         = XInternAtom(m_display, "TEXT", False);
 	m_atomCompoundText = XInternAtom(m_display, "COMPOUND_TEXT", False);
+	m_atomSynergyTime  = XInternAtom(m_display, "SYNERGY_TIME", False);
 
 	// clipboard atoms
 	m_atomClipboard[kClipboardClipboard] =
@@ -83,7 +85,7 @@ void					CXWindowsScreen::closeDisplay()
 
 	// clear out the clipboard request lists
 	for (ClipboardID id = 0; id < kClipboardEnd; ++id) {
-		ClipboardInfo& clipboard = m_clipboards[id];
+		CClipboardInfo& clipboard = m_clipboards[id];
 		for (CRequestMap::iterator index = clipboard.m_requests.begin();
 								index != clipboard.m_requests.end(); ++index) {
 			CRequestList* list = index->second;
@@ -221,6 +223,7 @@ bool					CXWindowsScreen::setDisplayClipboard(
 		// we got the selection
 		log((CLOG_INFO "grabbed clipboard at %d", timestamp));
 		m_clipboards[id].m_gotClipboard = timestamp;
+		m_clipboards[id].m_lostClipboard = CurrentTime;
 
 		if (clipboard != NULL) {
 			// save clipboard to serve requests
@@ -271,7 +274,7 @@ void					CXWindowsScreen::getDisplayClipboard(
 		const SInt32 numTargets = targets.size() / sizeof(Atom);
 		std::set<IClipboard::EFormat> clipboardFormats;
 		std::set<Atom> targets;
-		log((CLOG_INFO "getting selection with %d targets", numTargets));
+		log((CLOG_INFO "getting selection %d with %d targets", id, numTargets));
 		for (SInt32 i = 0; i < numTargets; ++i) {
 			Atom format = targetAtoms[i];
 			log((CLOG_DEBUG1 " source target %d", format));
@@ -341,6 +344,8 @@ bool					CXWindowsScreen::getDisplayClipboard(
 	assert(outputType != NULL);
 	assert(outputData != NULL);
 
+	// FIXME -- this doesn't work to retrieve Motif selections.
+
 	// delete data property
 	XDeleteProperty(m_display, requestor, m_atomData);
 
@@ -350,12 +355,18 @@ bool					CXWindowsScreen::getDisplayClipboard(
 
 	// wait for the selection notify event.  can't just mask out other
 	// events because X stupidly doesn't provide a mask for selection
-	// events, so we use a predicate to find our event.
+	// events, so we use a predicate to find our event.  we also set
+	// a time limit for a response so we're not screwed by a bad
+	// clipboard owner.
+	CStopwatch timer(true);
 	XEvent xevent;
-// FIXME -- must limit the time we wait for bad clients
 	while (XCheckIfEvent(m_display, &xevent,
 								&CXWindowsScreen::findSelectionNotify,
 								(XPointer)&requestor) != True) {
+		// return false if we've timed-out
+		if (timer.getTime() >= 1.0)
+			return false;
+
 		// wait a bit
 		CThread::sleep(0.05);
 	}
@@ -557,36 +568,41 @@ Bool					CXWindowsScreen::findPropertyNotify(
 }
 
 void					CXWindowsScreen::addClipboardRequest(
-								Window /*owner*/, Window requestor,
+								Window owner, Window requestor,
 								Atom selection, Atom target,
 								Atom property, Time time)
 {
+	bool success = false;
+
 	// see if it's a selection we know about
 	ClipboardID id;
 	for (id = 0; id < kClipboardEnd; ++id)
 		if (selection == m_atomClipboard[id])
 			break;
-	if (id == kClipboardEnd)
-		return;
 
 	// mutex the display
 	CLock lock(&m_mutex);
-	bool success = false;
 
 	// a request for multiple targets is special
-	if (target == m_atomMultiple) {
+	if (id != kClipboardEnd) {
-		// add a multiple request
+		// check time we own the selection against the requested time
-		if (property != None) {
+		if (!wasOwnedAtTime(id, owner, time)) {
-			success = sendClipboardMultiple(id, requestor, property, time);
+			// fail for time we didn't own selection
+		}
+		else if (target == m_atomMultiple) {
+			// add a multiple request
+			if (property != None) {
+				success = sendClipboardMultiple(id, requestor, property, time);
+			}
+		}
+		else {
+			// handle remaining request formats
+			success = sendClipboardData(id, requestor, target, property, time);
 		}
-	}
-	else {
-		// handle remaining request formats
-		success = sendClipboardData(id, requestor, target, property, time);
 	}
 
 	// send success or failure
-	sendNotify(id, requestor, target, success ? property : None, time);
+	sendNotify(requestor, selection, target, success ? property : None, time);
 }
 
 void					CXWindowsScreen::processClipboardRequest(
@@ -597,7 +613,7 @@ void					CXWindowsScreen::processClipboardRequest(
 
 	// check every clipboard
 	for (ClipboardID id = 0; id < kClipboardEnd; ++id) {
-		ClipboardInfo& clipboard = m_clipboards[id];
+		CClipboardInfo& clipboard = m_clipboards[id];
 
 		// find the request list
 		CRequestMap::iterator index = clipboard.m_requests.find(requestor);
@@ -670,7 +686,7 @@ void					CXWindowsScreen::destroyClipboardRequest(
 
 	// check every clipboard
 	for (ClipboardID id = 0; id < kClipboardEnd; ++id) {
-		ClipboardInfo& clipboard = m_clipboards[id];
+		CClipboardInfo& clipboard = m_clipboards[id];
 
 		// find the request list
 		CRequestMap::iterator index = clipboard.m_requests.find(requestor);
@@ -789,60 +805,57 @@ bool					CXWindowsScreen::sendClipboardMultiple(
 	Atom type;
 	SInt32 size;
 	CString data;
-	getData(requestor, property, &type, &size, &data);
+	if (!getData(requestor, property, &type, &size, &data)) {
-	if (type != m_atomAtomPair) {
+		type = 0;
-		// unexpected data type
-		return false;
 	}
 
-	// check each format, replacing ones we can't do with None.  set
+	// we only handle atom pair type
-	// the property for each to the requested data (for small requests)
-	// or INCR (for large requests).
 	bool success = false;
-	bool updated = false;
+	if (type == m_atomAtomPair) {
-	UInt32 numRequests = data.size() / (2 * sizeof(Atom));
+		// check each format, replacing ones we can't do with None.  set
-	for (UInt32 index = 0; index < numRequests; ++index) {
+		// the property for each to the requested data (for small requests)
-		// get request info
+		// or INCR (for large requests).
-		const Atom* request = reinterpret_cast<const Atom*>(data.data());
+		bool updated = false;
-		const Atom target   = request[2 * index + 0];
+		UInt32 numRequests = data.size() / (2 * sizeof(Atom));
-		const Atom property = request[2 * index + 1];
+		for (UInt32 index = 0; index < numRequests; ++index) {
+			// get request info
+			const Atom* request = reinterpret_cast<const Atom*>(data.data());
+			const Atom target   = request[2 * index + 0];
+			const Atom property = request[2 * index + 1];
 
-		// handle target
+			// handle target
-		if (property != None) {
+			if (property != None) {
-			if (!sendClipboardData(id, requestor, target, property, time)) {
+				if (!sendClipboardData(id, requestor, target, property, time)) {
-				// couldn't handle target.  change property to None.
+					// couldn't handle target.  change property to None.
-				const Atom none = None;
+					const Atom none = None;
-				data.replace((2 * index + 1) * sizeof(Atom), sizeof(Atom),
+					data.replace((2 * index + 1) * sizeof(Atom), sizeof(Atom),
 								reinterpret_cast<const char*>(&none),
 								sizeof(none));
-				updated = true;
+					updated = true;
-			}
+				}
-			else {
+				else {
-				success = true;
+					success = true;
+				}
 			}
 		}
-	}
 
-	// update property if we changed it
+		// update property if we changed it
-	if (updated) {
+		if (updated) {
-		// FIXME -- handle Alloc errors (by returning false)
+			// FIXME -- handle Alloc errors (by returning false)
-		XChangeProperty(m_display, requestor, property,
+			XChangeProperty(m_display, requestor, property,
 								m_atomAtomPair,
 								8 * sizeof(Atom),
 								PropModeReplace,
 								reinterpret_cast<const unsigned char*>(
 									data.data()),
 								data.length());
+		}
 	}
 
-	// send notify if any format was successful
+	// send notify
-	if (success) {
+	sendNotify(requestor, m_atomClipboard[id], m_atomMultiple,
-		sendNotify(id, requestor, m_atomMultiple,
 								success ? property : None, time);
-		return true;
+	return success;
-	}
-
-	return false;
 }
 
 bool					CXWindowsScreen::sendClipboardTargets(
@@ -902,20 +915,99 @@ bool					CXWindowsScreen::sendClipboardTimestamp(
 }
 
 void					CXWindowsScreen::sendNotify(
-								ClipboardID id, Window requestor,
+								Window requestor, Atom selection,
 								Atom target, Atom property, Time time)
 {
 	XEvent event;
 	event.xselection.type      = SelectionNotify;
 	event.xselection.display   = m_display;
 	event.xselection.requestor = requestor;
-	event.xselection.selection = m_atomClipboard[id];
+	event.xselection.selection = selection;
 	event.xselection.target    = target;
 	event.xselection.property  = property;
 	event.xselection.time      = time;
 	XSendEvent(m_display, requestor, False, 0, &event);
 }
 
+bool					CXWindowsScreen::wasOwnedAtTime(
+								ClipboardID id, Window window, Time time) const
+{
+	const CClipboardInfo& clipboard = m_clipboards[id];
+
+	// not owned if we've never owned the selection
+	if (clipboard.m_gotClipboard == CurrentTime)
+		return false;
+
+	// if time is CurrentTime then return true if we still own the
+	// selection and false if we do not.  else if we still own the
+	// selection then get the current time, otherwise use
+	// m_lostClipboard as the end time.
+	Time lost = clipboard.m_lostClipboard;
+	if (lost == CurrentTime)
+		if (time == CurrentTime)
+			return true;
+		else
+			lost = getCurrentTimeNoLock(window);
+	else
+		if (time == CurrentTime)
+			return false;
+
+	// compare time to range
+	Time duration = clipboard.m_lostClipboard - clipboard.m_gotClipboard;
+	Time when     = time - clipboard.m_gotClipboard;
+	return (/*when >= 0 &&*/ when < duration);
+}
+
+Time					CXWindowsScreen::getCurrentTime(Window window) const
+{
+	CLock lock(&m_mutex);
+	return getCurrentTimeNoLock(window);
+}
+
+Time					CXWindowsScreen::getCurrentTimeNoLock(
+								Window window) const
+{
+	// do a zero-length append to get the current time
+	unsigned char dummy;
+	XChangeProperty(m_display, window, m_atomSynergyTime,
+								m_atomInteger, 8,
+								PropModeAppend,
+								&dummy, 0);
+
+	// look for property notify events with the following
+	CPropertyNotifyInfo filter;
+	filter.m_window   = window;
+	filter.m_property = m_atomSynergyTime;
+
+	// wait for reply
+	XEvent xevent;
+	while (XCheckIfEvent(m_display, &xevent,
+								&CXWindowsScreen::findPropertyNotify,
+								(XPointer)&filter) != True) {
+		// wait a bit
+		CThread::sleep(0.05);
+	}
+	assert(xevent.type             == PropertyNotify);
+	assert(xevent.xproperty.window == window);
+	assert(xevent.xproperty.atom   == m_atomSynergyTime);
+
+	return xevent.xproperty.time;
+}
+
+
+//
+// CXWindowsScreen::CClipboardInfo
+//
+
+CXWindowsScreen::CClipboardInfo::CClipboardInfo() :
+								m_clipboard(),
+								m_gotClipboard(CurrentTime),
+								m_lostClipboard(CurrentTime),
+								m_requests()
+{
+	// do nothing
+}
+
 
 //
 // CXWindowsScreen::CDisplayLock

synergy/CXWindowsScreen.h

@@ -91,6 +91,9 @@ class CXWindowsScreen {
 	// terminate a selection request
 	void				destroyClipboardRequest(Window window);
 
+	// get the current server time
+	Time				getCurrentTime(Window) const;
+
 	// called by openDisplay() to allow subclasses to prepare the display
 	virtual void		onOpenDisplay() = 0;
 
@@ -135,11 +138,16 @@ class CXWindowsScreen {
 								Atom property, Time time);
 	bool				sendClipboardTimestamp(ClipboardID, Window requestor,
 								Atom property, Time time);
-	void				sendNotify(ClipboardID, Window requestor,
+	void				sendNotify(Window requestor, Atom selection,
 								Atom target, Atom property, Time time);
+	bool				wasOwnedAtTime(ClipboardID, Window, Time) const;
+	Time				getCurrentTimeNoLock(Window) const;
 
   private:
-	class ClipboardInfo {
+	class CClipboardInfo {
+	public:
+		CClipboardInfo();
+
 	public:
 		// the contents of the clipboard
 		CClipboard		m_clipboard;
@@ -171,20 +179,10 @@ class CXWindowsScreen {
 	Atom				m_atomText;
 	Atom				m_atomCompoundText;
 	Atom				m_atomClipboard[kClipboardEnd];
+	Atom				m_atomSynergyTime;
 
 	// clipboard info
-	ClipboardInfo		m_clipboards[kClipboardEnd];
+	CClipboardInfo		m_clipboards[kClipboardEnd];
-/*
-	// the contents of our selection
-	CClipboard			m_clipboard;
-
-	// when we got the selection and when we lost it
-	Time				m_gotClipboard;
-	Time				m_lostClipboard;
-
-	// the request queues
-	CRequestMap			m_requests;
-*/
 
 	// X is not thread safe
 	CMutex				m_mutex;