Implement clipboard for Chromoting Linux hosts.

remoting/host/linux/x_server_clipboard.h

+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Don't include this file from any .h files because it pulls in some X headers.
+
+#ifndef REMOTING_HOST_LINUX_X_SERVER_CLIPBOARD_H_
+#define REMOTING_HOST_LINUX_X_SERVER_CLIPBOARD_H_
+
+#include <X11/Xatom.h>
+#include <X11/Xlib.h>
+
+#include <map>
+#include <set>
+#include <string>
+
+#include "base/basictypes.h"
+#include "base/callback_forward.h"
+#include "base/timer.h"
+
+namespace remoting {
+
+// A class to allow manipulation of the X clipboard, using only X API calls.
+class XServerClipboard {
+ public:
+  typedef base::Callback<void(const std::string& mime_type,
+                              const std::string& data)>
+    ClipboardChangedCallback;
+
+  XServerClipboard();
+  ~XServerClipboard();
+
+  // The caller must ensure |display| is still valid whenever any other
+  // methods are called on this object.
+  void Init(Display* display, const ClipboardChangedCallback& callback);
+
+  // Copy data to the X Clipboard.  This acquires ownership of the
+  // PRIMARY and CLIPBOARD selection.

[Wez, 2012/09/20 23:37:46]

nit: selections

[Lambros, 2012/09/21 17:05:57]

Done.

+  void SetClipboard(const std::string& mime_type, const std::string& data);
+
+  // This method allows this object to be hooked up to the program's main event
+  // loop.  It should be invoked by the caller for each X11 event.  This class
+  // is not designed to be thread-safe, so all its methods should ideally be

[Wez, 2012/09/20 23:37:46]

Surely you mean "must", not "should ideally"?

[Lambros, 2012/09/21 17:05:57]

Done.

+  // called on the same thread.  In practice, this will mean calling all these
+  // methods on the application's main event-processing thread, which has the
+  // added benefit that all calls to Xlib (including XPending/XNextEvent) occur
+  // on one thread (per display).

[Wez, 2012/09/20 23:37:46]

This comment seems to imply that the caller might opt to split calls across
different threads but it's not clear to me what synchronization the caller would
need to do to make this safe, and since we don't intend to use it multi-threaded
- let's just state that this class isn't designed for multi-threaded use?

[Lambros, 2012/09/21 17:05:57]

You're right, no point in being long-winded about this :) Reworded.

+  void ProcessXEvent(XEvent* event);
+
+ private:
+  // Check the state of the X PRIMARY and CLIPBOARD selections and queues a

[Wez, 2012/09/20 23:37:46]

nit: queues -> queue

[Lambros, 2012/09/21 17:05:57]

Done.

+  // notification to the registered ClipboardChangedCallback if the content has
+  // changed since the previous call.
+  // |selection| specifies the selection to be checked (either PRIMARY or
+  // CLIPBOARD), or None to check both the CLIPBOARD and PRIMARY selections.

[Wez, 2012/09/20 23:37:46]

nit: It looked to me like the implementation actually just checks PRIMARY if
None is supplied?

[Lambros, 2012/09/21 17:05:57]

Removed comment.  This is just an event-handler now.

+  // Other values are treated as invalid and ignored.
+  // If |timestamp| is non-zero, it is assumed to come from an
+  // XFixesSelectionNotify event, and is recorded as the current selection
+  // timestamp and treated as new (when comparing with timestamps from incoming
+  // SelectionNotify events).

[Wez, 2012/09/20 23:37:46]

I still don't understand this; if you're treating the event as "new" then you
don't need a timestamp to compare other events to, surely?

+  void GetSelections(Atom selection, Time timestamp);
+
+  // This stops the GetSelections timer when the process of getting a selection
+  // is complete.

[Wez, 2012/09/20 23:37:46]

There's no mention of a timer in the comment for GetSelections, though?

I'd suggest adding a line to the GetSelections() comment stating:
We ignore new GetSelections() requests while we're processing a previous
request, unless the earlier request appears to have stalled, e.g. the peer app
is not responding.

Then change this comment to something like:
Called when the current GetSelections request completes, either having processed
new text, or having ignored the new content, e.g. because it's text we set
ourselves, or it's not text at all.

[Lambros, 2012/09/21 17:05:57]

Removed this method - it's now just one line of code, used in only two places. 
No point in having a method for this.

+  void FinishGetSelections();
+
+  void OnPropertyNotify(XEvent* event);
+  void OnSelectionNotify(XEvent* event);
+  void OnSelectionRequest(XEvent* event);
+
+  // Called when the selection owner has replied to a request for information
+  // about a selection.
+  void DoSelectionNotify(XSelectionEvent* event,
+                         Atom type,
+                         int format,
+                         int item_count,
+                         void* data);
+
+  bool HandleSelectionTimestampEvent(XSelectionEvent* event,

[Wez, 2012/09/20 23:37:46]

nit: Add a comment before this block of methods indicating that they return true
if the event completes the current GetSelections() request, false otherwise.

+                                     int format,
+                                     int item_count,
+                                     void* data);
+  bool HandleSelectionTargetsEvent(XSelectionEvent* event,
+                                   int format,
+                                   int item_count,
+                                   void* data);
+  bool HandleSelectionStringEvent(XSelectionEvent* event,
+                                  int format,
+                                  int item_count,
+                                  void* data);
+
+  // Notify the registered callback of new clipboard text.
+  void NotifyClipboardText(const std::string& text);
+
+  bool IsSelectionOwner(Atom selection);
+  void RequestSelectionTimestamp(Atom selection, Time time);

[Wez, 2012/09/20 23:37:46]

nit: Consider adding a comment to this block of methods indicating that they
request details about a selection, triggering the events handled by the methods
above?

+  void RequestSelectionTargets(Atom selection);
+  void RequestSelectionString(Atom selection, Atom target);
+
+  // Assert ownership of the specified |selection|.
+  void AssertSelectionOwnership(Atom selection);
+
+  Display* display_;
+  Window clipboard_window_;
+  bool have_xfixes_;
+  int xfixes_event_base_;
+  int xfixes_error_base_;
+  Atom clipboard_atom_;
+  Atom large_selection_atom_;
+  Atom selection_string_atom_;
+  Atom targets_atom_;
+  Atom timestamp_atom_;
+  Atom utf8_string_atom_;
+  std::set<Atom> selections_owned_;
+  std::map<Atom, Time> selection_own_time_;
+  std::string data_;
+  Atom large_selection_property_;
+  Time current_selection_time_;
+  Atom new_selection_;
+  bool getting_initial_selection_;
+  bool new_cut_text_;
+  bool had_valid_timestamp_;
+  base::TimeTicks get_selections_time_;

[Wez, 2012/09/20 23:37:46]

nit: get_selections_start_time_?

+  ClipboardChangedCallback callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(XServerClipboard);
+};
+
+}  // namespace remoting
+
+#endif  // REMOTING_HOST_LINUX_X_SERVER_CLIPBOARD_H_