RESOLVED FIXED 126251
Add an owning smart pointer for RenderObjects and start using it.
https://bugs.webkit.org/show_bug.cgi?id=126251
Summary Add an owning smart pointer for RenderObjects and start using it.
Andreas Kling
Reported 2013-12-26 21:15:37 PST
Add an owning smart pointer for RenderObjects and start using it.
Attachments
Patch (13.90 KB, patch)
2013-12-26 21:42 PST, Andreas Kling
no flags
Andreas Kling
Comment 1 2013-12-26 21:42:30 PST
WebKit Commit Bot
Comment 2 2013-12-26 21:43:14 PST
Attachment 220046 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/dom/PseudoElement.cpp', u'Source/WebCore/rendering/RenderPtr.h', u'Source/WebCore/rendering/style/ContentData.cpp', u'Source/WebCore/rendering/style/ContentData.h', '--commit-queue']" exit_code: 1 ERROR: Source/WebCore/rendering/style/ContentData.h:30: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/rendering/RenderPtr.h:29: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/rendering/RenderPtr.h:41: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WebCore/rendering/RenderPtr.h:42: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WebCore/rendering/RenderPtr.h:43: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] Total errors found: 5 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
Antti Koivisto
Comment 3 2013-12-28 03:48:30 PST
It would be nice if we could eventually just use unique_ptr and split the destroy() stuff between subtree removal (invoked when that happens) and actual destruction (in destructor).
Zan Dobersek
Comment 4 2013-12-28 04:15:49 PST
Comment on attachment 220046 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=220046&action=review > Source/WebCore/ChangeLog:8 > + This patch adds a RenderPtr pointer, essentially an OwnPtr for > + RenderObjects. The difference is that RenderPtr destroys the object > + by calling destroy() on it. If this is the only difference you could switch to std::unique_ptr, with an std::default_delete<RenderObject> specialization that would call destroy().
Andreas Kling
Comment 5 2013-12-28 10:21:00 PST
(In reply to comment #4) > (From update of attachment 220046 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=220046&action=review > > > Source/WebCore/ChangeLog:8 > > + This patch adds a RenderPtr pointer, essentially an OwnPtr for > > + RenderObjects. The difference is that RenderPtr destroys the object > > + by calling destroy() on it. > > If this is the only difference you could switch to std::unique_ptr, with an std::default_delete<RenderObject> specialization that would call destroy(). Wouldn't that require me to specialize separately for every single RenderObject subclass, or e.g std::unique_ptr<RenderBlock>'s destructor would just delete without calling destroy()?
Zan Dobersek
Comment 6 2013-12-28 12:17:06 PST
Yes, unfortunately I don't think there's a way to partially specialize std::default_delete for both the base class and its derivatives. The subclass specializations could just inherit from the base class specialization, but that's still not very scalable: template<> std::default_delete<RenderBlock> : std::default_delete<RenderObject> { };
WebKit Commit Bot
Comment 7 2013-12-28 17:32:43 PST
Comment on attachment 220046 [details] Patch Clearing flags on attachment: 220046 Committed r161115: <http://trac.webkit.org/changeset/161115>
WebKit Commit Bot
Comment 8 2013-12-28 17:32:45 PST
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.