Properly repaint RenderLayer's descendants when it moves

Properly repaint RenderLayer's descendants when it moves

When a RenderLayer moves, it can cause non-composited descendants of itself
that are placed relatively to it to need to be repainted. These descandants
may not have needed layout themselves, nor have they moved as far as the
RenderLayer itself is concerned. This patch forces all descendant RenderLayers
of a moved ancestor to at a minimum be checked by invalidateTreeAfterLayout
to see if they need to be repainted.

Ideally we'd bail in markDescendantLayersMayNeedPaintInvalidation when
we hit a composited layer, but because updateLayerPositions occurs before
compositing update, this would be a chicken and egg problem.

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

[dsinclair, 2014-06-24 19:17:50]

lgtm. Looks like you need some rebaselines though.

[leviw_travelin_and_unemployed, 2014-06-24 20:00:35]

On 2014/06/24 at 19:17:50, dsinclair wrote:
> lgtm. Looks like you need some rebaselines though.

I figured I would, but wanted to validate the results before continuing. Looks
like we're good to go :)

[esprehn, 2014-06-24 20:37:07]

https://codereview.chromium.org/354613003/diff/20001/Source/core/rendering/Re...
File Source/core/rendering/RenderLayer.cpp (right):

https://codereview.chromium.org/354613003/diff/20001/Source/core/rendering/Re...
Source/core/rendering/RenderLayer.cpp:262: for (RenderLayer* childLayer =
firstChild(); childLayer; childLayer = childLayer->nextSibling()) {
We should take a different approach than this and mark a "Forced" update type on
this layer, and then during the walk the Force should make us traverse down the
subtree. What you've added here is going to walk down the subtree over and over
again.

https://codereview.chromium.org/354613003/diff/20001/Source/core/rendering/Re...
Source/core/rendering/RenderLayer.cpp:271:
markDescendantLayersMayNeedPaintInvalidation();
Doesn't this make us traverse down the same subtree repeatedly?

[leviw_travelin_and_unemployed, 2014-06-24 20:44:43]

On 2014/06/24 at 20:37:07, esprehn wrote:
>
https://codereview.chromium.org/354613003/diff/20001/Source/core/rendering/Re...
> File Source/core/rendering/RenderLayer.cpp (right):
> 
>
https://codereview.chromium.org/354613003/diff/20001/Source/core/rendering/Re...
> Source/core/rendering/RenderLayer.cpp:262: for (RenderLayer* childLayer =
firstChild(); childLayer; childLayer = childLayer->nextSibling()) {
> We should take a different approach than this and mark a "Forced" update type
on this layer, and then during the walk the Force should make us traverse down
the subtree. What you've added here is going to walk down the subtree over and
over again.
> 
>
https://codereview.chromium.org/354613003/diff/20001/Source/core/rendering/Re...
> Source/core/rendering/RenderLayer.cpp:271:
markDescendantLayersMayNeedPaintInvalidation();
> Doesn't this make us traverse down the same subtree repeatedly?

Good points. I'm adding it to UpdateLayerPositionsFlags.

[esprehn, 2014-06-24 20:54:40]

On 2014/06/24 at 20:44:43, leviw wrote:
> On 2014/06/24 at 20:37:07, esprehn wrote:
> >
https://codereview.chromium.org/354613003/diff/20001/Source/core/rendering/Re...
> > File Source/core/rendering/RenderLayer.cpp (right):
> > 
> >
https://codereview.chromium.org/354613003/diff/20001/Source/core/rendering/Re...
> > Source/core/rendering/RenderLayer.cpp:262: for (RenderLayer* childLayer =
firstChild(); childLayer; childLayer = childLayer->nextSibling()) {
> > We should take a different approach than this and mark a "Forced" update
type on this layer, and then during the walk the Force should make us traverse
down the subtree. What you've added here is going to walk down the subtree over
and over again.
> > 
> >
https://codereview.chromium.org/354613003/diff/20001/Source/core/rendering/Re...
> > Source/core/rendering/RenderLayer.cpp:271:
markDescendantLayersMayNeedPaintInvalidation();
> > Doesn't this make us traverse down the same subtree repeatedly?
> 
> Good points. I'm adding it to UpdateLayerPositionsFlags.

lgtm

[leviw_travelin_and_unemployed, 2014-06-24 20:59:10]

The CQ bit was checked by leviw@chromium.org

[commit-bot: I haz the power, 2014-06-24 20:59:19]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/leviw@chromium.org/354613003/40001

[commit-bot: I haz the power, 2014-06-24 22:21:23]

Change committed as 176880