Bug 120773 - WTF::OwnPtr should behave similarly with the rest of WTF smart pointers
Summary: WTF::OwnPtr should behave similarly with the rest of WTF smart pointers
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mikhail Pozdnyakov
URL:
Keywords:
Depends on: 120778
Blocks:
  Show dependency tree / graph
 
Reported: 2013-09-05 07:53 PDT by Mikhail Pozdnyakov
Modified: 2013-09-11 07:42 PDT (History)
12 users (show)

See Also:

Attachments
patch (7.80 KB, patch)
2013-09-05 08:01 PDT, Mikhail Pozdnyakov 		buildbot: commit-queue-
Details | Formatted Diff | Diff
try fix Win (10.33 KB, patch)
2013-09-05 09:13 PDT, Mikhail Pozdnyakov 		buildbot: commit-queue-
Details | Formatted Diff | Diff
rebased (7.63 KB, patch)
2013-09-11 01:43 PDT, Mikhail Pozdnyakov 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Mikhail Pozdnyakov 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.
Comment 1 Mikhail Pozdnyakov 2013-09-05 08:01:00 PDT 
Created attachment 210616 [details]
patch
Comment 2 Build Bot 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
Comment 3 Mikhail Pozdnyakov 2013-09-05 09:13:56 PDT 
Created attachment 210625 [details]
try fix Win
Comment 4 Build Bot 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
Comment 5 Darin Adler 2013-09-05 09:52:42 PDT 
RetainPtr also has this feature.
Comment 6 Anders Carlsson 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.
Comment 7 Anders Carlsson 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.
Comment 8 Darin Adler 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.
Comment 9 Mikhail Pozdnyakov 2013-09-11 01:43:57 PDT 
Created attachment 211290 [details]
rebased
Comment 10 WebKit Commit Bot 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>
Comment 11 WebKit Commit Bot 2013-09-11 07:42:05 PDT 
All reviewed patches have been landed.  Closing bug.