Bug 120773

Summary: WTF::OwnPtr should behave similarly with the rest of WTF smart pointers
Product: WebKit Reporter: Mikhail Pozdnyakov <mikhail.pozdnyakov>
Component: Web Template FrameworkAssignee: Mikhail Pozdnyakov <mikhail.pozdnyakov>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, benjamin, cdumez, cmarcelo, commit-queue, darin, dino, dstockwell, gyuyoung.kim, kling, rakuco, simon.fraser
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 120778    
Bug Blocks:    
Attachments:
Description Flags
patch
buildbot: commit-queue-
try fix Win
buildbot: commit-queue-
rebased none

Mikhail Pozdnyakov
Reported 2013-09-05 07:53:53 PDT
At the moment, unlike most of WTF smart pointers, PassOwnPtr can take either the pointer type or the pointed-to type. It means that OwnPtr<SomeType> is the same as OwnPtr<SomeType*> which is wrong for the following reasons: 1) It distinguishes OwnPtr behaviour from other WTF (and not only WTF!) smart pointer classes behaviour 2) The code like OwnPtr<SomeType> a; OwnPtr<SomeType*> b = adoptRef(..); a = b.release(); looks weird 3) It is potentially error-prone as it actually modifies the type given by the Client in opaque way.
Attachments
patch (7.80 KB, patch)
2013-09-05 08:01 PDT, Mikhail Pozdnyakov 		buildbot: commit-queue-
DetailsFormatted DiffDiff
try fix Win (10.33 KB, patch)
2013-09-05 09:13 PDT, Mikhail Pozdnyakov 		buildbot: commit-queue-
DetailsFormatted DiffDiff
rebased (7.63 KB, patch)
2013-09-11 01:43 PDT, Mikhail Pozdnyakov 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Mikhail Pozdnyakov
Comment 1 2013-09-05 08:01:00 PDT
Created attachment 210616 [details] patch
Build Bot
Comment 2 2013-09-05 08:46:57 PDT
Comment on attachment 210616 [details] patch Attachment 210616 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/1691867
Mikhail Pozdnyakov
Comment 3 2013-09-05 09:13:56 PDT
Created attachment 210625 [details] try fix Win
Build Bot
Comment 4 2013-09-05 09:52:25 PDT
Comment on attachment 210625 [details] try fix Win Attachment 210625 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/1692916
Darin Adler
Comment 5 2013-09-05 09:52:42 PDT
RetainPtr also has this feature.
Anders Carlsson
Comment 6 2013-09-05 09:56:43 PDT
(In reply to comment #5) > RetainPtr also has this feature. RetainPtr has this for a good reason. OwnPtr has this to support putting GDI objects in OwnPtrs. I think we should add a new smart pointer for GDI objects and make OwnPtr simpler.
Anders Carlsson
Comment 7 2013-09-05 10:11:22 PDT
(In reply to comment #6) > (In reply to comment #5) > > RetainPtr also has this feature. > > RetainPtr has this for a good reason. OwnPtr has this to support putting GDI objects in OwnPtrs. > > I think we should add a new smart pointer for GDI objects and make OwnPtr simpler. Brent filed https://bugs.webkit.org/show_bug.cgi?id=120778 following our discussion on IRC.
Darin Adler
Comment 8 2013-09-05 17:54:16 PDT
Ah, right. RetainPtr has this so that it can work with both CF style where the typedef is a pointer type and the NS style where the type is an Objective-C object type.
Mikhail Pozdnyakov
Comment 9 2013-09-11 01:43:57 PDT
Created attachment 211290 [details] rebased
WebKit Commit Bot
Comment 10 2013-09-11 07:42:02 PDT
Comment on attachment 211290 [details] rebased Clearing flags on attachment: 211290 Committed r155527: <http://trac.webkit.org/changeset/155527>
WebKit Commit Bot
Comment 11 2013-09-11 07:42:05 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.