Bug 25495 - Implement PassOwnPtr and replace uses of std::auto_ptr
Summary: Implement PassOwnPtr and replace uses of std::auto_ptr
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: David Kilzer (:ddkilzer)
URL: http://webkit.org/coding/RefPtr.html
Keywords:
Depends on:
Blocks:
 
Reported: 2009-04-30 15:36 PDT by David Kilzer (:ddkilzer)
Modified: 2009-05-23 10:33 PDT (History)
5 users (show)

See Also:


Attachments
Part 1 of 2: Implement PassOwnPtr (28.29 KB, patch)
2009-05-06 20:03 PDT, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff
Part 2 of 2: Use PassOwnPtr (49.54 KB, patch)
2009-05-06 20:04 PDT, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff
Part 1 of 2 (v2): Implement PassOwnPtr (29.18 KB, patch)
2009-05-17 16:09 PDT, David Kilzer (:ddkilzer)
oliver: review+
Details | Formatted Diff | Diff
Part 2 of 2 (v2): Use PassOwnPtr (49.44 KB, patch)
2009-05-17 16:09 PDT, David Kilzer (:ddkilzer)
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 2009-04-30 15:36:17 PDT
* SUMMARY
I think we should implement our own PassOwnPtr templates and get rid of std::auto_ptr.

1. You can't do this with auto_ptr without getting compilation errors:

    class SomeClass;
    [...]
    virtual auto_ptr<SomeClass> createSomeClass() { return 0; }

2. When you fix the above code, it requires you do the following *and* include the whole SomeClass.h header instead of just using a class predeclaration:

    #include "SomeClass.h"
    [...]
    virtual auto_ptr<SomeClass> createSomeClass() { return auto_ptr<SomeClass>(); }

Lame.

