RESOLVED FIXED 121323
Make FloatingObjects own it's FloatingObject instances 
https://bugs.webkit.org/show_bug.cgi?id=121323
Summary Make FloatingObjects own it's FloatingObject instances
Bem Jones-Bey
Reported Friday, September 13, 2013 10:36:42 PM UTC
Make FloatingObjects own it's FloatingObject instances
Attachments
Patch (19.80 KB, patch)
2013-09-13 16:10 PDT, Bem Jones-Bey 		no flags
DetailsFormatted DiffDiff
Patch (19.79 KB, patch)
2013-09-16 14:28 PDT, Bem Jones-Bey 		no flags
DetailsFormatted DiffDiff
Patch (19.80 KB, patch)
2013-09-16 14:30 PDT, Bem Jones-Bey 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Bem Jones-Bey
Comment 1 Saturday, September 14, 2013 12:10:50 AM UTC
Created attachment 211598 [details] Patch
Alexandru Chiculita
Comment 2 Saturday, September 14, 2013 12:46:48 AM UTC
Comment on attachment 211598 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=211598&action=review Looks good. Thanks! > Source/WebCore/rendering/FloatingObjects.cpp:174 > +void FloatingObjects::exportToFloatInfoMap(RendererToFloatInfoMap& map) nit: The function name is confusing as it doesn't say it is releasing the ownership of its own content. > Source/WebCore/rendering/RenderBlock.cpp:3307 > + m_floatingObjects->add(newObj.release()); nit: Might be usefull to make "add" return a pointer to the floating object to avoid the manual store on the stack.
Bem Jones-Bey
Comment 3 Monday, September 16, 2013 10:28:28 PM UTC
Created attachment 211831 [details] Patch rename export method and make FloatingObjects::add have a return value per review feedback.
Bem Jones-Bey
Comment 4 Monday, September 16, 2013 10:30:10 PM UTC
Created attachment 211832 [details] Patch Add reviewer name
WebKit Commit Bot
Comment 5 Monday, September 16, 2013 11:00:37 PM UTC
Comment on attachment 211832 [details] Patch Clearing flags on attachment: 211832 Committed r155906: <http://trac.webkit.org/changeset/155906>
WebKit Commit Bot
Comment 6 Monday, September 16, 2013 11:00:39 PM UTC
All reviewed patches have been landed. Closing bug.
Philippe Wittenbergh
Comment 7 Wednesday, September 18, 2013 4:10:04 AM UTC
Did this cause bug 121532 ?
Bem Jones-Bey
Comment 8 Wednesday, September 18, 2013 4:40:33 PM UTC
(In reply to comment #7) > Did this cause bug 121532 ? It looks like it may have. I'm looking into it.
Simon Fraser (smfr)
Comment 9 Monday, October 27, 2014 9:31:52 PM UTC
This caused bug 138100.
Note You need to log in before you can comment on or make changes to this bug.