The idea here is that when certain style attribute changes, the invalidation logic depends on the type of the formatting context the box lives in.
Created attachment 339407 [details] Patch
Comment on attachment 339407 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=339407&action=review > Source/WebCore/layout/FormattingState.h:60 > + enum BaseTypeFlag { > + BlockStateFlag = 1 << 0, > + InlineStateFlag = 1 << 1 > + }; > + > + FormattingState(Ref<FloatingState>&&, BaseTypeFlags); These two are mutually exclusive so could be replaced with 'enum class Type { Block, Inline }'. But maybe there is going to be a deeper hierarchy of FormattingStates and this will expand? This could be done with virtual functions but maybe it is a good idea to avoid them entirely.
Created attachment 339411 [details] Patch
Comment on attachment 339411 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=339411&action=review > Source/WebCore/layout/FormattingState.h:57 > + enum StateType { > + BlockState, > + InlineState > + }; I would go with enum class Type { Block, Inline } This is all scoped to FormattingState so repeating word 'State' is redundant. > Source/WebCore/layout/LayoutContext.h:62 > + enum UpdateType { enum class?
Created attachment 339417 [details] Patch
Created attachment 339419 [details] Patch
Comment on attachment 339419 [details] Patch Clearing flags on attachment: 339419 Committed r231312: <https://trac.webkit.org/changeset/231312>
All reviewed patches have been landed. Closing bug.
<rdar://problem/39944996>
Comment on attachment 339419 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=339419&action=review > Source/WebCore/layout/LayoutContext.h:69 > + enum class UpdateType { > + Overflow = 1 << 0, > + Position = 1 << 1, > + Size = 1 << 2, > + All = Overflow | Position | Size > + }; > + void markNeedsUpdate(const Box&, OptionSet<UpdateType>); 'All' value like this is awkward with OptionSet (it will assert unless used with OptionSet::fromRaw), the enum should contain single bit values only. Defining constant like static constexpr OptionSet<UpdateType> UpdateAll = { Overflow, Position, Size }; should work (after 185298).
(In reply to Antti Koivisto from comment #10) > Comment on attachment 339419 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=339419&action=review > > > Source/WebCore/layout/LayoutContext.h:69 > > + enum class UpdateType { > > + Overflow = 1 << 0, > > + Position = 1 << 1, > > + Size = 1 << 2, > > + All = Overflow | Position | Size > > + }; > > + void markNeedsUpdate(const Box&, OptionSet<UpdateType>); > > 'All' value like this is awkward with OptionSet (it will assert unless used > with OptionSet::fromRaw), the enum should contain single bit values only. > Defining constant like > > static constexpr OptionSet<UpdateType> UpdateAll = { Overflow, Position, > Size }; > > should work (after 185298). Good point. Will fix it soon.