SelectionOwner: add support for TIMESTAMP target

ui/base/x/selection_owner.cc

Index: ui/base/x/selection_owner.cc
diff --git a/ui/base/x/selection_owner.cc b/ui/base/x/selection_owner.cc
index 170f32e19b2c0ea71e90bdb53e9386c5aea26fb0..8da95c1a915bfeb7b01d1ae268be84010987e9fe 100644
--- a/ui/base/x/selection_owner.cc
+++ b/ui/base/x/selection_owner.cc
@@ -22,15 +22,19 @@ const char kIncr[] = "INCR";
 const char kMultiple[] = "MULTIPLE";
 const char kSaveTargets[] = "SAVE_TARGETS";
 const char kTargets[] = "TARGETS";
+const char kTimestamp[] = "TIMESTAMP";
 
-const char* kAtomsToCache[] = {
-  kAtomPair,
-  kIncr,
-  kMultiple,
-  kSaveTargets,
-  kTargets,
-  NULL
-};
+// Used in conjunction with GetCurrentTimeFromX.
+const char kDummyTimestampProperty[] = "CHROMIUM_TIMESTAMP_PROPERTY";
+
+const char* kAtomsToCache[] = {kAtomPair,
+                               kIncr,
+                               kMultiple,
+                               kSaveTargets,
+                               kTargets,
+                               kTimestamp,
+                               kDummyTimestampProperty,
+                               NULL};
 
 // The period of |incremental_transfer_abort_timer_|. Arbitrary but must be <=
 // than kIncrementalTransferTimeoutMs.
@@ -53,6 +57,32 @@ size_t GetMaxRequestSize(XDisplay* display) {
                   std::max(static_cast<long>(0), max_size));
 }
 
+// Get the value of CurrentTime from X. There's no easy way to do this, so like
+// GTK, we do this by triggering a spurious property change and inspecting the
+// resulting event.
+Time GetCurrentTimeFromX(XDisplay* display, XID window, XAtom property) {
+  unsigned char c = 0;
+  XChangeProperty(display, window, property, property, 8, PropModeReplace, &c,
+                  1);
+  struct ArgData {
+    // Target window and atom to wait for.
+    XID window;
+    XAtom atom;
+  } args = {window, property};
+  auto predicate = [](XDisplay* display, XEvent* event, XPointer arg) -> Bool {
+    ArgData* data = reinterpret_cast<ArgData*>(arg);
+    return (event->type == PropertyNotify &&
+            event->xproperty.window == data->window &&
+            event->xproperty.atom == data->atom)
+               ? True
+               : False;
+
+  };
+  XEvent event;
+  XIfEvent(display, &event, predicate, reinterpret_cast<XPointer>(&args));
+  return event.xproperty.time;

[sadrul, 2016/04/19 16:11:52]

How does things break when we use CurrentTime?

We have had enough fun with events being processed out of order, or
disappearing. I wouldn't want to complicate things farther. I am tempted to just
live with whatever brokenness we get with CurrentTime (or even
base::Time::Now()/UnixEpoch() etc.) instead.

[dcheng, 2016/04/19 17:24:15]

Just using CurrentTime is unlikely to actually resolve the issue seen by the bug
reporters: VMWare uses the TIMESTAMP target to determine which selection was
more recently set and needs to be forwarded to a VM.
base::Time::Now() / UnixEpoch() have similar issues. This seems like a pretty
reasonable complaint to me, especially since it used to work just fine before we
switched to Aura =)

Processing events out of order is kind of lame, but in this particular case,
it's pretty narrowly scoped: the only event we should pull out of the event
queue is the one we triggered ourselves. The alternative, as I mentioned, is to
save the timestamp off events in X11EventSource or the like.

[Elliot Glaysher, 2016/04/19 17:30:36]

Sadly, I wouldn't rely on that. We currently have a bug open where we're getting
MapNotify messages that don't correspond to us calling XMapWindow(). Saving the
timestamps off X11EventSource sounds significantly safer.

[dcheng, 2016/04/19 17:44:11]

Is the concern here that some other app will XChangeProprety on our dummy
property and cause us to mistakenly pull the wrong event out of the queue? Or
that we'll just hang like the MapNotify bug (assuming I'm looking at the right
one)?

+}
+
 // Gets the value of an atom pair array property. On success, true is returned
 // and the value is stored in |value|.
 bool GetAtomPairArrayProperty(XID window,
@@ -122,7 +152,12 @@ void SelectionOwner::RetrieveTargets(std::vector<XAtom>* targets) {
 
 void SelectionOwner::TakeOwnershipOfSelection(
     const SelectionFormatMap& data) {
-  XSetSelectionOwner(x_display_, selection_name_, x_window_, CurrentTime);
+  // Note that we don't just use CurrentTime, since we need to return an actual
+  // timestamp for the TIMESTAMP target later.
+  acquired_selection_timestamp_ = GetCurrentTimeFromX(
+      x_display_, x_window_, atom_cache_.GetAtom(kDummyTimestampProperty));
+  XSetSelectionOwner(x_display_, selection_name_, x_window_,
+                     acquired_selection_timestamp_);
 
   if (XGetSelectionOwner(x_display_, selection_name_) == x_window_) {
     // The X server agrees that we are the selection owner. Commit our data.
@@ -219,14 +254,21 @@ bool SelectionOwner::ProcessTarget(XAtom target,
   XAtom multiple_atom = atom_cache_.GetAtom(kMultiple);
   XAtom save_targets_atom = atom_cache_.GetAtom(kSaveTargets);
   XAtom targets_atom = atom_cache_.GetAtom(kTargets);
+  XAtom timestamp_atom = atom_cache_.GetAtom(kTimestamp);
 
   if (target == multiple_atom || target == save_targets_atom)
     return false;
 
-  if (target == targets_atom) {
+  if (target == timestamp_atom) {
+    XChangeProperty(
+        x_display_, requestor, property, XA_INTEGER, 32, PropModeReplace,
+        reinterpret_cast<unsigned char*>(&acquired_selection_timestamp_), 1);
+    return true;
+  } else if (target == targets_atom) {
     // We have been asked for TARGETS. Send an atom array back with the data
     // types we support.
     std::vector<XAtom> targets;
+    targets.push_back(timestamp_atom);
     targets.push_back(targets_atom);
     targets.push_back(save_targets_atom);
     targets.push_back(multiple_atom);