Revert "[X11] Split PutImage requests into multiple requests if necessary"

This reverts commit 3fe154df.

Reason for revert: Appears to have caused compile failure: https://ci.chromium.org/p/chromium/builders/ci/Linux%20Builder%20(dbg)(32)/166101?

[1052/1665] CXX obj/ui/base/x/x/selection_owner.o
FAILED: obj/ui/base/x/x/selection_owner.o
/b/s/w/ir/cache/goma/client/gomacc ../../third_party/llvm-build/Release+Asserts/bin/clang++ -MMD -MF...(too long)
../../ui/base/x/selection_owner.cc:46:10: error: no matching function for call to 'min'
return std::min(size - 100, 0x100000l);
^~~~~~~~
../../buildtools/third_party/libc++/trunk/include/algorithm:2532:1: note: candidate template ignored: deduced conflicting types for parameter '_Tp' ('int' vs. 'long')
min(const _Tp& __a, const _Tp& __b)
^

Original change's description:
> [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}

TBR=sky@chromium.org,thomasanderson@chromium.org,chromium-scoped@luci-project-accounts.iam.gserviceaccount.com

Change-Id: I3989bb05bc58bfadf33f7b67bae0ac2c4a5402e7
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1164669
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2644744Reviewed-by: Katie Dektar <katie@chromium.org>
Commit-Queue: Katie Dektar <katie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#846351}

ui/base/x/selection_owner.cc

@@ -39,11 +39,16 @@ const int kIncrementalTransferTimeoutMs = 10000;
 static_assert(KSelectionOwnerTimerPeriodMs <= kIncrementalTransferTimeoutMs,
               "timer period must be <= transfer timeout");
 
-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);
+// 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));
 }
 
 // Gets the value of an atom pair array property. On success, true is returned
@@ -80,7 +85,9 @@ 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) {}
+    : x_window_(x_window),
+      selection_name_(selection_name),
+      max_request_size_(GetMaxRequestSize(connection)) {}
 
 SelectionOwner::~SelectionOwner() {
   // If we are the selection owner, we need to release the selection so we
@@ -210,7 +217,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() > GetMaxIncrementalTransferSize()) {
+    if (it->second->size() > max_request_size_) {
       // 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".
@@ -253,7 +260,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, GetMaxIncrementalTransferSize());
+  size_t chunk_length = std::min(remaining, max_request_size_);
   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,6 +136,9 @@ 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,7 +27,6 @@
 #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"
@@ -249,10 +248,6 @@ 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;
@@ -276,30 +271,19 @@ 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);
-
-  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({
+  x11::PutImageRequest put_image_request{
       .format = x11::ImageFormat::ZPixmap,
       .drawable = drawable,
       .gc = gc,
       .width = width,
-        .height = n_rows,
+      .height = height,
       .dst_x = dst_x,
-        .dst_y = dst_y + row,
+      .dst_y = dst_y,
       .left_pad = 0,
       .depth = visual_info->format->depth,
-        .data = data,
-    });
-  }
-  // Flush since the PutImage requests depend on |vec| being alive.
-  connection->Flush();
+      .data = base::RefCountedBytes::TakeVector(&vec),
+  };
+  connection->PutImage(put_image_request);
 }
 
 bool IsXInput2Available() {

ui/gfx/x/connection.cc

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