Bug 126251 – Add an owning smart pointer for RenderObjects and start using it.

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Andreas Kling]

Add an owning smart pointer for RenderObjects and start using it.

[Andreas Kling]

Created attachment 220046 [details]
Patch

[WebKit Commit Bot]

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]

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

(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]

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 on attachment 220046 [details]
Patch

Clearing flags on attachment: 220046

Committed r161115: <http://trac.webkit.org/changeset/161115>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.