3. Looking at auto_ptr and OwnPtr, you wouldn't know they're related (like PassRefPtr and RefPtr).
Comment 1 David Kilzer (:ddkilzer) 2009-05-04 11:54:00 PDT
(In reply to comment #0)
> 2. When you fix the above code, it requires you do the following *and* include
> the whole SomeClass.h header instead of just using a class predeclaration:
> 
>     #include "SomeClass.h"
>     [...]
>     virtual auto_ptr<SomeClass> createSomeClass() { return
> auto_ptr<SomeClass>(); }

After implementing PassOwnPtr, I realized that you still must #include "SomeClass.h" anyway, but you can just return 0:

    #include "SomeClass.h"
    [...]
    virtual auto_ptr<SomeClass> createSomeClass() { return 0; }
Comment 2 Adam Roben (:aroben) 2009-05-04 14:34:45 PDT
(In reply to comment #1)
> After implementing PassOwnPtr, I realized that you still must #include
> "SomeClass.h" anyway, but you can just return 0:
> 
>     #include "SomeClass.h"
>     [...]
>     virtual auto_ptr<SomeClass> createSomeClass() { return 0; }

If you do that, it will compile, but it will crash (at least in MSVC).
Comment 3 David Kilzer (:ddkilzer) 2009-05-04 17:32:48 PDT
(In reply to comment #2)
> (In reply to comment #1)
> > After implementing PassOwnPtr, I realized that you still must #include
> > "SomeClass.h" anyway, but you can just return 0:
> > 
> >     #include "SomeClass.h"
> >     [...]
> >     virtual auto_ptr<SomeClass> createSomeClass() { return 0; }
> 
> If you do that, it will compile, but it will crash (at least in MSVC).

Sorry, that was a typo.  I meant to change it to:

> >     virtual PassOwnPtr<SomeClass> createSomeClass() { return 0; }
Comment 4 David Kilzer (:ddkilzer) 2009-05-06 20:03:27 PDT
Created attachment 30083 [details]
Part 1 of 2: Implement PassOwnPtr
Comment 5 David Kilzer (:ddkilzer) 2009-05-06 20:04:08 PDT
Created attachment 30084 [details]
Part 2 of 2: Use PassOwnPtr
Comment 6 David Kilzer (:ddkilzer) 2009-05-17 16:09:18 PDT
Created attachment 30430 [details]
Part 1 of 2 (v2): Implement PassOwnPtr

Rebased against ToT WebKit.
Comment 7 David Kilzer (:ddkilzer) 2009-05-17 16:09:55 PDT
Created attachment 30431 [details]
Part 2 of 2 (v2): Use PassOwnPtr

Rebased against ToT WebKit.
Comment 8 Oliver Hunt 2009-05-22 20:25:51 PDT
Comment on attachment 30430 [details]
Part 1 of 2 (v2): Implement PassOwnPtr

r=me
Comment 9 Oliver Hunt 2009-05-22 20:26:27 PDT
Comment on attachment 30431 [details]
Part 2 of 2 (v2): Use PassOwnPtr

r=me, yay !
Comment 10 David Kilzer (:ddkilzer) 2009-05-23 09:42:08 PDT
$ git svn dcommit
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	JavaScriptCore/ChangeLog
	M	JavaScriptCore/GNUmakefile.am
	M	JavaScriptCore/JavaScriptCore.vcproj/WTF/WTF.vcproj
	M	JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj
	M	JavaScriptCore/wtf/OwnPtr.h
	A	JavaScriptCore/wtf/OwnPtrCommon.h
	A	JavaScriptCore/wtf/PassOwnPtr.h
	M	JavaScriptGlue/ChangeLog
	A	JavaScriptGlue/ForwardingHeaders/wtf/OwnPtrCommon.h
	A	JavaScriptGlue/ForwardingHeaders/wtf/PassOwnPtr.h
	M	WebCore/ChangeLog
	A	WebCore/ForwardingHeaders/wtf/OwnPtrCommon.h
	A	WebCore/ForwardingHeaders/wtf/PassOwnPtr.h
	M	WebCore/WebCore.vcproj/WebCore.vcproj
	M	WebKit/mac/ChangeLog
	A	WebKit/mac/ForwardingHeaders/wtf/OwnPtrCommon.h
	A	WebKit/mac/ForwardingHeaders/wtf/PassOwnPtr.h
	M	WebKitTools/ChangeLog
	A	WebKitTools/DumpRenderTree/ForwardingHeaders/wtf/OwnPtrCommon.h
	A	WebKitTools/DumpRenderTree/ForwardingHeaders/wtf/PassOwnPtr.h
Committed r44095

http://trac.webkit.org/changeset/44095

$ git svn dcommit
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/WebCore.base.exp
	M	WebCore/dom/Node.cpp
	M	WebCore/dom/NodeRareData.h
	M	WebCore/history/HistoryItem.cpp
	M	WebCore/history/HistoryItem.h
	M	WebCore/html/CanvasRenderingContext2D.cpp
	M	WebCore/html/HTMLCanvasElement.cpp
	M	WebCore/loader/EmptyClients.h
	M	WebCore/page/ChromeClient.h
	M	WebCore/platform/graphics/GeneratedImage.cpp
	M	WebCore/platform/graphics/ImageBuffer.h
	M	WebCore/platform/mac/ScrollbarThemeMac.mm
	M	WebCore/platform/text/TextCodec.h
	M	WebCore/platform/text/TextCodecICU.cpp
	M	WebCore/platform/text/TextCodecLatin1.cpp
	M	WebCore/platform/text/TextCodecUTF16.cpp
	M	WebCore/platform/text/TextCodecUserDefined.cpp
	M	WebCore/platform/text/TextEncodingRegistry.cpp
	M	WebCore/platform/text/TextEncodingRegistry.h
	M	WebCore/platform/text/mac/TextCodecMac.cpp
	M	WebCore/rendering/RenderBoxModelObject.cpp
	M	WebCore/svg/SVGMaskElement.cpp
	M	WebCore/svg/SVGMaskElement.h
	M	WebCore/svg/SVGPatternElement.cpp
	M	WebCore/svg/graphics/SVGImage.cpp
	M	WebCore/svg/graphics/SVGPaintServerGradient.cpp
	M	WebCore/svg/graphics/SVGPaintServerPattern.cpp
	M	WebCore/svg/graphics/SVGPaintServerPattern.h
	M	WebCore/svg/graphics/SVGResourceMasker.cpp
	M	WebKit/gtk/ChangeLog
	M	WebKit/gtk/WebCoreSupport/ChromeClientGtk.h
	M	WebKit/mac/ChangeLog
	M	WebKit/mac/History/WebHistory.mm
	M	WebKit/mac/History/WebHistoryItem.mm
	M	WebKit/mac/WebCoreSupport/WebChromeClient.h
	M	WebKit/qt/ChangeLog
	M	WebKit/qt/WebCoreSupport/ChromeClientQt.h
	M	WebKit/win/ChangeLog
	M	WebKit/win/WebCoreSupport/WebChromeClient.h
	M	WebKit/wx/ChangeLog
	M	WebKit/wx/WebKitSupport/ChromeClientWx.h
Committed r44096

http://trac.webkit.org/changeset/44096
Comment 11 David Kilzer (:ddkilzer) 2009-05-23 10:17:07 PDT
Attempted Windows build fix:

$ git svn dcommit
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebKit/win/ChangeLog
	M	WebKit/win/WebHistory.cpp
	M	WebKit/win/WebHistoryItem.cpp
Committed r44098

http://trac.webkit.org/changeset/44098
Comment 12 David Kilzer (:ddkilzer) 2009-05-23 10:33:22 PDT
Attempted to fix Gtk and Qt builds:

$ git svn dcommit
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/platform/graphics/cairo/ImageCairo.cpp
	M	WebCore/platform/graphics/qt/PathQt.cpp
Committed r44099

http://trac.webkit.org/changeset/44099