Allow paint invalidation containers to cross frame boundaries. (re-land #2)

Allow paint invalidation containers to cross frame boundaries. (re-land #2)

The previous commit was rolled out because it did not make the required changes
to PaintInvalidationState in order to teach it how to cross frame boundaries.

Previously, the *actual* paint invalidation container, meaning the enclosing
compositing layer / root RenderView, could already have been across a frame boundary.
The logic to do this correctly was done via special code in RenderView.

Instead, generalize the existing mechanisms to find a paint invalidation container
and map rects to repaint container coordinate space to cross frame boundaries.
This simplifies the code, and also causes paint invalidation rects to always be stored
in the coordinate space of their graphics layer backing. The latter is important if we want
to use these rects for determining which parts of a graphics layer need to be painted.

BUG=

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

[chrishtr, 2014-08-19 17:42:10]

https://codereview.chromium.org/482063005/diff/20001/LayoutTests/platform/lin...
File
LayoutTests/platform/linux/fast/repaint/line-flow-with-floats-2-expected.txt
(left):

https://codereview.chromium.org/482063005/diff/20001/LayoutTests/platform/lin...
LayoutTests/platform/linux/fast/repaint/line-flow-with-floats-2-expected.txt:18:
[8, 74, 418, 526],
Don't know why precisely, but this one gets rid of a redundant rect. This test
uses iframes.

https://codereview.chromium.org/482063005/diff/60001/LayoutTests/fast/repaint...
File LayoutTests/fast/repaint/fixed-table-overflow-expected.txt (right):

https://codereview.chromium.org/482063005/diff/60001/LayoutTests/fast/repaint...
LayoutTests/fast/repaint/fixed-table-overflow-expected.txt:12: [100, 100, 100,
100],
Now it's offset correctly in PaintInvalidationState.

https://codereview.chromium.org/482063005/diff/60001/LayoutTests/fast/repaint...
File
LayoutTests/fast/repaint/fixed-to-relative-position-with-absolute-child-expected.txt
(right):

https://codereview.chromium.org/482063005/diff/60001/LayoutTests/fast/repaint...
LayoutTests/fast/repaint/fixed-to-relative-position-with-absolute-child-expected.txt:9:
[116, 500, 100, 100],
Now it's offset correctly in PaintInvalidationState.

https://codereview.chromium.org/482063005/diff/60001/LayoutTests/fast/repaint...
File
LayoutTests/fast/repaint/scroll-fixed-layer-with-no-visible-content-expected.txt
(right):

https://codereview.chromium.org/482063005/diff/60001/LayoutTests/fast/repaint...
LayoutTests/fast/repaint/scroll-fixed-layer-with-no-visible-content-expected.txt:9:
[200, 1300, 100, 100],
Now it's offset correctly in PaintInvalidationState.

https://codereview.chromium.org/482063005/diff/60001/Source/core/rendering/Pa...
File Source/core/rendering/PaintInvalidationState.cpp (left):

https://codereview.chromium.org/482063005/diff/60001/Source/core/rendering/Pa...
Source/core/rendering/PaintInvalidationState.cpp:66: // FIXME: This doesn't work
correctly with transforms.
localToContainerPoint is supposed to work with transforms, so I think it's good
now?

https://codereview.chromium.org/482063005/diff/60001/Source/core/rendering/Pa...
File Source/core/rendering/PaintInvalidationState.cpp (right):

https://codereview.chromium.org/482063005/diff/60001/Source/core/rendering/Pa...
Source/core/rendering/PaintInvalidationState.cpp:59: FloatPoint fixedOffset =
m_renderer.localToContainerPoint(FloatPoint(), &m_paintInvalidationContainer,
TraverseDocumentBoundaries);
Map to the space of the invalidation container rather than the view, since it
paints into the former.
Also change m_renderer.view()-> ... to m_renderer, don't know why it was correct
before, maybe it wasn't?

[leviw_travelin_and_unemployed, 2014-08-19 17:52:12]

https://codereview.chromium.org/482063005/diff/60001/Source/core/rendering/Pa...
File Source/core/rendering/PaintInvalidationState.cpp (right):

https://codereview.chromium.org/482063005/diff/60001/Source/core/rendering/Pa...
Source/core/rendering/PaintInvalidationState.cpp:59: FloatPoint fixedOffset =
m_renderer.localToContainerPoint(FloatPoint(), &m_paintInvalidationContainer,
TraverseDocumentBoundaries);
On 2014/08/19 17:42:10, chrishtr wrote:
> Map to the space of the invalidation container rather than the view, since it
> paints into the former.
> Also change m_renderer.view()-> ... to m_renderer, don't know why it was
correct
> before, maybe it wasn't?

We always stopped at the View boundary before. Can we please add a test case for
repainting a fixed position renderer inside an iframe?

[chrishtr, 2014-08-19 18:19:23]

https://codereview.chromium.org/482063005/diff/60001/Source/core/rendering/Pa...
File Source/core/rendering/PaintInvalidationState.cpp (right):

https://codereview.chromium.org/482063005/diff/60001/Source/core/rendering/Pa...
Source/core/rendering/PaintInvalidationState.cpp:59: FloatPoint fixedOffset =
m_renderer.localToContainerPoint(FloatPoint(), &m_paintInvalidationContainer,
TraverseDocumentBoundaries);
On 2014/08/19 17:52:12, leviw wrote:
> On 2014/08/19 17:42:10, chrishtr wrote:
> > Map to the space of the invalidation container rather than the view, since
it
> > paints into the former.
> > Also change m_renderer.view()-> ... to m_renderer, don't know why it was
> correct
> > before, maybe it wasn't?
> 
> We always stopped at the View boundary before. Can we please add a test case
for
> repainting a fixed position renderer inside an iframe?

Done.

https://codereview.chromium.org/482063005/diff/60001/Source/core/rendering/Pa...
Source/core/rendering/PaintInvalidationState.cpp:59: FloatPoint fixedOffset =
m_renderer.localToContainerPoint(FloatPoint(), &m_paintInvalidationContainer,
TraverseDocumentBoundaries);
On 2014/08/19 17:52:12, leviw wrote:
> On 2014/08/19 17:42:10, chrishtr wrote:
> > Map to the space of the invalidation container rather than the view, since
it
> > paints into the former.
> > Also change m_renderer.view()-> ... to m_renderer, don't know why it was
> correct
> > before, maybe it wasn't?
> 
> We always stopped at the View boundary before. Can we please add a test case
for
> repainting a fixed position renderer inside an iframe?

Done. Also updated repaint-in-iframe.html to set a width on the div.

[leviw_travelin_and_unemployed, 2014-08-19 18:47:06]

https://codereview.chromium.org/482063005/diff/100001/LayoutTests/fast/overfl...
File LayoutTests/fast/overflow/abs-rel-pos-invalidation-ancestor-expected.txt
(right):

https://codereview.chromium.org/482063005/diff/100001/LayoutTests/fast/overfl...
LayoutTests/fast/overflow/abs-rel-pos-invalidation-ancestor-expected.txt:10:
[16, 32, 100, 100]
Is this change expected?

https://codereview.chromium.org/482063005/diff/100001/Source/core/rendering/R...
File Source/core/rendering/RenderObject.cpp (right):

https://codereview.chromium.org/482063005/diff/100001/Source/core/rendering/R...
Source/core/rendering/RenderObject.cpp:1475: renderView =
renderView->frame()->ownerRenderer()->view();
To be clear, this means we're going to walk up all the way to the root document
view and call that our invalidation container. In this case we'll never skip
over other composited layers from containing views, right? This seems a bit
weird.

[leviw_travelin_and_unemployed, 2014-08-19 18:49:16]

This is how I feel about this patch: http://i.imgur.com/ULFengu.gif

[chrishtr, 2014-08-19 21:18:03]

https://codereview.chromium.org/482063005/diff/100001/LayoutTests/fast/overfl...
File LayoutTests/fast/overflow/abs-rel-pos-invalidation-ancestor-expected.txt
(right):

https://codereview.chromium.org/482063005/diff/100001/LayoutTests/fast/overfl...
LayoutTests/fast/overflow/abs-rel-pos-invalidation-ancestor-expected.txt:10:
[16, 32, 100, 100]
On 2014/08/19 at 18:47:06, leviw wrote:
> Is this change expected?

Fixed.

https://codereview.chromium.org/482063005/diff/100001/Source/core/rendering/R...
File Source/core/rendering/RenderObject.cpp (right):

https://codereview.chromium.org/482063005/diff/100001/Source/core/rendering/R...
Source/core/rendering/RenderObject.cpp:1475: renderView =
renderView->frame()->ownerRenderer()->view();
On 2014/08/19 at 18:47:06, leviw wrote:
> To be clear, this means we're going to walk up all the way to the root
document view and call that our invalidation container. In this case we'll never
skip over other composited layers from containing views, right? This seems a bit
weird.

This code only executes in the situation when there is no composited layer along
the way. Otherwise enclosingLayerForPaintInvalidationCrossingFrameBoundaries()
would have returned it.

https://codereview.chromium.org/482063005/diff/120001/Source/core/rendering/P...
File Source/core/rendering/PaintInvalidationState.cpp (left):

https://codereview.chromium.org/482063005/diff/120001/Source/core/rendering/P...
Source/core/rendering/PaintInvalidationState.cpp:68: m_paintOffset =
LayoutSize(fixedOffset.x(), fixedOffset.y()) + offset;
Adding offset here was wrong, and resulted in the doubled offsets.

[chrishtr, 2014-08-20 17:44:56]

Now passing all tests, and with no diffs to layout tests except the ones I
changed.

[leviw_travelin_and_unemployed, 2014-08-20 18:30:03]

LGTM.

https://codereview.chromium.org/482063005/diff/160001/Source/core/rendering/R...
File Source/core/rendering/RenderView.cpp (right):

https://codereview.chromium.org/482063005/diff/160001/Source/core/rendering/R...
Source/core/rendering/RenderView.cpp:435: // We specifically need to repaint the
viewRect since other renderers
Why are you changing the language back to repaint? Dan worked hard to change
that all over :p

[chrishtr, 2014-08-20 18:40:10]

The CQ bit was checked by chrishtr@chromium.org

[commit-bot: I haz the power, 2014-08-20 18:40:38]

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

[commit-bot: I haz the power, 2014-08-20 19:57:05]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/2...)
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/2...)
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/23713)
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)

