Implement Wayland nsClipboard backend

Comment on attachment 8937432 [details] Bug 1417890 - rename nsRetrievalContext::WaitForClipboardContext() to nsRetrievalContext::GetClipboardData() and implement nsRetrievalContext::ReleaseClipboardData(), https://reviewboard.mozilla.org/r/208096/#review215376 ::: widget/gtk/nsClipboard.h:24 (Diff revision 1) > public: > - // Returned data must be released by free() > + virtual const char* GetClipboardData(const char* aMimeType, > - virtual const char* WaitForClipboardContext(const char* aMimeType, > - int32_t aWhichClipboard, > + int32_t aWhichClipboard, > - uint32_t* aContentLength) = 0; > + uint32_t* aContentLength) = 0; > + virtual void ReleaseClipboardData(const char* aClipboardData) = 0; Does the ReleaseClipboardData has to be virutal there? Isn't the implementation is the same for X11 and wayland? I see the idea behind that you want to manage memory better, question is if this needs to be method of nsRetrievalContext as long as it does not manipulate with object instance. ::: widget/gtk/nsClipboard.cpp:311 (Diff revision 1) > continue; > } > > // Convert utf-8 into our unicode format. > - NS_ConvertUTF8toUTF16 ucs2string(rawData, clipboardDataLength); > - const char* clipboardData = (const char *)ToNewUnicode(ucs2string); > + NS_ConvertUTF8toUTF16 ucs2string(clipboardData, clipboardDataLength); > + const char* unicodeData = (const char *)ToNewUnicode(ucs2string); Free the clipboardData as soon as it is not needed anymore. ::: widget/gtk/nsClipboard.cpp:337 (Diff revision 1) > &htmlBody, htmlBodyLen); > - free((void *)clipboardData); > > // Try next data format? > if (!htmlBodyLen) > continue; It is possible that you're leaking clipboardData here.

Comment on attachment 8937433 [details] Bug 1417890 - Implement nsRetrievalContextWayland, https://reviewboard.mozilla.org/r/208098/#review214846 ::: widget/gtk/nsClipboardWayland.cpp:367 (Diff revision 1) > + } > + > + GArray* dataStream = g_array_new(false, false, sizeof(guchar)); > + int length; > + > + #define BUFFER_SIZE 4096 If you prefer to use glib2 for the reading, check the https://developer.gnome.org/glib/stable/glib-IO-Channels.html object and its reading method g_io_channel_read_to_end (). ::: widget/gtk/nsClipboardWayland.cpp:207 (Diff revision 2) > + > +void > +nsRetrievalContextWayland::ConfigureKeyboard(wl_seat_capability caps) > +{ > + if (caps & WL_SEAT_CAPABILITY_KEYBOARD) { > + mKeyboard = wl_seat_get_keyboard(mSeat); You most likely need to call wl_keyboard_destroy(mKeyboard) somewhere if this succeed. ::: widget/gtk/nsClipboardWayland.cpp:358 (Diff revision 2) > + struct pollfd fds; > + fds.fd = pipe_fd[0]; > + fds.events = POLLIN; > + > + // Choose some reasonable timeout here > + int ret = poll(&fds, 1, kClipboardTimeout*1000); The poll() timeout is in milliseconds, so with kClipboardTimeout = 500000 we would wait there for 500 seconds, that's too much :). Better to divide by 1000. ::: widget/gtk/nsClipboardWayland.cpp:375 (Diff revision 2) > + if (!error) { > + g_io_channel_read_to_end(channel, &clipboardData, &dataLength, &error); > + } > + > + if (error) { > + g_error_free(error); please add something like: NS_WARNING(nsPrintfCString("Unexpected error when reading clipboard data: %s", error->message)); to keep us informed.

Comment on attachment 8937434 [details] Bug 1417890 - Use nsRetrievalContextWayland for Wayland displays, https://reviewboard.mozilla.org/r/208100/#review215382 ::: widget/gtk/nsClipboardWayland.h:24 (Diff revision 3) > nsRetrievalContextWayland(); > > virtual const char* GetClipboardData(const char* aMimeType, > int32_t aWhichClipboard, > uint32_t* aContentLength) override; > - virtual void ReleaseClipboardData(const char* aClipboardData); > + virtual void ReleaseClipboardData(const char* aClipboardData) override; Please move the implementation to the nsRetrievalContext class since it is has the same implementation in X11 and Wayland. ::: widget/gtk/nsClipboardX11.h:22 (Diff revision 3) > enum State { INITIAL, COMPLETED, TIMED_OUT }; > > virtual const char* GetClipboardData(const char* aMimeType, > int32_t aWhichClipboard, > uint32_t* aContentLength) override; > - virtual void ReleaseClipboardData(const char* aClipboardData); > + virtual void ReleaseClipboardData(const char* aClipboardData) override; dtto

