Bug 162267

Summary: [GTK] Clean up DataObjectGtk handling
Product: WebKit Reporter: Carlos Garcia Campos <cgarcia>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: berto, bugs-noreply, commit-queue, gustavo, mcatanzaro, mrobinson
Priority: P2 Keywords: Gtk
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch mcatanzaro: review+

Carlos Garcia Campos
Reported 2016-09-20 06:09:03 PDT
In some cases the ownership of DataObjectGtk instances is not clear enough, and we have hacks to avoid memory leaks because of that.
Attachments
Patch (48.11 KB, patch)
2016-09-20 06:34 PDT, Carlos Garcia Campos
mcatanzaro: review+
Carlos Garcia Campos
Comment 1 2016-09-20 06:34:57 PDT
WebKit Commit Bot
Comment 2 2016-09-20 06:37:16 PDT
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Michael Catanzaro
Comment 3 2016-09-20 10:30:31 PDT
Comment on attachment 289342 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=289342&action=review > Source/WebCore/platform/gtk/PasteboardHelper.cpp:295 > + TemporaryChange<DataObjectGtk*> change(settingClipboardDataObject, const_cast<DataObjectGtk*>(&dataObject)); Why is this function receiving a const DataObjectGtk*? Why were you unable to get rid of this const_cast? > Source/WebKit2/Shared/gtk/PasteboardContent.cpp:36 > +PasteboardContent::PasteboardContent(const WebCore::DataObjectGtk& data) > + : dataObject(const_cast<WebCore::DataObjectGtk&>(data)) Again, why is the parameter const? What call site cannot be fixed?
Carlos Garcia Campos
Comment 4 2016-09-20 23:17:40 PDT
(In reply to comment #3) > Comment on attachment 289342 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=289342&action=review > > > Source/WebCore/platform/gtk/PasteboardHelper.cpp:295 > > + TemporaryChange<DataObjectGtk*> change(settingClipboardDataObject, const_cast<DataObjectGtk*>(&dataObject)); > > Why is this function receiving a const DataObjectGtk*? Why were you unable > to get rid of this const_cast? > > > Source/WebKit2/Shared/gtk/PasteboardContent.cpp:36 > > +PasteboardContent::PasteboardContent(const WebCore::DataObjectGtk& data) > > + : dataObject(const_cast<WebCore::DataObjectGtk&>(data)) > > Again, why is the parameter const? What call site cannot be fixed? Both are special cases. You can't create smart pointers from const references/pointers, but in these particular cases the object is going to be const, the only change we are going to do to the object is taking a reference, and RefCounted::ref() is const indeed. So we can't fix the callers, because it's correct to receive consts, but we cast to allow creating a Ref<> that we know we are ot going to modify.
Carlos Garcia Campos
Comment 5 2016-09-20 23:19:42 PDT
Note You need to log in before you can comment on or make changes to this bug.