Change output method of paint invalidation failure message

Change output method of paint invalidation failure message

Previously we remove all contents of the test html and insert message.
This makes the actual result invisible.

Put a mesage box at the top-right corner to indicate paint invalidation
objects mismatch. pdr@ contributed the idea and also he svg support
code.

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

[Xianzhu, 2015-09-16 20:41:06]

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

[pdr., 2015-09-16 21:27:02]

LGTM

https://codereview.chromium.org/1340423004/diff/1/LayoutTests/paint/invalidat...
File LayoutTests/paint/invalidation/spv2/resources/paint-invalidation-test.js
(right):

https://codereview.chromium.org/1340423004/diff/1/LayoutTests/paint/invalidat...
LayoutTests/paint/invalidation/spv2/resources/paint-invalidation-test.js:49: //
TODO(wangxianzhu): The following works for HTML only. Should support SVG.
Lets support SVG:

function finishPaintInvalidationTest()
{
    if (!window.testRunner || !window.internals)
        return;

    testRunner.layoutAndPaintAsyncThen(function()
    {
        var isSvg = document.rootElement && document.rootElement.nodeName ==
"svg";
        var paintInvalidationObjects =
JSON.stringify(internals.trackedPaintInvalidationObjects());
        var expectedPaintInvalidationObjects =
JSON.stringify(window.expectedPaintInvalidationObjects);
        internals.stopTrackingPaintInvalidationObjects();

        if (paintInvalidationObjects != expectedPaintInvalidationObjects) {
            if (isSvg) {
                var svgns = "http://www.w3.org/2000/svg"
                var rect = document.createElementNS(svgns, 'rect');
                rect.setAttribute('width', '30%');
                rect.setAttribute('height', '10%');
                rect.setAttribute('x', '70%');
                rect.setAttribute('fill', 'white');
                rect.setAttribute('stroke', 'black');
                rect.setAttribute('stroke-width', '1px');
                document.documentElement.appendChild(rect);
                var text = document.createElementNS(svgns, 'text');
                text.setAttribute('x', '100%');
                text.setAttribute('y', '5%');
                text.setAttribute('text-anchor', 'end');
                text.textContent = "Paint invalidation objects mismatch.";
                document.documentElement.appendChild(text);
            } else {
                var message = 'Paint invalidation objects mismatch.'
                    + '\nExpected:\n' +
expectedPaintInvalidationObjects.replace(/","/g, '",\n "')
                    + '\nActual:\n' + paintInvalidationObjects.replace(/","/g,
'",\n "');
                testRunner.logToStderr(message);

                var div = document.createElement('div');
                div.style.zIndex = '2147483647';
                div.style.visibility = 'visible';
                div.style.position = 'fixed';
                div.style.top = '0';
                div.style.right = '0';
                div.style.width = '300px';
                div.style.height = '36px';
                div.style.color = 'red';
                div.style.fontSize = '14px';
                div.style.lineHeight = '16px';
                div.style.fontFamily = 'Arial';
                div.style.border = '1px solid red';
                div.style.background = 'white';
                div.appendChild(document.createTextNode('Paint invalidation
objects mismatch.'));
                div.appendChild(document.createElement('br'));
                div.appendChild(document.createTextNode('See stderr for
details.'));
                document.body.appendChild(div);
            }
        }
        testRunner.notifyDone();
    });

https://codereview.chromium.org/1340423004/diff/1/LayoutTests/paint/invalidat...
LayoutTests/paint/invalidation/spv2/resources/paint-invalidation-test.js:68:
div.style.background = 'none';
Lets set the backgroundColor to white so the text is readable above any text.

[pdr., 2015-09-16 21:31:13]

On 2015/09/16 at 21:27:02, pdr wrote:
Note that I removed the force layout in the above code. I think that'll be safe,
but we should check (svg doesn't have document.body).

Just for your information, a paint invalidation test for svg looks like:
<?xml version="1.0" standalone="no"?>
<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN"
"http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
<svg width="100%" height="100%" version="1.1" xmlns="http://www.w3.org/2000/svg"
xmlns:xlink="http://www.w3.org/1999/xlink">
<script xlink:href="resources/paint-invalidation-test.js"/>
<script>
window.expectedPaintInvalidationObjects = [
];
function paintInvalidationTest() {
  document.getElementById('rect').style.fill = "green";
}
onload = runPaintInvalidationTest;
</script>
<rect id="rect" width="100px" height="100px" fill="blue"/>
</svg>

[Xianzhu, 2015-09-17 03:16:30]

The CQ bit was checked by wangxianzhu@chromium.org

[Xianzhu, 2015-09-17 03:16:30]

The patchset sent to the CQ was uploaded after l-g-t-m from pdr@chromium.org
Link to the patchset: https://codereview.chromium.org/1340423004/#ps20001
(title: "Add pdr's svg code")

[commit-bot: I haz the power, 2015-09-17 03:16:34]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1340423004/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1340423004/20001

[Xianzhu, 2015-09-17 03:17:03]

The CQ bit was unchecked by wangxianzhu@chromium.org

[Xianzhu, 2015-09-17 03:17:57]

The CQ bit was checked by wangxianzhu@chromium.org

[Xianzhu, 2015-09-17 03:17:57]

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

[commit-bot: I haz the power, 2015-09-17 03:18:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1340423004/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1340423004/40001

[commit-bot: I haz the power, 2015-09-17 04:04:59]

Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=202425

[Xianzhu, 2015-09-17 17:00:39]

With this patch, the experience of checking failure results is better, but still
not good enough because it's still time-consuming:

- I must carefully examine the each pixel result to see if there is real pixel
mismatch, to distinguish paint failure or paint invalidation mismatch;

- For each paint invalidation mismatch, I must look at the stderr output, and if
it should be rebaselined, I must manually copy the actual output into the test.
Neither auto-rebaseline nor --reset-results works for these tests.

I thought of two ways to save the time for this:
1. Allow text expectations for ref tests
(https://codereview.chromium.org/1346673003/);
2. Convert this tests to pixel tests.

I would like to do 2 if we could not agree upon 1.

Wdyt?

[Dirk Pranke, 2015-09-17 18:20:07]

On 2015/09/17 17:00:39, Xianzhu wrote:
> With this patch, the experience of checking failure results is better, but
still
> not good enough because it's still time-consuming:
> 
> - I must carefully examine the each pixel result to see if there is real pixel
> mismatch, to distinguish paint failure or paint invalidation mismatch;
> 
> - For each paint invalidation mismatch, I must look at the stderr output, and
if
> it should be rebaselined, I must manually copy the actual output into the
test.
> Neither auto-rebaseline nor --reset-results works for these tests.
> 
> I thought of two ways to save the time for this:
> 1. Allow text expectations for ref tests
> (https://codereview.chromium.org/1346673003/);
> 2. Convert this tests to pixel tests.
> 
> I would like to do 2 if we could not agree upon 1.
> 
> Wdyt?

I mostly defer to pdr@ here. I don't like the idea of having to create more
pixel tesets, but I like the idea of having tests that are reftests that *also*
have -expected.txt output even less. I'm not really familiar with how the
invalidation tests need to work, so I don't really know if there might be 
better options, sorry.

[ojan, 2015-09-19 03:15:56]

ojan@chromium.org changed reviewers:
+ ojan@chromium.org

[ojan, 2015-09-19 03:15:57]

https://codereview.chromium.org/1340423004/diff/40001/LayoutTests/paint/inval...
File LayoutTests/paint/invalidation/spv2/resources/paint-invalidation-test.js
(left):

https://codereview.chromium.org/1340423004/diff/40001/LayoutTests/paint/inval...
LayoutTests/paint/invalidation/spv2/resources/paint-invalidation-test.js:43:
document.body.offsetTop;
Doesn't removing this make your tests flaky because you'll get different tracked
objects depending on whether a layout happened to happen or not?

Can you just do document.documentElement.offsetTop? Does that work in SVG?