Fix assertions related to floats and z-order lists when compositing opaque scrollers

Fix assertions related to floats and z-order lists when compositing opaque scrollers

The codepath for compositing opaque scrollers, enabled by
--enable-blink-feature="CompositeOpaqueScrollbars", was triggering two asserts.

One assertion was due to querying stacking order lists when those lists were
dirty. The assertion is fixed by updating the stacking order lists if needed
before the query, only if we actually need to look at the list.

The other assertion was a side-effect of an earlier change to remove a
different assertion, https://codereview.chromium.org/2191953002. The code
in question is attempting to update the shouldPaint flag of a FloatingObject
when that object has had a change in selfPaintingLayer status. But because
selfPaintingLayer status has already been updated, and the query for
FloatingObject::shouldPaint() checks the current selfPaintingLayer status,
we have no way of finding out if a floating object with a selfpaintingLayer
was previously shouldPaint. We were requiring that shouldPaint return
true so that we could set it false and return, but instead we were falling
out of the loop and hitting an ASSERT_NOT_REACHED.

The fix is to not check the previous status and just set shouldPaint to false
for all FloatingObject where the object has a selfPaintingLayer, and not assert
not reached as now we might exit the loop. We are no longer returning as soon
as we update the shouldPaint to false because we can no longer be sure that
we are actually changing the value of shouldPaint.

The lack of assertions allows 2 webkit_unit_tests to pass without crashing,
and prevents crashes in a slew of LayoutTests. Of note,
fast/overflow/overflow-float-stacking.html no longer crashes, but does fail
analogously to fast/overflow/overflow-stacking.html. In
https://codereview.chromium.org/1826013002 we had decided to live with that
failure as it is a manifestation of the fundamental compositing bug that
users on high DPI devices have always seen.

R=wangxianzhu@chromium.org,flackr@chromium.org
BUG=593097, 381840
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Committed: https://crrev.com/7081b331ccd1903e0b3cc539d8decccb0a09863d
Cr-Commit-Position: refs/heads/master@{#420477}

[Stephen Chennney, 2016-09-22 19:51:42]

Description was changed from

==========
Fix assertions related to floats and z-order lists when compositing opaque
scrollers

The codepath for compositing opaque scrollers, enabled by
--enable-blink-feature="CompositeOpaqueScrollbars", was triggering two asserts.

One assertion was due to querying stacking order lists when those lists were
dirty. The assertion is fixed by updating the stacking order lists if needed
before the query, only if we actually need to look at the list.

The other assertion was a side-effect of an earlier change to remove a
different assertion, https://codereview.chromium.org/2191953002. The code
in question is attempting to update the shouldPaint flag of a FloatingObject
when that object has had a change in selfPaintingLayer status. But because
selfPaintingLayer status has already been updated, and the query for
FloatingObject::shouldPaint() checks the current selfPaintingLayer status,
we have no way of finding out if a floating object with a selfpaintingLayer
was previously shouldPaint. We were requiring that shouldPaint return
true so that we could set it false and return, but instead we were falling
out of the loop and hitting an ASSERT_NOT_REACHED.

The fix is to not check the previous status and just set shouldPaint to false
for all FloatingObject where the object has a selfPaintingLayer, and not assert
not reached as now we might exit the loop. We are no longer returning as soon
as we update the shouldPaint to false because we can no longer be sure that
we are actually changing the value of shouldPaint.

The lack of assertions allows 2 webkit_unit_tests to pass without crashing,
and prevents crashes in a slew of LayoutTests. Of note,
fast/overflow/overflow-float-stacking.html no longer crashes, but does fail
analogously to fast/overflow/overflow-stacking.html. In
https://codereview.chromium.org/1826013002 we had decided to live with that
failure as it is a manifestation of the fundamental compositing bug that
users on high DPI devices have always seen.

R=wangxianzhu@chromium.org,flackr@chromium.org
BUG=593097,381840
==========

to

==========
Fix assertions related to floats and z-order lists when compositing opaque
scrollers

The codepath for compositing opaque scrollers, enabled by
--enable-blink-feature="CompositeOpaqueScrollbars", was triggering two asserts.

One assertion was due to querying stacking order lists when those lists were
dirty. The assertion is fixed by updating the stacking order lists if needed
before the query, only if we actually need to look at the list.

The other assertion was a side-effect of an earlier change to remove a
different assertion, https://codereview.chromium.org/2191953002. The code
in question is attempting to update the shouldPaint flag of a FloatingObject
when that object has had a change in selfPaintingLayer status. But because
selfPaintingLayer status has already been updated, and the query for
FloatingObject::shouldPaint() checks the current selfPaintingLayer status,
we have no way of finding out if a floating object with a selfpaintingLayer
was previously shouldPaint. We were requiring that shouldPaint return
true so that we could set it false and return, but instead we were falling
out of the loop and hitting an ASSERT_NOT_REACHED.

The fix is to not check the previous status and just set shouldPaint to false
for all FloatingObject where the object has a selfPaintingLayer, and not assert
not reached as now we might exit the loop. We are no longer returning as soon
as we update the shouldPaint to false because we can no longer be sure that
we are actually changing the value of shouldPaint.

The lack of assertions allows 2 webkit_unit_tests to pass without crashing,
and prevents crashes in a slew of LayoutTests. Of note,
fast/overflow/overflow-float-stacking.html no longer crashes, but does fail
analogously to fast/overflow/overflow-stacking.html. In
https://codereview.chromium.org/1826013002 we had decided to live with that
failure as it is a manifestation of the fundamental compositing bug that
users on high DPI devices have always seen.

R=wangxianzhu@chromium.org,flackr@chromium.org
BUG=593097,381840
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[Stephen Chennney, 2016-09-22 19:52:37]

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

[commit-bot: I haz the power, 2016-09-22 19:53:16]

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

[Stephen Chennney, 2016-09-22 19:53:51]

To see the asserts before and after you need to be running in a debug build with
either --enable-prefer-compositing-to-lcd-text or
--enable-blink-features="CompositeOpaqueScrollers".

[Xianzhu, 2016-09-22 20:27:48]

lgtm

[Stephen Chennney, 2016-09-22 20:51:37]

The CQ bit was unchecked by schenney@chromium.org

[Stephen Chennney, 2016-09-22 20:51:38]

The CQ bit was checked by schenney@chromium.org

[commit-bot: I haz the power, 2016-09-22 20:52:08]

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-09-22 22:09:45]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-09-22 22:11:51]

