When setting needsRepaint, don't stop at non-stacking composited PaintLayers.

When setting needsRepaint, don't stop at non-stacking composited PaintLayers.

Content may paint into an ancestor graphics layer.

BUG=675205
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2611233003
Cr-Commit-Position: refs/heads/master@{#442444}
Committed: https://chromium.googlesource.com/chromium/src/+/ad9aa972891607894db3a60370afd8b19445df0c

[chrishtr, 2017-01-06 21:41:45]

Description was changed from

==========
none

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

to

==========
Invalidate subsequences from containing frames when removing sub-frames.

BUG=675205
==========

[chrishtr, 2017-01-06 22:41:06]

Description was changed from

==========
Invalidate subsequences from containing frames when removing sub-frames.

BUG=675205
==========

to

==========
Invalidate subsequences from containing frames when removing sub-frames.

BUG=675205
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[chrishtr, 2017-01-06 22:47:27]

Description was changed from

==========
Invalidate subsequences from containing frames when removing sub-frames.

BUG=675205
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
When setting needsRepaint, don't stop at non-stacking composited PaintLayers.

Content may paint into an ancestor graphics layer.

BUG=675205
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[chrishtr, 2017-01-07 00:56:11]

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

[chrishtr, 2017-01-07 00:56:57]

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

[commit-bot: I haz the power, 2017-01-07 00:57:23]

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

[Xianzhu, 2017-01-07 01:04:47]

lgtm

https://codereview.chromium.org/2611233003/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/paint/PaintLayer.cpp (right):

https://codereview.chromium.org/2611233003/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/PaintLayer.cpp:922: return
m_layoutObject->containingBlock()->enclosingLayer();
Please add a comment why we needs this, and why we don't need the
compositingContainer to be a stacking context (I think this is because the
floating layer is not stacked, right?)

https://codereview.chromium.org/2611233003/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/paint/PaintLayerTest.cpp (right):

https://codereview.chromium.org/2611233003/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/PaintLayerTest.cpp:505: "<div
style='backface-visibility: hidden'></div>"
Nit: indent line 505-508 by 2 spaces after '"'.

https://codereview.chromium.org/2611233003/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/PaintLayerTest.cpp:509: "</html>");
Nit: s/html/div/

[chrishtr, 2017-01-07 01:11:55]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2017-01-07 01:11:56]

The patchset sent to the CQ was uploaded after l-g-t-m from
wangxianzhu@chromium.org
Link to the patchset: https://codereview.chromium.org/2611233003/#ps180001
(title: "none")

[commit-bot: I haz the power, 2017-01-07 01:12:28]

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

[chrishtr, 2017-01-07 01:13:24]

The CQ bit was unchecked by chrishtr@chromium.org

[chrishtr, 2017-01-07 01:14:49]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2017-01-07 01:14:50]

The patchset sent to the CQ was uploaded after l-g-t-m from
wangxianzhu@chromium.org
Link to the patchset: https://codereview.chromium.org/2611233003/#ps220001
(title: "none")

[chrishtr, 2017-01-07 01:15:00]

Also added a layout test to check crashing.

https://codereview.chromium.org/2611233003/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/paint/PaintLayer.cpp (right):

https://codereview.chromium.org/2611233003/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/PaintLayer.cpp:922: return
m_layoutObject->containingBlock()->enclosingLayer();
On 2017/01/07 at 01:04:46, Xianzhu wrote:
> Please add a comment why we needs this, and why we don't need the
compositingContainer to be a stacking context (I think this is because the
floating layer is not stacked, right?)

Right. Noted.

https://codereview.chromium.org/2611233003/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/paint/PaintLayerTest.cpp (right):

https://codereview.chromium.org/2611233003/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/PaintLayerTest.cpp:505: "<div
style='backface-visibility: hidden'></div>"
On 2017/01/07 at 01:04:46, Xianzhu wrote:
> Nit: indent line 505-508 by 2 spaces after '"'.

Fixed.

https://codereview.chromium.org/2611233003/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/PaintLayerTest.cpp:509: "</html>");
On 2017/01/07 at 01:04:46, Xianzhu wrote:
> Nit: s/html/div/

Fixed.

[commit-bot: I haz the power, 2017-01-07 01:15:13]

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, 2017-01-07 02:14:22]

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

