Issue repaints for RenderLayers that get new composited backings after

Issue repaints for RenderLayers that get new composited backings after
all compositing layers and their geometry are updated.

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

[Ian Vollick, 2014-06-04 02:45:43]

Ah, I remember this! lgtm.

https://codereview.chromium.org/311813005/diff/1/Source/core/rendering/compos...
File Source/core/rendering/compositing/CompositingLayerAssigner.cpp (right):

https://codereview.chromium.org/311813005/diff/1/Source/core/rendering/compos...
Source/core/rendering/compositing/CompositingLayerAssigner.cpp:244: 
Nit: extra ws.

[Ian Vollick, 2014-06-04 02:55:11]

The CQ bit was checked by vollick@chromium.org

[Ian Vollick, 2014-06-04 02:55:12]

The CQ bit was unchecked by vollick@chromium.org

[Ian Vollick, 2014-06-04 02:55:48]

The CQ bit was checked by vollick@chromium.org

[commit-bot: I haz the power, 2014-06-04 02:56:28]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/chrishtr@chromium.org/311813005/1

[Ian Vollick, 2014-06-04 03:01:07]

The CQ bit was unchecked by vollick@chromium.org

[Ian Vollick, 2014-06-04 03:02:39]

On 2014/06/04 03:01:07, Ian Vollick wrote:
> The CQ bit was unchecked by mailto:vollick@chromium.org

Sorry for the noise -- that was prematurely cq'd. It looks like it may need to
be rebased in any case.

[chrishtr, 2014-06-04 03:54:37]

https://codereview.chromium.org/311813005/diff/1/Source/core/rendering/compos...
File Source/core/rendering/compositing/CompositingLayerAssigner.cpp (right):

https://codereview.chromium.org/311813005/diff/1/Source/core/rendering/compos...
Source/core/rendering/compositing/CompositingLayerAssigner.cpp:244: 
On 2014/06/04 02:45:43, Ian Vollick wrote:
> Nit: extra ws.

Done.

[Ian Vollick, 2014-06-04 04:40:35]

https://codereview.chromium.org/311813005/diff/1/Source/core/rendering/compos...
File Source/core/rendering/compositing/CompositingLayerAssigner.cpp (left):

https://codereview.chromium.org/311813005/diff/1/Source/core/rendering/compos...
Source/core/rendering/compositing/CompositingLayerAssigner.cpp:190: // FIXME:
what's up with parent()?
Hmm. Perhaps the issue causing the layout test crashes is this change (allowing
the repainting of layers with no parent)? It looks like some of the repaint rect
computation code in RenderLayerRepainter might be expecting an ancestor?

https://codereview.chromium.org/311813005/diff/1/Source/core/rendering/compos...
File Source/core/rendering/compositing/CompositingLayerAssigner.h (right):

https://codereview.chromium.org/311813005/diff/1/Source/core/rendering/compos...
Source/core/rendering/compositing/CompositingLayerAssigner.h:87:
Vector<RenderLayer*> m_layersNeedingRepaint;
(Note: we've chatted about removing this offline; it's already gone).

[chrishtr, 2014-06-04 16:35:54]

https://codereview.chromium.org/311813005/diff/1/Source/core/rendering/compos...
File Source/core/rendering/compositing/CompositingLayerAssigner.h (right):

https://codereview.chromium.org/311813005/diff/1/Source/core/rendering/compos...
Source/core/rendering/compositing/CompositingLayerAssigner.h:87:
Vector<RenderLayer*> m_layersNeedingRepaint;
On 2014/06/04 04:40:35, Ian Vollick wrote:
> (Note: we've chatted about removing this offline; it's already gone).

Done.

[chrishtr, 2014-06-04 16:40:06]

The one failing test
(compositing/repaint/fixed-pos-inside-composited-intermediate-layer.html) is due
to there being a
squashed layer that has a position:fixed descendant that
is not composited and hence paints together with the squashed
layer. We then compute its repaint rect, which is at 25, 25
w.r.t. the squashing layer. But the actual repaint that 
occurs later is w.r.t. the root layer, which is 20px left and
up of the squashing layer, so 45, 45.

Looking deeper now.

https://codereview.chromium.org/311813005/diff/1/Source/core/rendering/compos...
File Source/core/rendering/compositing/CompositingLayerAssigner.cpp (left):

https://codereview.chromium.org/311813005/diff/1/Source/core/rendering/compos...
Source/core/rendering/compositing/CompositingLayerAssigner.cpp:190: // FIXME:
what's up with parent()?
On 2014/06/04 04:40:35, Ian Vollick wrote:
> Hmm. Perhaps the issue causing the layout test crashes is this change
(allowing
> the repainting of layers with no parent)? It looks like some of the repaint
rect
> computation code in RenderLayerRepainter might be expecting an ancestor? 

The pad-gesture-fling.html crash was unrelated. See crbug.com/380656

[chrishtr, 2014-06-05 06:02:35]

The CQ bit was checked by chrishtr@chromium.org

[commit-bot: I haz the power, 2014-06-05 06:03:59]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/chrishtr@chromium.org/311813005/60001

[commit-bot: I haz the power, 2014-06-05 06:57:23]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  blink_presubmit on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/6999)
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/1...)
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/1...)
  mac_blink_compile_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_compile_dbg/bu...)
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/10370)
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/10727)

[commit-bot: I haz the power, 2014-06-05 07:00:07]

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

[commit-bot: I haz the power, 2014-06-05 07:00:07]

Try jobs failed on following builders:
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/1...)

[chrishtr, 2014-06-06 02:13:57]

The CQ bit was checked by chrishtr@chromium.org

[commit-bot: I haz the power, 2014-06-06 02:15:07]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/chrishtr@chromium.org/311813005/100001

[commit-bot: I haz the power, 2014-06-06 03:19:36]

Change committed as 175625

[abarth-chromium, 2014-06-06 03:22:21]

https://codereview.chromium.org/311813005/diff/100001/Source/core/rendering/c...
File Source/core/rendering/compositing/CompositingLayerAssigner.h (right):

https://codereview.chromium.org/311813005/diff/100001/Source/core/rendering/c...
Source/core/rendering/compositing/CompositingLayerAssigner.h:43: void
assign(RenderLayer* updateRoot, bool& layersChanged, Vector<RenderLayer*>&
layersNeedingRepaint);
You can also store this sort of variable on the CompositingLayerAssigner
instance.  That way you don't need to pass it around through the entire
recursion.