Bug 112075 – De-flake fast/dom/title-directionality.html

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[James Robinson]

De-flake fast/dom/title-directionality.html

[James Robinson]

Created attachment 192584 [details]
Patch

[James Robinson]

This patch does de-flake the test, but I don't fully understand how.  Without this patch:

Found 1 test; running 1 (500 times each: --repeat-each=100 --iterations=5), skipping 0.

Running 1 DumpRenderTree.       

[17/500] fast/dom/title-directionality.html failed unexpectedly (text diff)
...
                                        
478 tests ran as expected, 22 didn't:

With this patch:

Found 1 test; running 1 (500 times each: --repeat-each=100 --iterations=5), skipping 0.

Running 1 DumpRenderTree.       

All 500 tests ran as expected. 

I've seen similar tests flake in the same way (differing in a trailing newline) because the HTML parser was at a different point in the main document when the test code ran, usually because of a setTimeout(..., 0) in an inline script.  There's no setTimeout here, but the parser is pumped synchronously on an iframe by doc.open()/write()/close().

[Darin Adler]

Comment on attachment 192584 [details]
Patch

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

> LayoutTests/ChangeLog:9
> +        De-flake fast/dom/title-directionality.html
> +        https://bugs.webkit.org/show_bug.cgi?id=112075
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        * fast/dom/title-directionality-expected.txt:
> +        * fast/dom/title-directionality.html:

Patch would be much better if this said what the fix was, and why it works.

[Tony Chang]

Comment on attachment 192584 [details]
Patch

Seems worth trying.  I guess there could be a race between the email iframe load and the first test case.  Not sure what type of error that would cause.

[Elliott Sprehn]

This smells like lazyAttach + Node::attach logic and the whitespace fixup stuff in there. eseidel had mentioned that code doesn't look right recently.

[James Robinson]

(In reply to comment #3)
> (From update of attachment 192584 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=192584&action=review
> 
> > LayoutTests/ChangeLog:9
> > +        De-flake fast/dom/title-directionality.html
> > +        https://bugs.webkit.org/show_bug.cgi?id=112075
> > +
> > +        Reviewed by NOBODY (OOPS!).
> > +
> > +        * fast/dom/title-directionality-expected.txt:
> > +        * fast/dom/title-directionality.html:
> 
> Patch would be much better if this said what the fix was, and why it works.

Agreed - I don't fully understand why it works yet, so I can't populate a description.  I can put a guess in.

[James Robinson]

Elliot pointed out that the lazy attachment logic works by considering all nodes 'attached' and then setting the style recalc timer.  This style recalc races against the HTML parser hitting the end of the document.  If the HTML parser yields after attaching some nodes before the end of the test (i.e. after running the inline script but before hitting the end of the main document), the collapsing logic in Node::attach() behaves in a different way than if the rest of the document is parsed without any interleaving style recalculations.

Since this particular test isn't intended to exercise the whitespace collapsing path and we have other (non-flaky) tests for this behavior, this just moves the test logic into an onload handler which fires after the document is fully parsed.

I'll land this patch with a description in the ChangeLog based on ^^.

[James Robinson]

Committed r145429: <http://trac.webkit.org/changeset/145429>