Process SelectionRequestor::PerformBlockingConvertSelection() requests one at a time

ui/base/x/selection_requestor.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_requestor.h"
 
 #include "base/run_loop.h"
 #include "ui/base/x/selection_utils.h"
 #include "ui/base/x/x11_util.h"
 #include "ui/events/platform/platform_event_dispatcher.h"
 #include "ui/events/platform/platform_event_source.h"
 #include "ui/gfx/x/x11_types.h"
 
 namespace ui {
 
 namespace {
 
-const char kChromeSelection[] = "CHROME_SELECTION";
+// Properties to be used in XConvertSelection() requests. The number of
+// properties is arbitrary.
+const char* kChromeSelectionProperties[] = {
+    "CHROME_SELECTION0",
+    "CHROME_SELECTION1",
+    "CHROME_SELECTION2",
+    "CHROME_SELECTION3",
+    "CHROME_SELECTION4"
+};
 
 const char* kAtomsToCache[] = {
-  kChromeSelection,
+  kChromeSelectionProperties[0],
+  kChromeSelectionProperties[1],
+  kChromeSelectionProperties[2],
+  kChromeSelectionProperties[3],
+  kChromeSelectionProperties[4],
   NULL
 };
 
 }  // namespace
 
 SelectionRequestor::SelectionRequestor(Display* x_display,
                                        Window x_window,
-                                       Atom selection_name,
                                        PlatformEventDispatcher* dispatcher)
     : x_display_(x_display),
       x_window_(x_window),
-      selection_name_(selection_name),
       dispatcher_(dispatcher),
+      property_pool_(kChromeSelectionProperties,
+                     kChromeSelectionProperties
+                         + arraysize(kChromeSelectionProperties)),
       atom_cache_(x_display_, kAtomsToCache) {
 }
 
 SelectionRequestor::~SelectionRequestor() {}
 
 bool SelectionRequestor::PerformBlockingConvertSelection(
+    Atom selection,
     Atom target,
     scoped_refptr<base::RefCountedMemory>* out_data,
     size_t* out_data_items,
     Atom* out_type) {
-  // The name of the property that we are either:
-  // - Passing as a parameter with the XConvertSelection() request.
-  // OR
-  // - Asking the selection owner to set on |x_window_|.
-  Atom property = atom_cache_.GetAtom(kChromeSelection);
+  // Find a property for XConvertSelection() to use. The selection owner will
+  // set |property| on |xwindow_| with the data for the requested target. If
+  // there is no available property, make the request fail.
+  if (property_pool_.empty())
+    return false;
 
-  XConvertSelection(x_display_,
-                    selection_name_,
-                    target,
-                    property,
-                    x_window_,
-                    CurrentTime);
-
-  // Now that we've thrown our message off to the X11 server, we block waiting
-  // for a response.
-  PendingRequest pending_request(target);
-  BlockTillSelectionNotifyForRequest(&pending_request);
-
-  bool success = false;
-  if (pending_request.returned_property == property) {
-    success =  ui::GetRawBytesOfProperty(x_window_,
-                                         pending_request.returned_property,
-                                         out_data, out_data_items, out_type);
-  }
-  if (pending_request.returned_property != None)
-    XDeleteProperty(x_display_, x_window_, pending_request.returned_property);
+  const char* property_name = property_pool_.back();
+  property_pool_.pop_back();

[Daniel Erat, 2014/07/15 01:04:09]

with the disclaimer that i don't know much about selections, and i didn't know
anything about INCR until just now: what's the reason for having multiple atoms
here? it looks like you pop the back one off, use it synchronously, and then put
it back at the end of the list. if i'm not misreading the code, won't this
result in both this method and PerformBlockingConvertSelectionWithParameter()
always only using the last atom? if so, what's the reason for having multiple
atoms?

[pkotwicz, 2014/07/15 02:07:24]

The problem is that there are callers of clipboard functions from posted tasks.
So a second PerformBlockingConvertSelection() call can be made before the first
one completes.
This is the case with chrome.bookmarkManagerPrivate.canPaste

[Daniel Erat, 2014/07/15 03:32:01]

oh. is the nested message loop in BlockTillSelectionNotifyForRequest() where the
second call can get run?

+  bool success = PerformBlockingConvertSelectionImpl(selection, target,
+      property_name, out_data, out_data_items, out_type);
+  property_pool_.push_back(property_name);
   return success;
 }
 
 void SelectionRequestor::PerformBlockingConvertSelectionWithParameter(
+    Atom selection,
     Atom target,
     const std::vector< ::Atom>& parameter) {
-  SetAtomArrayProperty(x_window_, kChromeSelection, "ATOM", parameter);
-  PerformBlockingConvertSelection(target, NULL, NULL, NULL);
+  if (property_pool_.empty())
+    return;
+  const char* property_name = property_pool_.back();
+  property_pool_.pop_back();
+  SetAtomArrayProperty(x_window_, property_name, "ATOM", parameter);
+  PerformBlockingConvertSelectionImpl(selection, target, property_name, NULL,
+      NULL, NULL);
+  property_pool_.push_back(property_name);
 }
 
 SelectionData SelectionRequestor::RequestAndWaitForTypes(
+    Atom selection,
     const std::vector< ::Atom>& types) {
   for (std::vector< ::Atom>::const_iterator it = types.begin();
        it != types.end(); ++it) {
     scoped_refptr<base::RefCountedMemory> data;
     ::Atom type = None;
-    if (PerformBlockingConvertSelection(*it,
+    if (PerformBlockingConvertSelection(selection,
+                                        *it,
                                         &data,
                                         NULL,
                                         &type) &&
         type == *it) {
       return SelectionData(type, data);
     }
   }
 
   return SelectionData();
 }
 
 void SelectionRequestor::OnSelectionNotify(const XSelectionEvent& event) {
   // Find the PendingRequest for the corresponding XConvertSelection call. If
   // there are multiple pending requests on the same target, satisfy them in
   // FIFO order.
   PendingRequest* request_notified = NULL;
-  if (selection_name_ == event.selection) {
-    for (std::list<PendingRequest*>::iterator iter = pending_requests_.begin();
-         iter != pending_requests_.end(); ++iter) {
-      PendingRequest* request = *iter;
-      if (request->returned)
-        continue;
-      if (request->target != event.target)
-        continue;
+  for (std::list<PendingRequest*>::iterator iter = pending_requests_.begin();
+       iter != pending_requests_.end(); ++iter) {
+    PendingRequest* request = *iter;
+    if (!request->returned &&
+        request->selection == event.selection &&
+        request->target == event.target) {
       request_notified = request;
       break;
     }
   }
 
   // This event doesn't correspond to any XConvertSelection calls that we
   // issued in PerformBlockingConvertSelection. This shouldn't happen, but any
   // client can send any message, so it can happen.
   if (!request_notified) {
     // ICCCM requires us to delete the property passed into SelectionNotify. If
     // |request_notified| is true, the property will be deleted when the run
     // loop has quit.
     if (event.property != None)
       XDeleteProperty(x_display_, x_window_, event.property);
     return;
   }
 
   request_notified->returned_property = event.property;
   request_notified->returned = true;
 
   if (!request_notified->quit_closure.is_null())
     request_notified->quit_closure.Run();
 }
 
+bool SelectionRequestor::PerformBlockingConvertSelectionImpl(
+    Atom selection,
+    Atom target,
+    const char* property_name,
+    scoped_refptr<base::RefCountedMemory>* out_data,
+    size_t* out_data_items,
+    Atom* out_type) {
+  Atom property = atom_cache_.GetAtom(property_name);
+  XConvertSelection(x_display_,
+                    selection,
+                    target,
+                    property,
+                    x_window_,
+                    CurrentTime);
+
+  // Now that we've thrown our message off to the X11 server, we block waiting
+  // for a response.
+  PendingRequest pending_request(selection, target);
+  BlockTillSelectionNotifyForRequest(&pending_request);
+
+  bool success = false;
+  if (pending_request.returned_property == property) {
+    success = ui::GetRawBytesOfProperty(x_window_,
+                                        pending_request.returned_property,
+                                        out_data, out_data_items, out_type);
+  }
+  if (pending_request.returned_property != None)
+    XDeleteProperty(x_display_, x_window_, pending_request.returned_property);
+  return success;
+}
+
 void SelectionRequestor::BlockTillSelectionNotifyForRequest(
     PendingRequest* request) {
   pending_requests_.push_back(request);
 
   const int kMaxWaitTimeForClipboardResponse = 300;
   if (PlatformEventSource::GetInstance()) {
     base::MessageLoopForUI* loop = base::MessageLoopForUI::current();
     base::MessageLoop::ScopedNestableTaskAllower allow_nested(loop);
     base::RunLoop run_loop;
 
@@ skipping 18 matching lines @@
       if (wait_time.InMilliseconds() > kMaxWaitTimeForClipboardResponse)
         break;
     }
   }
 
   DCHECK(!pending_requests_.empty());
   DCHECK_EQ(request, pending_requests_.back());
   pending_requests_.pop_back();
 }
 
-SelectionRequestor::PendingRequest::PendingRequest(Atom target)
-    : target(target),
+SelectionRequestor::PendingRequest::PendingRequest(Atom requested_selection,
+                                                   Atom requested_target)
+    : selection(requested_selection),
+      target(requested_target),
       returned_property(None),
       returned(false) {
 }
 
 SelectionRequestor::PendingRequest::~PendingRequest() {
 }
 
 }  // namespace ui