Don't update height in LayoutBlock::markFixedPositionObjectForLayoutIfNeeded.

Don't update height in LayoutBlock::markFixedPositionObjectForLayoutIfNeeded.

This prevented LayoutBlockFlow::layoutBlockFlow from seeing that the height had
changed.  As a result, it failed to set relayoutChildren = true.

The bug affects fixed-pos objects in abs-pos containers, so it got worse with
http://crrev.com/1172153004 but existed before.

An equivalent patch for width was made in http://crrev.com/171463003.

Additionally, update the early return for the case where the abs-pos container
is the LayoutView.  (This is just an optimization.)

BUG=640639
Committed: https://crrev.com/1abcf254f3e55be39879ca76d268192faf64a254
Cr-Commit-Position: refs/heads/master@{#426515}

[skobes, 2016-10-19 01:58:41]

Description was changed from

==========
Don't update height in LayoutBlock::markFixedPositionObjectForLayoutIfNeeded.

BUG=
==========

to

==========
Don't update height in LayoutBlock::markFixedPositionObjectForLayoutIfNeeded.

This prevented LayoutBlockFlow::layoutBlockFlow from seeing that the height had
changed.  As a result, it failed to set relayoutChildren = true.

The bug affects fixed-pos objects in abs-pos containers, so it got worse with
http://crrev.com/1172153004 but existed before.

An equivalent patch for width was made in http://crrev.com/171463003.

BUG=640639
==========

[skobes, 2016-10-19 02:02:15]

The CQ bit was checked by skobes@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-19 02:03:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[skobes, 2016-10-19 02:22:59]

Description was changed from

==========
Don't update height in LayoutBlock::markFixedPositionObjectForLayoutIfNeeded.

This prevented LayoutBlockFlow::layoutBlockFlow from seeing that the height had
changed.  As a result, it failed to set relayoutChildren = true.

The bug affects fixed-pos objects in abs-pos containers, so it got worse with
http://crrev.com/1172153004 but existed before.

An equivalent patch for width was made in http://crrev.com/171463003.

BUG=640639
==========

to

==========
Don't update height in LayoutBlock::markFixedPositionObjectForLayoutIfNeeded.

This prevented LayoutBlockFlow::layoutBlockFlow from seeing that the height had
changed.  As a result, it failed to set relayoutChildren = true.

The bug affects fixed-pos objects in abs-pos containers, so it got worse with
http://crrev.com/1172153004 but existed before.

An equivalent patch for width was made in http://crrev.com/171463003.

Additionally, update the early return for the case where the abs-pos container
is the LayoutView.  (This is just an optimization.)

BUG=640639
==========

[commit-bot: I haz the power, 2016-10-19 03:37:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-19 03:37:49]

Dry run: Exceeded global retry quota

[skobes, 2016-10-19 04:22:21]

The CQ bit was checked by skobes@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-19 04:23:47]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-19 05:11:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-19 05:11:20]

Dry run: Exceeded global retry quota

[skobes, 2016-10-19 15:20:38]

skobes@chromium.org changed reviewers:
+ esprehn@chromium.org

[mstensho (USE GERRIT), 2016-10-19 18:24:30]

mstensho@opera.com changed reviewers:
+ mstensho@opera.com

[mstensho (USE GERRIT), 2016-10-19 18:24:30]

https://chromiumcodereview.appspot.com/2429113002/diff/1/third_party/WebKit/L...
File
third_party/WebKit/LayoutTests/fast/block/positioning/fixed-in-abs-height-change.html
(right):

https://chromiumcodereview.appspot.com/2429113002/diff/1/third_party/WebKit/L...
third_party/WebKit/LayoutTests/fast/block/positioning/fixed-in-abs-height-change.html:36:
assert_equals(box.offsetTop, 350);
This value depends on the viewport height, which is not so great for testing, in
general. Do you really need to use percentages on the fixedpos? Seems to me it's
just as broken if you used px values (without your fix, that is).

https://chromiumcodereview.appspot.com/2429113002/diff/1/third_party/WebKit/S...
File third_party/WebKit/Source/core/layout/LayoutBlock.cpp (right):

https://chromiumcodereview.appspot.com/2429113002/diff/1/third_party/WebKit/S...
third_party/WebKit/Source/core/layout/LayoutBlock.cpp:770: // If
positionedObject is fixed-position and moves with an absolute-
"fixed-positioned"

