Use an OptionSet<> for LayoutBox::BaseTypeFlags
Created attachment 408613 [details] Patch
Comment on attachment 408613 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=408613&action=review > Source/WebCore/layout/layouttree/LayoutContainerBox.cpp:40 > + : Box(attributes, WTFMove(style), baseTypeFlags | OptionSet<BaseTypeFlag>(ContainerBoxFlag)) Is there a better way to do this?
Comment on attachment 408613 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=408613&action=review > Source/WebCore/layout/layouttree/LayoutBox.h:113 > + bool isInitialContainingBlock() const { return OptionSet<BaseTypeFlag>::fromRaw(m_baseTypeFlags).contains(InitialContainingBlockFlag); } I think we should make a getter that does OptionSet<BaseTypeFlag>::fromRaw(m_baseTypeFlags) so we don’t have to repeat it 5 times. Can be private and can call it flags() or baseTypeFlags(). >> Source/WebCore/layout/layouttree/LayoutContainerBox.cpp:40 >> + : Box(attributes, WTFMove(style), baseTypeFlags | OptionSet<BaseTypeFlag>(ContainerBoxFlag)) > > Is there a better way to do this? Doesn’t it work without the explicit constructor call? baseTypeFlags | ContainerBoxFlag I’d think it would because of the OptionSet constructor that takes a value of the underlying enumeration. If not (and I don’t know why not) we could: 1) Add an overload of the | operator that takes underlying enumeration type. But why is this needed? 2) Add a free function version of add. Then we could just write: add(baseTypeFlags, ContainerBoxFlag). Pretty sure that would work even if the operator doesn’t. > Source/WebCore/layout/layouttree/LayoutContainerBox.h:42 > + ContainerBox(Optional<ElementAttributes>, RenderStyle&&, OptionSet<BaseTypeFlag> = { Box::ContainerBoxFlag }); Not new, but no need for the "Box::" here.
Created attachment 408619 [details] Patch
Committed r266985: <https://trac.webkit.org/changeset/266985> All reviewed patches have been landed. Closing bug and clearing flags on attachment 408619 [details].
<rdar://problem/68783520>