Bug 218197 – Clean up BoxSide and BorderEdge code

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Simon Fraser (smfr)]

Clean up BoxSide and BorderEdge code

[Simon Fraser (smfr)]

Created attachment 412334 [details]
Patch

[Sam Weinig]

Comment on attachment 412334 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=412334&action=review

> Source/WebCore/platform/RectEdges.h:52
> +    T& at(BoxSide side) { return m_sides[static_cast<size_t>(side)]; }

The more traditional thing would be to use operation[] here.

> Source/WebCore/platform/RectEdges.h:89
> +    std::array<T, 4> m_sides { };

I don't think you need the braces at all here. It should default initialize the members by default.

> Source/WebCore/platform/text/WritingMode.h:131
> +constexpr std::initializer_list<BoxSide> allBoxSides = { BoxSide::Top, BoxSide::Right, BoxSide::Bottom, BoxSide::Left };

I think this would make more sense as std::array<BoxSide, 4>. Keeping initializer_list is mostly best used as a parameter to a function.

I also think we should try to replace this, perhaps as a follow up, with specific iterators on RectEdges.

> Source/WebCore/rendering/RenderBoxModelObject.cpp:1799
> +    static constexpr std::initializer_list<BoxSide> paintOrderSides = { BoxSide::Top, BoxSide::Bottom, BoxSide::Left, BoxSide::Right };

like with the other use, I think this would be better as a std::array.

[Simon Fraser (smfr)]

Created attachment 412385 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 412830 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 412831 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 412844 [details]
Patch

[EWS]

Committed r269228: <https://trac.webkit.org/changeset/269228>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 412844 [details].

[Radar WebKit Bug Importer]

<rdar://problem/70918166>