RESOLVED FIXED Bug 82767
Add a compile assert for the size of InlineFlowBox 
https://bugs.webkit.org/show_bug.cgi?id=82767
Summary Add a compile assert for the size of InlineFlowBox
Ryosuke Niwa
Reported 2012-03-30 12:10:33 PDT
Add a compile assert for the size of InlineFlowBox
Attachments
Patch (3.20 KB, patch)
2012-03-30 12:12 PDT, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1 2012-03-30 12:12:44 PDT
Created attachment 134856 [details] Patch
Tony Chang
Comment 2 2012-03-30 12:21:27 PDT
Comment on attachment 134856 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=134856&action=review > Source/WebCore/rendering/InlineFlowBox.cpp:50 > + uint32_t bitfields : 24; Can we drop the : 24? It's always going to round up to the packing size, right? > Source/WebCore/rendering/InlineFlowBox.h:315 > + unsigned m_hasAnnotationsBefore : 1; > + unsigned m_hasAnnotationsAfter : 1; Should we add getters and setters for these (I guess to RootInlineBox)?
Ryosuke Niwa
Comment 3 2012-03-30 12:23:57 PDT
Comment on attachment 134856 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=134856&action=review >> Source/WebCore/rendering/InlineFlowBox.h:315 >> + unsigned m_hasAnnotationsAfter : 1; > > Should we add getters and setters for these (I guess to RootInlineBox)? Maybe in the future. As is, these member variables are only used by RootInlineBox. It'll be nice if we had some macro or WTF template that enforces such a restriction.
Ojan Vafai
Comment 4 2012-03-30 13:47:04 PDT
Comment on attachment 134856 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=134856&action=review >>> Source/WebCore/rendering/InlineFlowBox.h:315 >>> + unsigned m_hasAnnotationsAfter : 1; >> >> Should we add getters and setters for these (I guess to RootInlineBox)? > > Maybe in the future. As is, these member variables are only used by RootInlineBox. > It'll be nice if we had some macro or WTF template that enforces such a restriction. Not adding getters/setters seems like it's asking for bugs down the road. Can you just make these private and manually add protected getters/setters? For something simple like this, I don't think we need complicated macros/templates.
WebKit Review Bot
Comment 5 2012-03-30 14:14:44 PDT
Comment on attachment 134856 [details] Patch Clearing flags on attachment: 134856 Committed r112725: <http://trac.webkit.org/changeset/112725>
WebKit Review Bot
Comment 6 2012-03-30 14:14:49 PDT
All reviewed patches have been landed. Closing bug.
Ryosuke Niwa
Comment 7 2012-03-30 14:21:12 PDT
(In reply to comment #4) > Not adding getters/setters seems like it's asking for bugs down the road. Can you just make these private and manually add protected getters/setters? For something simple like this, I don't think we need complicated macros/templates. I wouldn't think so since these member variables are only used in InlineFlowBox and RootInlineBox but I wouldn't object to making such a change.
Note You need to log in before you can comment on or make changes to this bug.