Comment on attachment 8937435 [details] Bug 1417890 - Install nsClipboard hook at nsWindow init, https://reviewboard.mozilla.org/r/208102/#review215384

Comment on attachment 8937432 [details] Bug 1417890 - rename nsRetrievalContext::WaitForClipboardContext() to nsRetrievalContext::GetClipboardData() and implement nsRetrievalContext::ReleaseClipboardData(), https://reviewboard.mozilla.org/r/208096/#review215766 ::: widget/gtk/nsClipboard.h:24 (Diff revision 1) > public: > - // Returned data must be released by free() > + virtual const char* GetClipboardData(const char* aMimeType, > - virtual const char* WaitForClipboardContext(const char* aMimeType, > - int32_t aWhichClipboard, > + int32_t aWhichClipboard, > - uint32_t* aContentLength) = 0; > + uint32_t* aContentLength) = 0; > + virtual void ReleaseClipboardData(const char* aClipboardData) = 0; ReleaseClipboardData() uses different memory release methods (free() / g_free()) due to wayland specific implementation.

Comment on attachment 8937432 [details] Bug 1417890 - rename nsRetrievalContext::WaitForClipboardContext() to nsRetrievalContext::GetClipboardData() and implement nsRetrievalContext::ReleaseClipboardData(), https://reviewboard.mozilla.org/r/208096/#review215772 ::: widget/gtk/nsClipboard.cpp:311 (Diff revision 1) > continue; > } > > // Convert utf-8 into our unicode format. > - NS_ConvertUTF8toUTF16 ucs2string(rawData, clipboardDataLength); > - const char* clipboardData = (const char *)ToNewUnicode(ucs2string); > + NS_ConvertUTF8toUTF16 ucs2string(clipboardData, clipboardDataLength); > + const char* unicodeData = (const char *)ToNewUnicode(ucs2string); After discussions se decided to leave it as it is.

Comment on attachment 8937433 [details] Bug 1417890 - Implement nsRetrievalContextWayland, https://reviewboard.mozilla.org/r/208098/#review215776 ::: widget/gtk/nsClipboardWayland.cpp:207 (Diff revision 2) > + > +void > +nsRetrievalContextWayland::ConfigureKeyboard(wl_seat_capability caps) > +{ > + if (caps & WL_SEAT_CAPABILITY_KEYBOARD) { > + mKeyboard = wl_seat_get_keyboard(mSeat); wl_keyboard_destroy(mKeyboard) is called when we lost keyboard at nsRetrievalContextWayland::ConfigureKeyboard(). when seat_handle_capabilities(wl_seat_capability ...) callback is called without WL_SEAT_CAPABILITY_KEYBOARD flag it means we lost it and it's released.

Comment on attachment 8937434 [details] Bug 1417890 - Use nsRetrievalContextWayland for Wayland displays, https://reviewboard.mozilla.org/r/208100/#review215782 ::: widget/gtk/nsClipboardWayland.h:24 (Diff revision 3) > nsRetrievalContextWayland(); > > virtual const char* GetClipboardData(const char* aMimeType, > int32_t aWhichClipboard, > uint32_t* aContentLength) override; > - virtual void ReleaseClipboardData(const char* aClipboardData); > + virtual void ReleaseClipboardData(const char* aClipboardData) override; We can't do that as we have different memory release methods for wayland/X11.

Comment on attachment 8937432 [details] Bug 1417890 - rename nsRetrievalContext::WaitForClipboardContext() to nsRetrievalContext::GetClipboardData() and implement nsRetrievalContext::ReleaseClipboardData(), https://reviewboard.mozilla.org/r/208096/#review217738

Comment on attachment 8937433 [details] Bug 1417890 - Implement nsRetrievalContextWayland, https://reviewboard.mozilla.org/r/208098/#review217740

Comment on attachment 8937434 [details] Bug 1417890 - Use nsRetrievalContextWayland for Wayland displays, https://reviewboard.mozilla.org/r/208100/#review217742

Pushed by stransky@redhat.com: https://hg.mozilla.org/integration/autoland/rev/8406e5c06039 rename nsRetrievalContext::WaitForClipboardContext() to nsRetrievalContext::GetClipboardData() and implement nsRetrievalContext::ReleaseClipboardData(), r=jhorak https://hg.mozilla.org/integration/autoland/rev/7269882141b8 Implement nsRetrievalContextWayland, r=jhorak https://hg.mozilla.org/integration/autoland/rev/86b56bad2991 Use nsRetrievalContextWayland for Wayland displays, r=jhorak https://hg.mozilla.org/integration/autoland/rev/2894b6be0c15 Install nsClipboard hook at nsWindow init, r=jhorak

https://hg.mozilla.org/mozilla-central/rev/8406e5c06039 https://hg.mozilla.org/mozilla-central/rev/7269882141b8 https://hg.mozilla.org/mozilla-central/rev/86b56bad2991 https://hg.mozilla.org/mozilla-central/rev/2894b6be0c15