Abs-positioned objects should move with inline rel-positioned containers when the latter change wid…

Abs-positioned objects should move with inline rel-positioned containers when the latter change width

In line layout the position of an absolute positioned object that has a fixed
offset from an inline rel-positioned container needs to move if the container changes
width. If we are positioning an abspos object inside a rel-pos container during line
layout then its container has probably changed width - so force a layout on the object
if its position depends on the edge that moves with the width.

BUG=338856
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=179335

[esprehn, 2014-07-23 22:49:36]

You're moving and refactoring a lot of code here all at once, can we break this
up?

[esprehn, 2014-07-28 19:35:15]

https://codereview.chromium.org/372023002/diff/80001/Source/core/rendering/Re...
File Source/core/rendering/RenderBox.cpp (right):

https://codereview.chromium.org/372023002/diff/80001/Source/core/rendering/Re...
Source/core/rendering/RenderBox.cpp:1947: ASSERT(isOutOfFlowPositioned() &&
container()->isRenderInline() && container()->isRelPositioned());
This assert doesn't make sense, the caller of this function has a case for
isOutOfFlowPositioned and one for when it's not. Why do we need to be out of
flow positioned if our container is inline? That doesn't make sense.

https://codereview.chromium.org/372023002/diff/80001/Source/core/rendering/li...
File Source/core/rendering/line/BreakingContextInlineHeaders.h (right):

https://codereview.chromium.org/372023002/diff/80001/Source/core/rendering/li...
Source/core/rendering/line/BreakingContextInlineHeaders.h:216:
child->moveWithEdgeOfInlineContainerIfNecessary(child->isHorizontalWritingMode());
Why not isHorizontal() here?

[rhogan, 2014-07-28 19:43:14]

On 2014/07/28 at 19:35:15, esprehn wrote:
>
https://codereview.chromium.org/372023002/diff/80001/Source/core/rendering/Re...
> File Source/core/rendering/RenderBox.cpp (right):
> 
>
https://codereview.chromium.org/372023002/diff/80001/Source/core/rendering/Re...
> Source/core/rendering/RenderBox.cpp:1947: ASSERT(isOutOfFlowPositioned() &&
container()->isRenderInline() && container()->isRelPositioned());
> This assert doesn't make sense, the caller of this function has a case for
isOutOfFlowPositioned and one for when it's not. Why do we need to be out of
flow positioned if our container is inline? That doesn't make sense.

Can you check again? The assert holds for both callers - the only time an
out-of-flow block's parent is inline is when it's relpositioned.

> 
>
https://codereview.chromium.org/372023002/diff/80001/Source/core/rendering/li...
> File Source/core/rendering/line/BreakingContextInlineHeaders.h (right):
> 
>
https://codereview.chromium.org/372023002/diff/80001/Source/core/rendering/li...
> Source/core/rendering/line/BreakingContextInlineHeaders.h:216:
child->moveWithEdgeOfInlineContainerIfNecessary(child->isHorizontalWritingMode());
> Why not isHorizontal() here?
That function is only available for lineboxes.

[rhogan, 2014-07-28 19:44:57]

On 2014/07/28 at 19:43:14, rhogan wrote:
> Can you check again? The assert holds for both callers - the only time an
out-of-flow block's parent is inline is when it's relpositioned.
s/parent/container/

[esprehn, 2014-07-28 19:51:24]

On 2014/07/28 at 19:44:57, robhogan wrote:
> On 2014/07/28 at 19:43:14, rhogan wrote:
> > Can you check again? The assert holds for both callers - the only time an
out-of-flow block's parent is inline is when it's relpositioned.
> s/parent/container/

If that's true then why isn't your code inside the isOutOfFlowPositioned() side
of the branch in the caller?

[esprehn, 2014-07-30 05:21:24]

lgtm, but please fix your description. The second paragraph seems to relate to a
refactor that's not in this patch anymore.

[rhogan, 2014-07-31 17:26:10]

The CQ bit was checked by robhogan@gmail.com

[commit-bot: I haz the power, 2014-07-31 17:27:05]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/robhogan@gmail.com/372023002/100001

[commit-bot: I haz the power, 2014-07-31 18:21:36]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  mac_blink_compile_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_compile_dbg/bu...)
  linux_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu_trigger...)

[commit-bot: I haz the power, 2014-07-31 18:46:53]

Change committed as 179335