Bug 70672 - Make relayout-nested-positioned-elements-crash.html more reliable
Summary: Make relayout-nested-positioned-elements-crash.html more reliable
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Benjamin Poulain
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-21 23:32 PDT by Benjamin Poulain
Modified: 2011-10-22 18:21 PDT (History)
1 user (show)

See Also:


Attachments
Patch (3.70 KB, patch)
2011-10-21 23:38 PDT, Benjamin Poulain
kenneth: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Benjamin Poulain 2011-10-21 23:32:20 PDT
The test relayout-nested-positioned-elements-crash.html is flaky when executing WebCore asynchronously. The reason is the test written in a weird way.

In the follow code, the code of runTest() _cannot_ be executed in order for the test to succeed. In the test result, the PASS is followed by a whitespace. That whitespace is what the TextIterator output for the input element, because it still has a renderer, because runTest() was never executed.

If you execute WebCore asynchronously from the test runner, the function runTest() may or may not be run, causing the test to be flaky.

<p>This tests that we don't cause an assertion failure on relayout of nested positioned elements. This test PASSED if we don't cause an assertion failure.</p>
<div style="position:absolute">
    <span style="position:relative">
        PASS
        <div style="position:absolute">
            <input id="hideMe"/>
        </div>
    </span>
</div>
<script>
window.setTimeout(runTest, 0); // For some reason we need the setTimeout() for this test to work.
function runTest()
{
    document.getElementById("hideMe").style.display = "none";
}

Assuming the test is correct, we should rewrite it in a way that is more reliable.
Comment 1 Benjamin Poulain 2011-10-21 23:38:23 PDT
Created attachment 112079 [details]
Patch
Comment 2 Kenneth Rohde Christiansen 2011-10-22 06:25:10 PDT
Comment on attachment 112079 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=112079&action=review

I see this as an improvement, but I have a few comments.

> LayoutTests/ChangeLog:9
> +        Change the test to use the Javascript test framework.
> +        Ensure the element has the correct style in the final state.

Any comment on why or when it was unreliable?

> LayoutTests/fast/block/positioning/relayout-nested-positioned-elements-crash.html:17
> +description("This tests that we don't cause an assertion failure on relayout of nested positioned elements. This test pass if we don't cause an assertion failure.");

and the element has the correct style in the final state. ?
Comment 3 Kenneth Rohde Christiansen 2011-10-22 06:28:21 PDT
The description is comment 1 is pretty good and could be summarized in the ChangeLog.
Comment 4 Benjamin Poulain 2011-10-22 17:48:27 PDT
Committed r98197: <http://trac.webkit.org/changeset/98197>