Bug 96129 – Make RenderBox::computeLogicalWidthInRegion const

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

[Tony Chang]

This allows us to get rid of the hack in RenderBox::renderBoxRegionInfo.

[Tony Chang]

Created attachment 162799 [details]
Patch

[Tony Chang]

Comment on attachment 162799 [details]
Patch

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

> Source/WebCore/rendering/RenderBox.cpp:1706
> +        bool hasInvertedDirection = cb->style()->isLeftToRightDirection() != style()->isLeftToRightDirection();
> +        computeInlineDirectionMargins(cb, containerLogicalWidthForAutoMargins, computedValues.m_extent,
> +            hasInvertedDirection ? computedValues.m_margins.m_end : computedValues.m_margins.m_start,
> +            hasInvertedDirection ? computedValues.m_margins.m_start : computedValues.m_margins.m_end);

This variable was named incorrectly. I made the name correct by flipping the expression.

> Source/WebCore/rendering/RenderBox.cpp:1715
> +        bool hasInvertedDirection = cb->style()->isLeftToRightDirection() != style()->isLeftToRightDirection();
> +        if (hasInvertedDirection)
> +            computedValues.m_margins.m_start = containerLogicalWidth - computedValues.m_extent - cb->marginStartForChild(this);
> +        else
> +            computedValues.m_margins.m_end = containerLogicalWidth - computedValues.m_extent - cb->marginStartForChild(this);

I could make a helper function for this on MarginValues if you think it helps.

[Ojan Vafai]

Comment on attachment 162799 [details]
Patch

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

>> Source/WebCore/rendering/RenderBox.cpp:1715
>> +            computedValues.m_margins.m_end = containerLogicalWidth - computedValues.m_extent - cb->marginStartForChild(this);
> 
> I could make a helper function for this on MarginValues if you think it helps.

Can you just create a local variable before the if/else?
LayoutUnit newMargin = containerLogicalWidth - computedValues.m_extent - cb->marginStartForChild(this);

[Tony Chang]

Created attachment 162815 [details]
Patch for landing

[Tony Chang]

Comment on attachment 162815 [details]
Patch for landing

Want to let the ews bots chew on this first.

[WebKit Review Bot]

Comment on attachment 162815 [details]
Patch for landing

Clearing flags on attachment: 162815

Committed r127914: <http://trac.webkit.org/changeset/127914>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.