[commit-bot: I haz the power, 2014-08-20 19:59:45]

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

[commit-bot: I haz the power, 2014-08-20 19:59:47]

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

[chrishtr, 2014-08-20 20:26:14]

The CQ bit was checked by chrishtr@chromium.org

[commit-bot: I haz the power, 2014-08-20 20:27:07]

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

[chrishtr, 2014-08-20 20:36:52]

The CQ bit was unchecked by chrishtr@chromium.org

[chrishtr, 2014-08-20 21:39:36]

Made a fix to handle the case when the iframe is inside of an invalidation
container which is
not a render view. Added a test for that also.

https://codereview.chromium.org/482063005/diff/160001/Source/core/rendering/R...
File Source/core/rendering/RenderView.cpp (right):

https://codereview.chromium.org/482063005/diff/160001/Source/core/rendering/R...
Source/core/rendering/RenderView.cpp:435: // We specifically need to repaint the
viewRect since other renderers
On 2014/08/20 18:30:02, leviw wrote:
> Why are you changing the language back to repaint? Dan worked hard to change
> that all over :p

Done.

[chrishtr, 2014-08-20 21:39:40]

The CQ bit was checked by chrishtr@chromium.org

[commit-bot: I haz the power, 2014-08-20 21:43:17]

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

[commit-bot: I haz the power, 2014-08-20 22:24:40]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/2...)
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/2...)
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/20617)
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)

[commit-bot: I haz the power, 2014-08-20 22:27:08]

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

[commit-bot: I haz the power, 2014-08-20 22:27:09]

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

[chrishtr, 2014-08-21 01:00:50]

The CQ bit was checked by chrishtr@chromium.org

[commit-bot: I haz the power, 2014-08-21 01:01:38]

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

[commit-bot: I haz the power, 2014-08-21 01:05:06]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/2...)
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/2...)
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)

[commit-bot: I haz the power, 2014-08-21 01:07:14]

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

[commit-bot: I haz the power, 2014-08-21 01:07:15]

Try jobs failed on following builders:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)

[chrishtr, 2014-08-21 17:09:29]

The CQ bit was checked by chrishtr@chromium.org

[commit-bot: I haz the power, 2014-08-21 17:10:30]

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

[commit-bot: I haz the power, 2014-08-21 21:27:59]

Committed patchset #13 (240001) as 180736