[webkit-reviews] review granted: [Bug 211779] [GTK4] Add support for drag and drop operations : [Attachment 399123] Patch

[bugzilla-daemon at webkit.org]

Adrian Perez <aperez at igalia.com> has granted Carlos Garcia Campos
<cgarcia at igalia.com>'s request for review:
Bug 211779: [GTK4] Add support for drag and drop operations
https://bugs.webkit.org/show_bug.cgi?id=211779

Attachment 399123: Patch

https://bugs.webkit.org/attachment.cgi?id=399123&action=review




--- Comment #2 from Adrian Perez <aperez at igalia.com> ---
Comment on attachment 399123
  --> https://bugs.webkit.org/attachment.cgi?id=399123
Patch

This was easier to follow after having read all the other patches in the series
=)

Patch LGTM, but I see an opportunity to avoid unnecessary copying of data.
Please
take a look and consider whether it may be good to apply the suggested change
before
landing.

View in context: https://bugs.webkit.org/attachment.cgi?id=399123&action=review

> Source/WebCore/platform/graphics/gtk/ImageGtk.cpp:64
> +GdkTexture* BitmapImage::gdkTexture()

Nice refactor :)

> Source/WebKit/UIProcess/API/gtk/DragSourceGtk4.cpp:59
> +	   GRefPtr<GBytes> bytes = adoptGRef(g_bytes_new(markup.data(),
markup.length()));

This (and the ones below) makes one copy if data into a CString, then another
copy
with g_bytes_new(). We could get the inner CStringBuffer from the CString,
which
is reference counted, and use that with g_bytes_new_with_free_func(). Rough
idea:

  CString markup = m_selectionData->markup().utf8();
  RefPtr<CStringBuffer> buffer = markup.buffer();
  GRefPtr<GBytes> bytes = adoptGRef(g_bytes_new_with_free_func(buffer->data(),
buffer->length(),
      [](void* data) { RefPtr<CStringBuffer> buffer =
adoptRef(static_cast<CStringBuffer*>(data)); },
      buffer.leakRef()));