Description was changed from

==========
Fix assertions related to floats and z-order lists when compositing opaque
scrollers

The codepath for compositing opaque scrollers, enabled by
--enable-blink-feature="CompositeOpaqueScrollbars", was triggering two asserts.

One assertion was due to querying stacking order lists when those lists were
dirty. The assertion is fixed by updating the stacking order lists if needed
before the query, only if we actually need to look at the list.

The other assertion was a side-effect of an earlier change to remove a
different assertion, https://codereview.chromium.org/2191953002. The code
in question is attempting to update the shouldPaint flag of a FloatingObject
when that object has had a change in selfPaintingLayer status. But because
selfPaintingLayer status has already been updated, and the query for
FloatingObject::shouldPaint() checks the current selfPaintingLayer status,
we have no way of finding out if a floating object with a selfpaintingLayer
was previously shouldPaint. We were requiring that shouldPaint return
true so that we could set it false and return, but instead we were falling
out of the loop and hitting an ASSERT_NOT_REACHED.

The fix is to not check the previous status and just set shouldPaint to false
for all FloatingObject where the object has a selfPaintingLayer, and not assert
not reached as now we might exit the loop. We are no longer returning as soon
as we update the shouldPaint to false because we can no longer be sure that
we are actually changing the value of shouldPaint.

The lack of assertions allows 2 webkit_unit_tests to pass without crashing,
and prevents crashes in a slew of LayoutTests. Of note,
fast/overflow/overflow-float-stacking.html no longer crashes, but does fail
analogously to fast/overflow/overflow-stacking.html. In
https://codereview.chromium.org/1826013002 we had decided to live with that
failure as it is a manifestation of the fundamental compositing bug that
users on high DPI devices have always seen.

R=wangxianzhu@chromium.org,flackr@chromium.org
BUG=593097,381840
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Fix assertions related to floats and z-order lists when compositing opaque
scrollers

The codepath for compositing opaque scrollers, enabled by
--enable-blink-feature="CompositeOpaqueScrollbars", was triggering two asserts.

One assertion was due to querying stacking order lists when those lists were
dirty. The assertion is fixed by updating the stacking order lists if needed
before the query, only if we actually need to look at the list.

The other assertion was a side-effect of an earlier change to remove a
different assertion, https://codereview.chromium.org/2191953002. The code
in question is attempting to update the shouldPaint flag of a FloatingObject
when that object has had a change in selfPaintingLayer status. But because
selfPaintingLayer status has already been updated, and the query for
FloatingObject::shouldPaint() checks the current selfPaintingLayer status,
we have no way of finding out if a floating object with a selfpaintingLayer
was previously shouldPaint. We were requiring that shouldPaint return
true so that we could set it false and return, but instead we were falling
out of the loop and hitting an ASSERT_NOT_REACHED.

The fix is to not check the previous status and just set shouldPaint to false
for all FloatingObject where the object has a selfPaintingLayer, and not assert
not reached as now we might exit the loop. We are no longer returning as soon
as we update the shouldPaint to false because we can no longer be sure that
we are actually changing the value of shouldPaint.

The lack of assertions allows 2 webkit_unit_tests to pass without crashing,
and prevents crashes in a slew of LayoutTests. Of note,
fast/overflow/overflow-float-stacking.html no longer crashes, but does fail
analogously to fast/overflow/overflow-stacking.html. In
https://codereview.chromium.org/1826013002 we had decided to live with that
failure as it is a manifestation of the fundamental compositing bug that
users on high DPI devices have always seen.

R=wangxianzhu@chromium.org,flackr@chromium.org
BUG=593097,381840
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Committed: https://crrev.com/7081b331ccd1903e0b3cc539d8decccb0a09863d
Cr-Commit-Position: refs/heads/master@{#420477}
==========

[commit-bot: I haz the power, 2016-09-22 22:11:52]

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