Delete the property returned by SelectionNotify as per ICCCM spec

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/message_loop/message_pump_x11.h"
 #include "base/run_loop.h"
 #include "ui/base/x/selection_utils.h"
 #include "ui/base/x/x11_util.h"
@@ skipping 51 matching lines @@
       run_loop.QuitClosure(),
       base::TimeDelta::FromMilliseconds(kMaxWaitTimeForClipboardResponse));
 
   PendingRequest pending_request(target, run_loop.QuitClosure());
   pending_requests_.push_back(&pending_request);
   run_loop.Run();
   DCHECK(!pending_requests_.empty());
   DCHECK_EQ(&pending_request, pending_requests_.back());
   pending_requests_.pop_back();
 
-  if (pending_request.returned_property != property_to_set)
-    return false;
-
-  return ui::GetRawBytesOfProperty(x_window_, pending_request.returned_property,
-                                   out_data, out_data_bytes, out_data_items,
-                                   out_type);
+  bool success = false;
+  if (pending_request.returned_property == property_to_set) {
+    success =  ui::GetRawBytesOfProperty(x_window_,
+                                         pending_request.returned_property,
+                                         out_data, out_data_bytes,
+                                         out_data_items, out_type);
+  }
+  XDeleteProperty(x_display_, x_window_, pending_request.returned_property);

[sadrul, 2014/04/15 16:20:15]

Should this be inside the if? and/or, should this check that returned_property
is not None?

[pkotwicz, 2014/04/15 16:57:55]

I checked and XDeleteProperty() does not do anything bad if returned_property is
None

[sadrul, 2014/04/15 18:04:30]

The man page claims XDeleteProperty can generate BadAtom errors. So I would add
a check for None

+  return success;
 }
 
 SelectionData SelectionRequestor::RequestAndWaitForTypes(
     const std::vector< ::Atom>& types) {
   for (std::vector< ::Atom>::const_iterator it = types.begin();
        it != types.end(); ++it) {
     scoped_refptr<base::RefCountedMemory> data;
     size_t data_bytes = 0;
     ::Atom type = None;
     if (PerformBlockingConvertSelection(*it,
@@ skipping 23 matching lines @@
       if (request->target != event.target)
         continue;
       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)
+  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.
+    XDeleteProperty(x_display_, x_window_, event.property);
     return;
+  }
 
   request_notified->returned_property = event.property;
   request_notified->returned = true;
   request_notified->quit_closure.Run();
 }
 
 SelectionRequestor::PendingRequest::PendingRequest(Atom target,
                                                    base::Closure quit_closure)
     : target(target),
       quit_closure(quit_closure),
       returned_property(None),
       returned(false) {
 }
 
 SelectionRequestor::PendingRequest::~PendingRequest() {
 }
 
 }  // namespace ui