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 treegraph
 
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

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.