[Webkit-unassigned] [Bug 161907] [GTK] Get rid of DataObjectGtk::forClipboard and cleanup pasteboard code

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=161907

Michael Catanzaro <mcatanzaro at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #288687|review?                     |review+
              Flags|                            |

--- Comment #2 from Michael Catanzaro <mcatanzaro at igalia.com> ---
Comment on attachment 288687
  --> https://bugs.webkit.org/attachment.cgi?id=288687
Patch

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

> Source/WebCore/platform/gtk/PasteboardGtk.cpp:99
> -DataObjectGtk* Pasteboard::dataObject() const
> +const DataObjectGtk& Pasteboard::dataObject() const
>  {
> -    return m_dataObject.get();
> +    return *m_dataObject;
>  }

You should also change m_dataObject to be a (non-const) reference, since it's always initialized in the constructor initializer lists and there are also ASSERTs to ensure it's not null. Then you can get rid of the ASSERTS, and don't have to dereference it throughout the file anymore.

> Source/WebCore/platform/gtk/PasteboardHelper.cpp:311
> +            // When gtk_clipboard_set_with_data fails the callbacks are ignored, so we need to leak the data we were passing to clearClipboardContentsCallback.
> +            data.release();

This confused me because I assumed this was on the failure path, because your comment starts with "when it fails," but in fact this is the success case. I see that you need to leak it only on success, so the code is right, just the comment is confusing.

> Source/WebKit2/WebProcess/WebCoreSupport/gtk/WebDragClientGtk.cpp:69
> +    DragData dragData(const_cast<DataObjectGtk*>(&dataObject), clientPosition, globalPosition, dataTransfer.sourceOperation());

Wouldn't it be better to provide a non-const accessor as well, to avoid the const_cast here?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160913/a5c28f2d/attachment.html>