[iOS] REGRESSION (r174642): DumpRenderTree.app test may dump result twice

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Daniel Bates]

When running layout tests for iOS today I noticed that occasionally many tests in directory LayoutTests/fast/dom would fail after test running test LayoutTests/fast/dom/gc-10.html. From briefly debugging, DumpRenderTree.app is dumping the test result twice for test LayoutTests/fast/dom/gc-10.html because a new load may starts after dump() is called. This makes run-webkit-tests confused (since it expects exactly one test result per test run) and leads to run-webkit-tests using the wrong actual result in the comparison with the expected test result. Specifically, run-webkit-tests compares the actual result of test t_(i - 1) to the expected result of test t_i for i > N once it becomes confused where N := "the index of a test that starts a new load after DumpRenderTree.app calls dump()".

[Daniel Bates]

Created attachment 243368 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 243368 [details]
Patch

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

> Tools/ChangeLog:15
> +        Following <http://trac.webkit.org/changeset/174642>, when we receive a -didFinishLoadForFrame
> +        delegate callback DumpRenderTree.app allows the WebThread to run before dumping the output of
> +        a test. And the WebThread may load a new page during this time, which will ultimately lead to
> +        dumping the test result again. Therefore, we should only dump the test result if neither a new
> +        load began nor testRunner.waitUntilDone() was called after allowing the WebThread to run.

I'm concerned that this adds more unpredictability and flakiness. We first ask to dump results, then refuse to based on some out-of-band state, and without any guarantee that we will ultimately dump the results.

If we already decided that the test is complete, we should not make racy second guesses later.

[Daniel Bates]

Comment on attachment 243368 [details]
Patch

This fix is incorrect.

[Radar WebKit Bug Importer]

<rdar://problem/19281317>

[Daniel Bates]

Created attachment 244736 [details]
Patch

[Daniel Bates]

Created attachment 244737 [details]
Patch

[Daniel Bates]

Comment on attachment 244737 [details]
Patch

Clearing flags on attachment: 244737

Committed r178570: <http://trac.webkit.org/changeset/178570>

[Daniel Bates]

All reviewed patches have been landed.  Closing bug.