Skip PaintPhaseDescendantBlockBackgroundsOnly phase if possible

Skip PaintPhaseDescendantBlockBackgroundsOnly phase if possible

Set containing self-painting layer's
needsPaintPhaseDescendantBlockBackground() if needed during paint
invalidation, and skip the phase of the layer during painting if
the flag is not set.

Also modified how TableSectionPainter and TableRowPainter handle
PaintPhaseSelfBlockBackgroundOnly to make this CL applicable to them.
Previously they ignored PaintPhaseSelfBlockBackgroundOnly and painted
their own backgrounds in PaintPhaseDescendantBlockBackgroundsOnly behind
cell. This conflicts with how LayoutBox::invalidatePaintIfNeeded() sets
needsPaintPhaseDescendantBlockBackground().

BUG=574938
TEST=PaintLayerPainterTest.PaintPhaseBlockBackground

Committed: https://crrev.com/99290b961c890e340db97b3d63028b3cf5511d32
Cr-Commit-Position: refs/heads/master@{#372730}

[Xianzhu, 2016-01-22 22:14:38]

Description was changed from

==========
Skip PaintPhaseDescendantBlockBackgroundsOnly phase if possible

BUG=574938
TEST=PaintLayerPainterTest.PaintPhaseBlockBackground
==========

to

==========
Skip PaintPhaseDescendantBlockBackgroundsOnly phase if possible

Set containing self-painting layer's
needsPaintPhaseDescendantBlockBackground() if needed during paint
invalidation, and skip the phase of the layer during painting if
the flag is not set.

Also modified how TableSectionPainter and TableRowPainter handle
PaintPhaseSelfBlockBackgroundOnly to make this CL applicable to them.
Previously they ignored PaintPhaseSelfBlockBackgroundOnly and painted
their own backgrounds in PaintPhaseDescendantBlockBackgroundsOnly behind
cell. This conflicts with how LayoutBox::invalidatePaintIfNeeded() sets
needsPaintPhaseDescendantBlockBackground().

BUG=574938
TEST=PaintLayerPainterTest.PaintPhaseBlockBackground
==========

[Xianzhu, 2016-01-22 22:14:54]

wangxianzhu@chromium.org changed reviewers:
+ chrishtr@chromium.org

[chrishtr, 2016-01-26 21:43:15]

https://codereview.chromium.org/1626623002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right):

https://codereview.chromium.org/1626623002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutBox.cpp:1470: if (isFloating())
Floats? Looks like you need to sync this patch.

https://codereview.chromium.org/1626623002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutObject.cpp (right):

https://codereview.chromium.org/1626623002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutObject.cpp:1244: static void
assertEnclosingSelfPaintingLayerHasSetNeedsRepaint(const LayoutObject&
layoutObject)
Again here, need to sync past this change..

[Xianzhu, 2016-01-26 21:56:15]

https://codereview.chromium.org/1626623002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right):

https://codereview.chromium.org/1626623002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutBox.cpp:1470: if (isFloating())
On 2016/01/26 21:43:15, chrishtr wrote:
> Floats? Looks like you need to sync this patch.

Sorry, I uploaded this patch set including all empty-phase optimizations to test
on telemetry cluster. I should have created another CL for this. Will remove
this patch set and the remaining latest one is based on the float phase one.

[Xianzhu, 2016-01-26 21:56:29]

Patchset #3 (id:40001) has been deleted

[Xianzhu, 2016-01-26 22:12:51]

On 2016/01/26 21:56:15, Xianzhu wrote:
>
https://codereview.chromium.org/1626623002/diff/40001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right):
> 
>
https://codereview.chromium.org/1626623002/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/layout/LayoutBox.cpp:1470: if (isFloating())
> On 2016/01/26 21:43:15, chrishtr wrote:
> > Floats? Looks like you need to sync this patch.
> 
> Sorry, I uploaded this patch set including all empty-phase optimizations to
test
> on telemetry cluster. I should have created another CL for this. Will remove
> this patch set and the remaining latest one is based on the float phase one.

Done. Please review the latest patch set.

[Xianzhu, 2016-01-27 02:16:00]

Patchset #3 (id:60001) has been deleted

[chrishtr, 2016-01-27 23:51:47]

https://codereview.chromium.org/1626623002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/TableRowPainter.cpp (right):

https://codereview.chromium.org/1626623002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/TableRowPainter.cpp:30:
TableCellPainter(*cell).paintBackgroundsBehindCell(paintInfoForCells,
paintOffset, &m_layoutTableRow, DisplayItem::TableCellBackgroundFromRow);
Won't this paint twice now? For PaintPhaseSelfBlockBackgroundOnly and
PaintPhaseBlockBackground?

Also, it would be better to break this out into its own patch. The changes to
table painting here
are not trivial to verify.

[Xianzhu, 2016-01-28 00:13:14]

https://codereview.chromium.org/1626623002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/paint/TableRowPainter.cpp (right):

https://codereview.chromium.org/1626623002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/paint/TableRowPainter.cpp:30:
TableCellPainter(*cell).paintBackgroundsBehindCell(paintInfoForCells,
paintOffset, &m_layoutTableRow, DisplayItem::TableCellBackgroundFromRow);
On 2016/01/27 23:51:47, chrishtr wrote:
> Won't this paint twice now? For PaintPhaseSelfBlockBackgroundOnly and
> PaintPhaseBlockBackground?
> 

No. PaintPhaseBlockBackground is like (PaintPhaseSelfBlockBackgroundOnly |
PaintPhaseDescendantBlockBackgroundsOnly) (but we can't actually use them as
bits, so use shouldPaintSelfBlockBackground() and
shouldPaintDescendantBlockBackgrounds()). PaintPhaseBlockBackground never happen
when either or both of the xxxOnly phases happens.

> Also, it would be better to break this out into its own patch. The changes to
> table painting here
> are not trivial to verify.

Done. Ptal at https://codereview.chromium.org/1646743002.

[chrishtr, 2016-02-01 17:43:49]

https://codereview.chromium.org/1626623002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right):

https://codereview.chromium.org/1626623002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/LayoutBox.cpp:1474: if
(hasBoxDecorationBackground() || (hasOverflowClip() &&
scrollableArea()->hasOverflowControls())) {
Why the is this logic the right logic? In particular the part about controls and
overflow clip. Please also add a comment explaining.

[Xianzhu, 2016-02-01 17:58:39]

https://codereview.chromium.org/1626623002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right):

https://codereview.chromium.org/1626623002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/LayoutBox.cpp:1474: if
(hasBoxDecorationBackground() || (hasOverflowClip() &&
scrollableArea()->hasOverflowControls())) {
On 2016/02/01 17:43:49, chrishtr wrote:
> Why the is this logic the right logic? In particular the part about controls
and
> overflow clip. Please also add a comment explaining.

Added comment:
// We also paint overflow controls in background phase.

[chrishtr, 2016-02-01 18:41:12]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2016-02-01 18:41:12]

lgtm

[commit-bot: I haz the power, 2016-02-01 18:42:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1626623002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1626623002/140001

[commit-bot: I haz the power, 2016-02-01 19:58:46]

Description was changed from

==========
Skip PaintPhaseDescendantBlockBackgroundsOnly phase if possible

Set containing self-painting layer's
needsPaintPhaseDescendantBlockBackground() if needed during paint
invalidation, and skip the phase of the layer during painting if
the flag is not set.

Also modified how TableSectionPainter and TableRowPainter handle
PaintPhaseSelfBlockBackgroundOnly to make this CL applicable to them.
Previously they ignored PaintPhaseSelfBlockBackgroundOnly and painted
their own backgrounds in PaintPhaseDescendantBlockBackgroundsOnly behind
cell. This conflicts with how LayoutBox::invalidatePaintIfNeeded() sets
needsPaintPhaseDescendantBlockBackground().

BUG=574938
TEST=PaintLayerPainterTest.PaintPhaseBlockBackground
==========

to

==========
Skip PaintPhaseDescendantBlockBackgroundsOnly phase if possible

Set containing self-painting layer's
needsPaintPhaseDescendantBlockBackground() if needed during paint
invalidation, and skip the phase of the layer during painting if
the flag is not set.

Also modified how TableSectionPainter and TableRowPainter handle
PaintPhaseSelfBlockBackgroundOnly to make this CL applicable to them.
Previously they ignored PaintPhaseSelfBlockBackgroundOnly and painted
their own backgrounds in PaintPhaseDescendantBlockBackgroundsOnly behind
cell. This conflicts with how LayoutBox::invalidatePaintIfNeeded() sets
needsPaintPhaseDescendantBlockBackground().

BUG=574938
TEST=PaintLayerPainterTest.PaintPhaseBlockBackground
==========

[commit-bot: I haz the power, 2016-02-01 19:58:47]

Committed patchset #6 (id:140001)

[commit-bot: I haz the power, 2016-02-01 19:59:45]

Description was changed from

==========
Skip PaintPhaseDescendantBlockBackgroundsOnly phase if possible

Set containing self-painting layer's
needsPaintPhaseDescendantBlockBackground() if needed during paint
invalidation, and skip the phase of the layer during painting if
the flag is not set.

Also modified how TableSectionPainter and TableRowPainter handle
PaintPhaseSelfBlockBackgroundOnly to make this CL applicable to them.
Previously they ignored PaintPhaseSelfBlockBackgroundOnly and painted
their own backgrounds in PaintPhaseDescendantBlockBackgroundsOnly behind
cell. This conflicts with how LayoutBox::invalidatePaintIfNeeded() sets
needsPaintPhaseDescendantBlockBackground().

BUG=574938
TEST=PaintLayerPainterTest.PaintPhaseBlockBackground
==========

to

==========
Skip PaintPhaseDescendantBlockBackgroundsOnly phase if possible

Set containing self-painting layer's
needsPaintPhaseDescendantBlockBackground() if needed during paint
invalidation, and skip the phase of the layer during painting if
the flag is not set.

Also modified how TableSectionPainter and TableRowPainter handle
PaintPhaseSelfBlockBackgroundOnly to make this CL applicable to them.
Previously they ignored PaintPhaseSelfBlockBackgroundOnly and painted
their own backgrounds in PaintPhaseDescendantBlockBackgroundsOnly behind
cell. This conflicts with how LayoutBox::invalidatePaintIfNeeded() sets
needsPaintPhaseDescendantBlockBackground().

BUG=574938
TEST=PaintLayerPainterTest.PaintPhaseBlockBackground

Committed: https://crrev.com/99290b961c890e340db97b3d63028b3cf5511d32
Cr-Commit-Position: refs/heads/master@{#372730}
==========

[commit-bot: I haz the power, 2016-02-01 19:59:46]

Patchset 6 (id:??) landed as
https://crrev.com/99290b961c890e340db97b3d63028b3cf5511d32
Cr-Commit-Position: refs/heads/master@{#372730}