Summary: | Add an owning smart pointer for RenderObjects and start using it. | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Andreas Kling <kling> | ||||
Component: | Layout and Rendering | Assignee: | Andreas Kling <kling> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | commit-queue, esprehn+autocc, glenn, kangil.han, kling, koivisto, kondapallykalyan, zan | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Andreas Kling
2013-12-26 21:15:37 PST
Created attachment 220046 [details]
Patch
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.
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). 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(). (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()? 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> { }; Comment on attachment 220046 [details] Patch Clearing flags on attachment: 220046 Committed r161115: <http://trac.webkit.org/changeset/161115> All reviewed patches have been landed. Closing bug. |