[X11] Split PutImage requests into multiple requests if necessary

R=sky

Change-Id: I1befaef494c74797333d6bf02e204bcdc721e752
Bug: 1164669
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2643711
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Auto-Submit: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#846344}

ui/base/x/selection_owner.cc

@@ -39,16 +39,11 @@ const int kIncrementalTransferTimeoutMs = 10000;
 static_assert(KSelectionOwnerTimerPeriodMs <= 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(x11::Connection* connection) {
-  long extended_max_size = connection->extended_max_request_length();
-  long max_size =
-      (extended_max_size ? extended_max_size
-                         : connection->setup().maximum_request_length) -
-      100;
-  return std::min(static_cast<long>(0x40000),
-                  std::max(static_cast<long>(0), max_size));
+size_t GetMaxIncrementalTransferSize() {
+  ssize_t size = x11::Connection::Get()->MaxRequestSizeInBytes();
+  // Conservatively subtract 100 bytes for the GetProperty request, padding etc.
+  DCHECK_GT(size, 100);
+  return std::min(size - 100, 0x100000l);
 }
 
 // Gets the value of an atom pair array property. On success, true is returned
@@ -85,9 +80,7 @@ void SetSelectionOwner(x11::Window window,
 SelectionOwner::SelectionOwner(x11::Connection* connection,
                                x11::Window x_window,
                                x11::Atom selection_name)
-    : x_window_(x_window),
-      selection_name_(selection_name),
-      max_request_size_(GetMaxRequestSize(connection)) {}
+    : x_window_(x_window), selection_name_(selection_name) {}
 
 SelectionOwner::~SelectionOwner() {
   // If we are the selection owner, we need to release the selection so we
@@ -217,7 +210,7 @@ bool SelectionOwner::ProcessTarget(x11::Atom target,
   // Try to find the data type in map.
   auto it = format_map_.find(target);
   if (it != format_map_.end()) {
-    if (it->second->size() > max_request_size_) {
+    if (it->second->size() > GetMaxIncrementalTransferSize()) {
       // We must send the data back in several chunks due to a limitation in
       // the size of X requests. Notify the selection requestor that the data
       // will be sent incrementally by returning data of type "INCR".
@@ -260,7 +253,7 @@ bool SelectionOwner::ProcessTarget(x11::Atom target,
 
 void SelectionOwner::ProcessIncrementalTransfer(IncrementalTransfer* transfer) {
   size_t remaining = transfer->data->size() - transfer->offset;
-  size_t chunk_length = std::min(remaining, max_request_size_);
+  size_t chunk_length = std::min(remaining, GetMaxIncrementalTransferSize());
   const uint8_t* data = transfer->data->front() + transfer->offset;
   std::vector<uint8_t> buf(data, data + chunk_length);
   SetArrayProperty(transfer->window, transfer->property, transfer->target, buf);

ui/base/x/selection_owner.h

@@ -136,9 +136,6 @@ class COMPONENT_EXPORT(UI_BASE_X) SelectionOwner {
   // The time that this instance took ownership of its selection.
   x11::Time acquired_selection_timestamp_;
 
-  // The maximum size of data we can put in XChangeProperty().
-  size_t max_request_size_;
-
   // The data we are currently serving.
   SelectionFormatMap format_map_;
 

ui/base/x/x11_util.cc

@@ -27,6 +27,7 @@
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/memory/ref_counted_memory.h"
+#include "base/memory/scoped_refptr.h"
 #include "base/memory/singleton.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/no_destructor.h"
@@ -248,6 +249,10 @@ void DrawPixmap(x11::Connection* connection,
                 int dst_y,
                 int width,
                 int height) {
+  // 24 bytes for the PutImage header, an additional 4 bytes in case this is an
+  // extended size request, and an additional 4 bytes in case padding is needed.
+  constexpr size_t kPutImageExtraSize = 32;
+
   const auto* visual_info = connection->GetVisualInfoFromId(visual);
   if (!visual_info)
     return;
@@ -271,19 +276,30 @@ void DrawPixmap(x11::Connection* connection,
   std::vector<uint8_t> vec(row_bytes * height);
   SkPixmap pixmap(image_info, vec.data(), row_bytes);
   skia_pixmap.readPixels(pixmap, src_x, src_y);
-  x11::PutImageRequest put_image_request{
-      .format = x11::ImageFormat::ZPixmap,
-      .drawable = drawable,
-      .gc = gc,
-      .width = width,
-      .height = height,
-      .dst_x = dst_x,
-      .dst_y = dst_y,
-      .left_pad = 0,
-      .depth = visual_info->format->depth,
-      .data = base::RefCountedBytes::TakeVector(&vec),
-  };
-  connection->PutImage(put_image_request);
+
+  DCHECK_GT(connection->MaxRequestSizeInBytes(), kPutImageExtraSize);
+  int rows_per_request =
+      (connection->MaxRequestSizeInBytes() - kPutImageExtraSize) / row_bytes;
+  DCHECK_GT(rows_per_request, 1);
+  for (int row = 0; row < height; row += rows_per_request) {
+    size_t n_rows = std::min<size_t>(rows_per_request, height - row);
+    auto data = base::MakeRefCounted<base::RefCountedStaticMemory>(
+        vec.data() + row * row_bytes, n_rows * row_bytes);
+    connection->PutImage({
+        .format = x11::ImageFormat::ZPixmap,
+        .drawable = drawable,
+        .gc = gc,
+        .width = width,
+        .height = n_rows,
+        .dst_x = dst_x,
+        .dst_y = dst_y + row,
+        .left_pad = 0,
+        .depth = visual_info->format->depth,
+        .data = data,
+    });
+  }
+  // Flush since the PutImage requests depend on |vec| being alive.
+  connection->Flush();
 }
 
 bool IsXInput2Available() {

ui/gfx/x/connection.cc

@@ -184,6 +184,11 @@ Connection::~Connection() {
   xcb_disconnect(connection_);
 }
 
+size_t Connection::MaxRequestSizeInBytes() const {
+  return 4 * std::max<size_t>(extended_max_request_length_,
+                              setup_.maximum_request_length);
+}
+
 XlibDisplayWrapper Connection::GetXlibDisplay(XlibDisplayType type) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   if (!xlib_display_)

ui/gfx/x/connection.h

@@ -90,10 +90,7 @@ class COMPONENT_EXPORT(X11) Connection : public XProto,
   XlibDisplayWrapper GetXlibDisplay(
       XlibDisplayType type = XlibDisplayType::kNormal);
 
-  uint32_t extended_max_request_length() const {
-    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-    return extended_max_request_length_;
-  }
+  size_t MaxRequestSizeInBytes() const;
 
   const Setup& setup() const {
     DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);