WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Andreas Kling
Comment 1
2013-12-26 21:42:30 PST
Created
attachment 220046
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug