[webkit-reviews] review canceled: [Bug 92154] Add horizontal writing mode logic to FractionalLayoutRect and update RenderBox, RenderBlock : [Attachment 154140] Patch

[bugzilla-daemon at webkit.org]

Julien Chaffraix <jchaffraix at webkit.org> has canceled Emil A Eklund
<eae at chromium.org>'s request for review:
Bug 92154: Add horizontal writing mode logic to FractionalLayoutRect and update
RenderBox, RenderBlock
https://bugs.webkit.org/show_bug.cgi?id=92154

Attachment 154140: Patch
https://bugs.webkit.org/attachment.cgi?id=154140&action=review

------- Additional Comments from Julien Chaffraix <jchaffraix at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=154140&action=review


To make an educated decision here, we would need to know the potential for code
sharing. The patch as-is doesn't increase the sharing much and could negatively
impact the speed. IMHO it does hurt readability so we would need a strong win
on the sharing side to offset the rest.

> Source/WebCore/platform/graphics/FractionalLayoutRect.cpp:50
> +FractionalLayoutUnit FractionalLayoutRect::logicalLeft(const RenderStyle*
style) const

Not really a fan of passing around our style() everywhere :-/

On top of this being annoying, it's an extra function call (which is not
inlined where most of the callers are) and an extra argument to pass so it's
likely a performance loss. Considering how much those functions are called, the
performance impact should be evaluated before proceeding.