[webkit-reviews] review denied: [Bug 95907] Make computePositionedLogicalWidth and computePositionedLogicalWidthReplaced const : [Attachment 162367] Patch

[bugzilla-daemon at webkit.org]

Ojan Vafai <ojan at chromium.org> has denied Tony Chang <tony at chromium.org>'s
request for review:
Bug 95907: Make computePositionedLogicalWidth and
computePositionedLogicalWidthReplaced const
https://bugs.webkit.org/show_bug.cgi?id=95907

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

------- Additional Comments from Ojan Vafai <ojan at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=162367&action=review


> Source/WebCore/rendering/RenderBox.cpp:2676
> +					      stretchValues);
> +	   computedValues.m_extent = stretchValues.m_extent;
> +	   computedValues.m_position = stretchValues.m_position;
> +	   computedValues.m_margins.m_start = stretchValues.m_margins.m_start;
> +	   computedValues.m_margins.m_end = stretchValues.m_margins.m_end;

Why can't you just pass computedValues in instead of stretchValues?

> Source/WebCore/rendering/RenderBox.cpp:3184
> +    LayoutUnit& marginLogicalLeftAlias = style()->isLeftToRightDirection() ?
computedValues.m_margins.m_start : computedValues.m_margins.m_end;
> +    LayoutUnit& marginLogicalRightAlias = style()->isLeftToRightDirection()
? computedValues.m_margins.m_end : computedValues.m_margins.m_start;

Here and elsewhere, these don't look equivalent to me. 

FractionalLayoutUnit& FractionalLayoutBoxExtent::mutableLogicalLeft(WritingMode
writingMode)
{
    return isHorizontalWritingMode(writingMode) ? m_left : m_top;
}