[commit-bot: I haz the power, 2017-01-07 02:14:23]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[chrishtr, 2017-01-09 19:40:31]

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

[commit-bot: I haz the power, 2017-01-09 19:41:25]

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

[chrishtr, 2017-01-09 19:42:24]

Also update GraphicsLayerUpdater to use the same logic. It does reduce
peformance for
floating elements, but probably ok?

[Xianzhu, 2017-01-09 20:04:15]

https://codereview.chromium.org/2611233003/diff/240001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/compositing/GraphicsLayerUpdater.cpp
(right):

https://codereview.chromium.org/2611233003/diff/240001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/compositing/GraphicsLayerUpdater.cpp:60:
return layer.layoutObject()->containingBlock()->enclosingLayer();
Two comments:

1. For performance we can use
   if (layer.layoutObject()->isFloating() &&
!layer.layoutObject()->parent()->isLayoutBlockFlow())
to make the slow line 60 apply to the rare case only.
(This 

2. Should we put the above logic where we set m_compositingAncestor?

https://codereview.chromium.org/2611233003/diff/240001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/paint/PaintLayer.cpp (right):

https://codereview.chromium.org/2611233003/diff/240001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/PaintLayer.cpp:923: if
(m_layoutObject->isFloating())
We can also add "&& !m_layoutObject->parent()->isLayoutBlockFlow() here to
improve performance.

[commit-bot: I haz the power, 2017-01-09 20:56:01]

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

[commit-bot: I haz the power, 2017-01-09 20:56:02]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[chrishtr, 2017-01-09 23:28:33]

https://codereview.chromium.org/2611233003/diff/240001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/compositing/GraphicsLayerUpdater.cpp
(right):

https://codereview.chromium.org/2611233003/diff/240001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/compositing/GraphicsLayerUpdater.cpp:60:
return layer.layoutObject()->containingBlock()->enclosingLayer();
On 2017/01/09 at 20:04:14, Xianzhu wrote:
> Two comments:
> 
> 1. For performance we can use
>    if (layer.layoutObject()->isFloating() &&
!layer.layoutObject()->parent()->isLayoutBlockFlow())
> to make the slow line 60 apply to the rare case only.
> (This

Done.
 
> 
> 2. Should we put the above logic where we set m_compositingAncestor?

That seems tricky, because it differs based on descendants being floats
and this item being not a block.

https://codereview.chromium.org/2611233003/diff/240001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/paint/PaintLayer.cpp (right):

https://codereview.chromium.org/2611233003/diff/240001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/paint/PaintLayer.cpp:923: if
(m_layoutObject->isFloating())
On 2017/01/09 at 20:04:14, Xianzhu wrote:
> We can also add "&& !m_layoutObject->parent()->isLayoutBlockFlow() here to
improve performance.

Done.

[chrishtr, 2017-01-09 23:28:38]

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

[commit-bot: I haz the power, 2017-01-09 23:29:11]

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

[Xianzhu, 2017-01-09 23:37:15]

lgtm

[chrishtr, 2017-01-09 23:37:56]

The CQ bit was unchecked by chrishtr@chromium.org

[chrishtr, 2017-01-09 23:37:58]

The CQ bit was checked by chrishtr@chromium.org

[commit-bot: I haz the power, 2017-01-09 23:38:58]

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, 2017-01-10 01:43:53]

CQ is committing da patch.

Bot data: {"patchset_id": 260001, "attempt_start_ts": 1484005078349850,
"parent_rev": "fe43aec35bceddb3187a83dcab03eedb2a2fbdb7", "commit_rev":
"ad9aa972891607894db3a60370afd8b19445df0c"}

[commit-bot: I haz the power, 2017-01-10 01:44:23]

Description was changed from

==========
When setting needsRepaint, don't stop at non-stacking composited PaintLayers.

Content may paint into an ancestor graphics layer.

BUG=675205
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
When setting needsRepaint, don't stop at non-stacking composited PaintLayers.

Content may paint into an ancestor graphics layer.

BUG=675205
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2611233003
Cr-Commit-Position: refs/heads/master@{#442444}
Committed:
https://chromium.googlesource.com/chromium/src/+/ad9aa972891607894db3a60370af...
==========

[commit-bot: I haz the power, 2017-01-10 01:44:25]

Committed patchset #14 (id:260001) as
https://chromium.googlesource.com/chromium/src/+/ad9aa972891607894db3a60370af...