Bug 131531

Summary: [cairo] Add GUniquePtrCairo and use it instead of OwnPtrCairo
Product: WebKit Reporter: Jae Hyun Park <jaepark>
Component: New BugsAssignee: Jae Hyun Park <jaepark>
Status: RESOLVED WONTFIX    
Severity: Normal CC: achristensen, bfulgham, bunhere, commit-queue, d-r, gyuyoung.kim, mrobinson, rakuco, sergio
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 130580    
Attachments:
Description Flags
Patch
none
Patch achristensen: review-

Jae Hyun Park
Reported 2014-04-11 03:02:41 PDT
Patch to follow.
Attachments
Patch (13.62 KB, patch)
2014-04-11 03:07 PDT, Jae Hyun Park 		no flags
DetailsFormatted DiffDiff
Patch (13.72 KB, patch)
2014-04-14 21:55 PDT, Jae Hyun Park 		achristensen: review-
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Jae Hyun Park
Comment 1 2014-04-11 03:07:02 PDT
Created attachment 229121 [details] Patch
Martin Robinson
Comment 2 2014-04-14 11:21:53 PDT
Comment on attachment 229121 [details] Patch It seems you are removing the FontConfig custom deleters and not replacing them.
Jae Hyun Park
Comment 3 2014-04-14 17:59:18 PDT
(In reply to comment #2) > (From update of attachment 229121 [details]) > It seems you are removing the FontConfig custom deleters and not replacing them. Thanks for the review. I could not find any piece of code that uses OwnPtr<FcObjectSet> or OwnPtr<FcFontSet>. So I thought it was deprecated and removed it. Should I include that in my changelog? or should I include them in GUniquePtrCairo anyway in case someone uses in the future?
Martin Robinson
Comment 4 2014-04-14 20:58:29 PDT
(In reply to comment #3) > I could not find any piece of code that uses OwnPtr<FcObjectSet> or OwnPtr<FcFontSet>. So I thought it was deprecated and removed it. Oh yes. It's probably a good idea to mention that in the ChangeLog. > Should I include that in my changelog? or should I include them in GUniquePtrCairo anyway in case someone uses in the future? That shouldn't be necessary.
Martin Robinson
Comment 5 2014-04-14 21:00:08 PDT
Comment on attachment 229121 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=229121&action=review > Source/WebCore/platform/graphics/freetype/FontCacheFreeType.cpp:-27 > -#include "RefPtrCairo.h" I think by removing this line you break the RefPtr<FcPattern> instance below. > Source/WebCore/platform/graphics/freetype/FontCacheFreeType.cpp:32 > +#include <wtf/PassOwnPtr.h> Including OwnPtr.h would be better, I think.
Jae Hyun Park
Comment 6 2014-04-14 21:55:18 PDT
Created attachment 229344 [details] Patch
Jae Hyun Park
Comment 7 2014-04-14 22:28:59 PDT
(In reply to comment #5) > (From update of attachment 229121 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=229121&action=review > > > Source/WebCore/platform/graphics/freetype/FontCacheFreeType.cpp:-27 > > -#include "RefPtrCairo.h" > > I think by removing this line you break the RefPtr<FcPattern> instance below. Fixed. > > > Source/WebCore/platform/graphics/freetype/FontCacheFreeType.cpp:32 > > +#include <wtf/PassOwnPtr.h> > > Including OwnPtr.h would be better, I think. Fixed. I also included why FontConfig custom deleters were removed in the ChangeLog. Thanks!
Martin Robinson
Comment 8 2014-04-15 07:42:13 PDT
Comment on attachment 229344 [details] Patch One thing I'm wondering about now is whether the WinCairo port uses GUniuqePtr and GLib in general. If not, I don't think we can actually use it here. :/
Martin Robinson
Comment 9 2014-04-15 07:42:44 PDT
Pinging a few WinCairo port maintainers.
Jae Hyun Park
Comment 10 2014-05-07 06:39:45 PDT
Any updates from WinCairo port maintainers?
Alex Christensen
Comment 11 2014-05-07 10:35:20 PDT
It is correct that WinCairo does not use glib, so relying on gio.h in GUniquePtr.h would break it.
Alex Christensen
Comment 12 2014-06-05 16:51:04 PDT
Comment on attachment 229344 [details] Patch This would break the WinCairo port.
Note You need to log in before you can comment on or make changes to this bug.