SelectionOwner: add support for TIMESTAMP target

ui/base/x/selection_owner.cc

 // Copyright (c) 2013 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.
 
 #include "ui/base/x/selection_owner.h"
 
 #include <algorithm>
 #include <X11/Xlib.h>
 #include <X11/Xatom.h>
 
 #include "base/logging.h"
 #include "ui/base/x/selection_utils.h"
 #include "ui/base/x/x11_foreign_window_manager.h"
 #include "ui/base/x/x11_util.h"
 
 namespace ui {
 
 namespace {
 
 const char kAtomPair[] = "ATOM_PAIR";
 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.
 const int kTimerPeriodMs = 1000;
 
 // The amount of time to wait for the selection requestor to process the data
 // sent by the selection owner before aborting an incremental data transfer.
 const int kIncrementalTransferTimeoutMs = 10000;
 
 static_assert(kTimerPeriodMs <= kIncrementalTransferTimeoutMs,
               "timer period must be <= transfer timeout");
 
 // Returns a conservative max size of the data we can pass into
 // XChangeProperty(). Copied from GTK.
 size_t GetMaxRequestSize(XDisplay* display) {
   long extended_max_size = XExtendedMaxRequestSize(display);
   long max_size =
       (extended_max_size ? extended_max_size : XMaxRequestSize(display)) - 100;
   return std::min(static_cast<long>(0x40000),
                   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,
                               XAtom property,
                               std::vector<std::pair<XAtom,XAtom> >* value) {
   XAtom type = None;
   int format = 0;  // size in bits of each item in 'property'
   unsigned long num_items = 0;
   unsigned char* properties = NULL;
   unsigned long remaining_bytes = 0;
@@ skipping 49 matching lines @@
 
 void SelectionOwner::RetrieveTargets(std::vector<XAtom>* targets) {
   for (SelectionFormatMap::const_iterator it = format_map_.begin();
        it != format_map_.end(); ++it) {
     targets->push_back(it->first);
   }
 }
 
 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.
     format_map_ = data;
   }
 }
 
 void SelectionOwner::ClearSelectionOwner() {
   XSetSelectionOwner(x_display_, selection_name_, None, CurrentTime);
   format_map_ = SelectionFormatMap();
@@ skipping 76 matching lines @@
   if (!it->data.get())
     CompleteIncrementalTransfer(it);
 }
 
 bool SelectionOwner::ProcessTarget(XAtom target,
                                    XID requestor,
                                    XAtom property) {
   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);
     RetrieveTargets(&targets);
 
     XChangeProperty(x_display_, requestor, property, XA_ATOM, 32,
                     PropModeReplace,
                     reinterpret_cast<unsigned char*>(&targets.front()),
                     targets.size());
     return true;
@@ skipping 136 matching lines @@
       foreign_window_manager_id(foreign_window_manager_id) {
 }
 
 SelectionOwner::IncrementalTransfer::IncrementalTransfer(
     const IncrementalTransfer& other) = default;
 
 SelectionOwner::IncrementalTransfer::~IncrementalTransfer() {
 }
 
 }  // namespace ui