Commit f6658bc4 authored by Tom Anderson's avatar Tom Anderson Committed by Chromium LUCI CQ

Avoid spinning a nested message loop for X11 clipboard

BUG=443355,1138143,1161141,1161143,1161144,1161145,1161146,1161147,1161149,1161151,1161152

Change-Id: I5c95a9d066683d18f344d694e517274e3ef7ccb4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2622521Reviewed-by: default avatarScott Violet <sky@chromium.org>
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#844318}
parent f84332a8
...@@ -7,7 +7,6 @@ ...@@ -7,7 +7,6 @@
#include <algorithm> #include <algorithm>
#include "base/memory/ref_counted_memory.h" #include "base/memory/ref_counted_memory.h"
#include "base/run_loop.h"
#include "ui/base/x/selection_owner.h" #include "ui/base/x/selection_owner.h"
#include "ui/base/x/selection_utils.h" #include "ui/base/x/selection_utils.h"
#include "ui/base/x/x11_util.h" #include "ui/base/x/x11_util.h"
...@@ -28,7 +27,7 @@ const char kChromeSelection[] = "CHROME_SELECTION"; ...@@ -28,7 +27,7 @@ const char kChromeSelection[] = "CHROME_SELECTION";
const int KSelectionRequestorTimerPeriodMs = 100; const int KSelectionRequestorTimerPeriodMs = 100;
// The amount of time to wait for a request to complete before aborting it. // The amount of time to wait for a request to complete before aborting it.
const int kRequestTimeoutMs = 10000; const int kRequestTimeoutMs = 1000;
static_assert(KSelectionRequestorTimerPeriodMs <= kRequestTimeoutMs, static_assert(KSelectionRequestorTimerPeriodMs <= kRequestTimeoutMs,
"timer period must be <= request timeout"); "timer period must be <= request timeout");
...@@ -237,29 +236,30 @@ void SelectionRequestor::ConvertSelectionForCurrentRequest() { ...@@ -237,29 +236,30 @@ void SelectionRequestor::ConvertSelectionForCurrentRequest() {
} }
void SelectionRequestor::BlockTillSelectionNotifyForRequest(Request* request) { void SelectionRequestor::BlockTillSelectionNotifyForRequest(Request* request) {
if (X11EventSource::HasInstance()) { auto* connection = x11::Connection::Get();
if (!abort_timer_.IsRunning()) { auto& events = connection->events();
abort_timer_.Start( size_t i = 0;
FROM_HERE, while (!request->completed && request->timeout > base::TimeTicks::Now()) {
base::TimeDelta::FromMilliseconds(KSelectionRequestorTimerPeriodMs), connection->Flush();
this, &SelectionRequestor::AbortStaleRequests); connection->ReadResponses();
size_t events_size = events.size();
for (; i < events_size; ++i) {
auto& event = events[i];
if (auto* notify = event.As<x11::SelectionNotifyEvent>()) {
if (notify->requestor == x_window_) {
OnSelectionNotify(*notify);
event = x11::Event();
}
} else if (auto* prop = event.As<x11::PropertyNotifyEvent>()) {
if (CanDispatchPropertyEvent(*prop)) {
OnPropertyEvent(*prop);
event = x11::Event();
}
}
} }
DCHECK_EQ(events_size, events.size());
base::RunLoop run_loop(base::RunLoop::Type::kNestableTasksAllowed);
request->quit_closure = run_loop.QuitClosure();
run_loop.Run();
// We cannot put logic to process the next request here because the RunLoop
// might be nested. For instance, request 'B' may start a RunLoop while the
// RunLoop for request 'A' is running. It is not possible to end the RunLoop
// for request 'A' without first ending the RunLoop for request 'B'.
} else {
// This occurs if PerformBlockingConvertSelection() is called during
// shutdown and the X11EventSource has already been destroyed.
auto* conn = x11::Connection::Get();
while (!request->completed && request->timeout > base::TimeTicks::Now())
conn->DispatchAll();
} }
AbortStaleRequests();
} }
SelectionRequestor::Request* SelectionRequestor::GetCurrentRequest() { SelectionRequestor::Request* SelectionRequestor::GetCurrentRequest() {
......
...@@ -101,7 +101,8 @@ void PerformBlockingConvertSelection(SelectionRequestor* requestor, ...@@ -101,7 +101,8 @@ void PerformBlockingConvertSelection(SelectionRequestor* requestor,
// Test that SelectionRequestor correctly handles receiving a request while it // Test that SelectionRequestor correctly handles receiving a request while it
// is processing another request. // is processing another request.
TEST_F(SelectionRequestorTest, NestedRequests) { // TODO(https://crbug.com/443355): Reenable once clipboard interface is async.
TEST_F(SelectionRequestorTest, DISABLED_NestedRequests) {
// Assume that |selection| will have no owner. If there is an owner, the owner // Assume that |selection| will have no owner. If there is an owner, the owner
// will set the property passed into the XConvertSelection() request which is // will set the property passed into the XConvertSelection() request which is
// undesirable. // undesirable.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment