Use the correct method for computing a repaint rect that takes into account squashing.

Use the correct method for computing a repaint rect that takes into account squashing.

This fixes a mistake in crrev.com/301843002

Includes a test.

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

[chrishtr, 2014-05-30 16:28:05]

https://codereview.chromium.org/306093002/diff/40001/LayoutTests/compositing/...
File
LayoutTests/compositing/repaint/fixed-pos-inside-composited-intermediate-layer-expected.txt
(right):

https://codereview.chromium.org/306093002/diff/40001/LayoutTests/compositing/...
LayoutTests/compositing/repaint/fixed-pos-inside-composited-intermediate-layer-expected.txt:9:
(rect 45.00 45.00 50.00 150.00)
It actually is 45px from the squashed container. Don't know where the 25px
originally came from, but it is wrong.

https://codereview.chromium.org/306093002/diff/40001/LayoutTests/compositing/...
File LayoutTests/compositing/squashing/add-remove-squashed-layers-expected.txt
(left):

https://codereview.chromium.org/306093002/diff/40001/LayoutTests/compositing/...
LayoutTests/compositing/squashing/add-remove-squashed-layers-expected.txt:95:
(rect 0.00 0.00 100.00 100.00)
Not sure why this went away exactly, but (80 80 100 100) is the only required
repaint in this case.

https://codereview.chromium.org/306093002/diff/40001/LayoutTests/compositing/...
File LayoutTests/compositing/squashing/add-remove-squashed-layers-expected.txt
(right):

https://codereview.chromium.org/306093002/diff/40001/LayoutTests/compositing/...
LayoutTests/compositing/squashing/add-remove-squashed-layers-expected.txt:202:
(rect 0.00 0.00 20.00 20.00)
This is the amount that overlap1 leaked into the new bounds of
the squashing layer.

https://codereview.chromium.org/306093002/diff/40001/LayoutTests/compositing/...
File
LayoutTests/compositing/squashing/invalidations-with-large-negative-margin-expected.txt
(right):

https://codereview.chromium.org/306093002/diff/40001/LayoutTests/compositing/...
LayoutTests/compositing/squashing/invalidations-with-large-negative-margin-expected.txt:12:
(rect 8.00 8.00 50.00 50.00)
The second and fourth repaints
are the important ones. The others
are parts of the scrollbar.

[chrishtr, 2014-05-30 16:58:14]

https://codereview.chromium.org/306093002/diff/60001/LayoutTests/compositing/...
File LayoutTests/compositing/squashing/tricky-element-removal-crash-expected.txt
(right):

https://codereview.chromium.org/306093002/diff/60001/LayoutTests/compositing/...
LayoutTests/compositing/squashing/tricky-element-removal-crash-expected.txt:23:
(rect 8.00 8.00 92.00 92.00)
This one is I believe a chicken-egg issue  with layout and compositing ordering.
The actual repaint rect is (8 8 108 108), which is cut off here to the bounds of
the graphics layer. These bouds are recorded in the first layout pass of the
page, in which there are no graphics layers assigned yet, and hence the painting
backing of this div is the document.

I tested this hypothesis by modifying the test to teak layout in a no-op way
before removing the element. I verified that this causes the correct repaint
rect of (0 0 100 100).

[leviw_travelin_and_unemployed, 2014-05-30 17:32:34]

lgtm

Okay.

[commit-bot: I haz the power, 2014-05-30 17:33:36]

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

[chrishtr, 2014-05-30 18:19:02]

The CQ bit was checked by chrishtr@chromium.org

[commit-bot: I haz the power, 2014-05-30 18:19:59]

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

[commit-bot: I haz the power, 2014-05-30 20:08:40]

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/6542)
  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...)
  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-05-30 20:34:56]

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

[commit-bot: I haz the power, 2014-05-30 20:34:57]

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

[chrishtr, 2014-05-30 22:35:51]

The CQ bit was checked by chrishtr@chromium.org

[commit-bot: I haz the power, 2014-05-30 22:38:09]

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

[chrishtr, 2014-05-30 23:29:36]

The CQ bit was unchecked by chrishtr@chromium.org

[chrishtr, 2014-05-30 23:32:10]

The CQ bit was checked by chrishtr@chromium.org

[commit-bot: I haz the power, 2014-05-30 23:35:23]

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

[commit-bot: I haz the power, 2014-05-31 01:10:23]

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

[commit-bot: I haz the power, 2014-05-31 02:21:04]

Change committed as 175189

[ojan, 2014-06-02 05:29:06]

A bunch of nits about the test. Except for the setTimeout thing, that really
should be fixed.

https://codereview.chromium.org/306093002/diff/100002/LayoutTests/compositing...
File
LayoutTests/compositing/squashing/invalidations-with-large-negative-margin.html
(right):

https://codereview.chromium.org/306093002/diff/100002/LayoutTests/compositing...
LayoutTests/compositing/squashing/invalidations-with-large-negative-margin.html:10:
<div id="testResults" style="display:none">
Why have the extra outer div? Why not just put the display:none on the pre
itself?

https://codereview.chromium.org/306093002/diff/100002/LayoutTests/compositing...
LayoutTests/compositing/squashing/invalidations-with-large-negative-margin.html:15:
// the move.
This comment doesn't really say anything that isn't obvious in the test. The
sorts of comments that are useful are the ones that say what the "this
situation" is that's being tested. It's clear from the test itself that it's
verifying repaint invalidations.

https://codereview.chromium.org/306093002/diff/100002/LayoutTests/compositing...
LayoutTests/compositing/squashing/invalidations-with-large-negative-margin.html:21:
setTimeout(function() {
Why the setTimeout? setTimeouts lead to slow/flaky tests.

https://codereview.chromium.org/306093002/diff/100002/LayoutTests/compositing...
LayoutTests/compositing/squashing/invalidations-with-large-negative-margin.html:22:
if (window.internals)
Having this if-check, but not checking it in the other places you check
window.internals seems kind of random.

[chrishtr, 2014-06-02 16:22:32]

Will send you a patch with these adjustments.

https://codereview.chromium.org/306093002/diff/100002/LayoutTests/compositing...
File
LayoutTests/compositing/squashing/invalidations-with-large-negative-margin.html
(right):

https://codereview.chromium.org/306093002/diff/100002/LayoutTests/compositing...
LayoutTests/compositing/squashing/invalidations-with-large-negative-margin.html:10:
<div id="testResults" style="display:none">
On 2014/06/02 05:29:06, ojan wrote:
> Why have the extra outer div? Why not just put the display:none on the pre
> itself?

Done.

https://codereview.chromium.org/306093002/diff/100002/LayoutTests/compositing...
LayoutTests/compositing/squashing/invalidations-with-large-negative-margin.html:15:
// the move.
On 2014/06/02 05:29:06, ojan wrote:
> This comment doesn't really say anything that isn't obvious in the test. The
> sorts of comments that are useful are the ones that say what the "this
> situation" is that's being tested. It's clear from the test itself that it's
> verifying repaint invalidations.

Done.

https://codereview.chromium.org/306093002/diff/100002/LayoutTests/compositing...
LayoutTests/compositing/squashing/invalidations-with-large-negative-margin.html:21:
setTimeout(function() {
On 2014/06/02 05:29:06, ojan wrote:
> Why the setTimeout? setTimeouts lead to slow/flaky tests.

Done.

https://codereview.chromium.org/306093002/diff/100002/LayoutTests/compositing...
LayoutTests/compositing/squashing/invalidations-with-large-negative-margin.html:22:
if (window.internals)
On 2014/06/02 05:29:06, ojan wrote:
> Having this if-check, but not checking it in the other places you check
> window.internals seems kind of random.

Done.