Bug 150082

Summary: [GTK] Use GUniquePtr for GtkIconInfo
Product: WebKit Reporter: ChangSeok Oh <changseok>
Component: WebKitGTKAssignee: ChangSeok Oh <changseok>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, cgarcia, commit-queue, esprehn+autocc, glenn, kondapallykalyan
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

ChangSeok Oh
Reported 2015-10-13 00:17:09 PDT
SSIA
Attachments
Patch (2.06 KB, patch)
2015-10-13 00:21 PDT, ChangSeok Oh 		no flags
DetailsFormatted DiffDiff
Patch (2.30 KB, patch)
2015-10-13 03:34 PDT, ChangSeok Oh 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
ChangSeok Oh
Comment 1 2015-10-13 00:21:54 PDT
Created attachment 262977 [details] Patch
Carlos Garcia Campos
Comment 2 2015-10-13 01:21:11 PDT
Comment on attachment 262977 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=262977&action=review Thanks for the patch. This is not enough, though. With this patch the GtkIconInfo will be freed with g_free, not with gtk_icon_info_free(), You need to also include GUniquePtrGtk.h. > Source/WebCore/rendering/RenderThemeGtk.cpp:242 > GdkPixbuf* icon = 0; Please change this to nullptr. > Source/WebCore/rendering/RenderThemeGtk.cpp:244 > + icon = gtk_icon_info_load_symbolic_for_context(info.get(), context, 0, 0); nullptr, nullptr
ChangSeok Oh
Comment 3 2015-10-13 02:48:51 PDT
(In reply to comment #2) > With this patch the GtkIconInfo will be freed with g_free, not with gtk_icon_info_free(), You need to also include GUniquePtrGtk.h. Thanks for your review. =) I am not sure of this comment. We already have the gtk_icon_info_free in GUniquePtrGtk.h So the smart pointer has been used in ImageGtk.cpp
Carlos Garcia Campos
Comment 4 2015-10-13 02:52:59 PDT
(In reply to comment #3) > (In reply to comment #2) > > With this patch the GtkIconInfo will be freed with g_free, not with gtk_icon_info_free(), You need to also include GUniquePtrGtk.h. > Thanks for your review. =) I am not sure of this comment. We already have > the gtk_icon_info_free in GUniquePtrGtk.h So the smart pointer has been used > in ImageGtk.cpp ImageGtk.cpp includes GUniquePtrGtk.h, but RenderThemeGtk.cpp doesn't.
ChangSeok Oh
Comment 5 2015-10-13 03:34:05 PDT
Created attachment 262981 [details] Patch
ChangSeok Oh
Comment 6 2015-10-13 03:37:01 PDT
(In reply to comment #4) > (In reply to comment #3) > > (In reply to comment #2) > > > With this patch the GtkIconInfo will be freed with g_free, not with gtk_icon_info_free(), You need to also include GUniquePtrGtk.h. > > Thanks for your review. =) I am not sure of this comment. We already have > > the gtk_icon_info_free in GUniquePtrGtk.h So the smart pointer has been used > > in ImageGtk.cpp > > ImageGtk.cpp includes GUniquePtrGtk.h, but RenderThemeGtk.cpp doesn't. Oh yeah. You are right.
WebKit Commit Bot
Comment 7 2015-10-13 04:31:23 PDT
Comment on attachment 262981 [details] Patch Clearing flags on attachment: 262981 Committed r190970: <http://trac.webkit.org/changeset/190970>
WebKit Commit Bot
Comment 8 2015-10-13 04:31:26 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.