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"
+#include "ui/events/platform/x11/x11_event_source.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
-};
+const char* kAtomsToCache[] = {kAtomPair, kIncr,      kMultiple, kSaveTargets,
+                               kTargets,  kTimestamp, 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,
@@ skipping 71 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) {
+  // Save the last server timestamp seen from X, to satisfy requests for the
+  // TIMESTAMP target later…
+  acquired_selection_timestamp_ =
+      X11EventSource::GetInstance()->last_seen_server_time();

[sadrul, 2016/04/20 17:58:43]

Can this use something else (like CurrentTime) if X11EventSource doesn't exist?
That way the other tests that don't care don't need to create an X11EventSource
thingy

[dcheng, 2016/04/20 18:55:43]

=/

I could, but I'm not a fan of adding checks for conditions that are only true in
tests. I'd strongly prefer this to break in production binaries if the
preconditions I'm assuming (the existence of an event source) are no longer
true.

+  // …but always pass CurrentTime to XSetSelectionOwner to increase the chances
+  // of this succeeding.

[sadrul, 2016/04/20 17:58:43]

um, lol?

[dcheng, 2016/04/20 18:55:42]

I couldn't figure out why, but without doing this, the omnibox copy/cut tests
didn't work. This seemed like the most expedient solution =/

   XSetSelectionOwner(x_display_, selection_name_, x_window_, CurrentTime);
 
   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);
@@ skipping 77 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) {

[sadrul, 2016/04/20 17:58:43]

Don't need the else.

[dcheng, 2016/04/20 18:55:43]

Done. I updated the rest of this function to be consistent as well.

     // 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