[skobes, 2016-10-19 21:47:33]

The CQ bit was checked by skobes@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-19 21:48:09]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[skobes, 2016-10-19 21:48:40]

https://codereview.chromium.org/2429113002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/fast/block/positioning/fixed-in-abs-height-change.html
(right):

https://codereview.chromium.org/2429113002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/block/positioning/fixed-in-abs-height-change.html:36:
assert_equals(box.offsetTop, 350);
On 2016/10/19 18:24:30, mstensho wrote:
> This value depends on the viewport height, which is not so great for testing,
in
> general. Do you really need to use percentages on the fixedpos? Seems to me
it's
> just as broken if you used px values (without your fix, that is).

Good point, done.

https://codereview.chromium.org/2429113002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutBlock.cpp (right):

https://codereview.chromium.org/2429113002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutBlock.cpp:770: // If
positionedObject is fixed-position and moves with an absolute-
On 2016/10/19 18:24:30, mstensho wrote:
> "fixed-positioned"

Done.

[commit-bot: I haz the power, 2016-10-19 23:43:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-19 23:43:34]

Dry run: This issue passed the CQ dry run.

[mstensho (USE GERRIT), 2016-10-20 08:58:21]

lgtm, nice test improvement!

[skobes, 2016-10-20 16:57:59]

The CQ bit was checked by skobes@chromium.org

[commit-bot: I haz the power, 2016-10-20 16:58:24]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-20 17:17:43]

Description was changed from

==========
Don't update height in LayoutBlock::markFixedPositionObjectForLayoutIfNeeded.

This prevented LayoutBlockFlow::layoutBlockFlow from seeing that the height had
changed.  As a result, it failed to set relayoutChildren = true.

The bug affects fixed-pos objects in abs-pos containers, so it got worse with
http://crrev.com/1172153004 but existed before.

An equivalent patch for width was made in http://crrev.com/171463003.

Additionally, update the early return for the case where the abs-pos container
is the LayoutView.  (This is just an optimization.)

BUG=640639
==========

to

==========
Don't update height in LayoutBlock::markFixedPositionObjectForLayoutIfNeeded.

This prevented LayoutBlockFlow::layoutBlockFlow from seeing that the height had
changed.  As a result, it failed to set relayoutChildren = true.

The bug affects fixed-pos objects in abs-pos containers, so it got worse with
http://crrev.com/1172153004 but existed before.

An equivalent patch for width was made in http://crrev.com/171463003.

Additionally, update the early return for the case where the abs-pos container
is the LayoutView.  (This is just an optimization.)

BUG=640639
==========

[commit-bot: I haz the power, 2016-10-20 17:17:44]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-10-21 13:19:32]

Description was changed from

==========
Don't update height in LayoutBlock::markFixedPositionObjectForLayoutIfNeeded.

This prevented LayoutBlockFlow::layoutBlockFlow from seeing that the height had
changed.  As a result, it failed to set relayoutChildren = true.

The bug affects fixed-pos objects in abs-pos containers, so it got worse with
http://crrev.com/1172153004 but existed before.

An equivalent patch for width was made in http://crrev.com/171463003.

Additionally, update the early return for the case where the abs-pos container
is the LayoutView.  (This is just an optimization.)

BUG=640639
==========

to

==========
Don't update height in LayoutBlock::markFixedPositionObjectForLayoutIfNeeded.

This prevented LayoutBlockFlow::layoutBlockFlow from seeing that the height had
changed.  As a result, it failed to set relayoutChildren = true.

The bug affects fixed-pos objects in abs-pos containers, so it got worse with
http://crrev.com/1172153004 but existed before.

An equivalent patch for width was made in http://crrev.com/171463003.

Additionally, update the early return for the case where the abs-pos container
is the LayoutView.  (This is just an optimization.)

BUG=640639

Committed: https://crrev.com/1abcf254f3e55be39879ca76d268192faf64a254
Cr-Commit-Position: refs/heads/master@{#426515}
==========

[commit-bot: I haz the power, 2016-10-21 13:19:34]

Patchset 2 (id:??) landed as
https://crrev.com/1abcf254f3e55be39879ca76d268192faf64a254
Cr-Commit-Position: refs/heads/master@{#426515}