Bug 139685 - [iOS] REGRESSION (r174642): DumpRenderTree.app test may dump result twice
Summary: [iOS] REGRESSION (r174642): DumpRenderTree.app test may dump result twice
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: iPhone / iPad iOS 8.1
: P2 Normal
Assignee: Daniel Bates
Keywords: InRadar, Regression
Depends on:
Reported: 2014-12-16 11:07 PST by Daniel Bates
Modified: 2015-01-15 19:29 PST (History)
3 users (show)

See Also:

Patch (1.93 KB, patch)
2014-12-16 11:12 PST, Daniel Bates
no flags Details | Formatted Diff | Diff
Patch (6.39 KB, patch)
2015-01-15 17:26 PST, Daniel Bates
no flags Details | Formatted Diff | Diff
Patch (6.42 KB, patch)
2015-01-15 17:27 PST, Daniel Bates
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2014-12-16 11:07:50 PST
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()".
Comment 1 Daniel Bates 2014-12-16 11:12:06 PST
Created attachment 243368 [details]
Comment 2 Alexey Proskuryakov 2014-12-16 11:30:21 PST
Comment on attachment 243368 [details]

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.
Comment 3 Daniel Bates 2014-12-16 18:58:52 PST
Comment on attachment 243368 [details]

This fix is incorrect.
Comment 4 Radar WebKit Bug Importer 2014-12-17 11:12:45 PST
Comment 5 Daniel Bates 2015-01-15 17:26:37 PST
Created attachment 244736 [details]
Comment 6 Daniel Bates 2015-01-15 17:27:34 PST
Created attachment 244737 [details]
Comment 7 Daniel Bates 2015-01-15 19:29:09 PST
Comment on attachment 244737 [details]

Clearing flags on attachment: 244737

Committed r178570: <http://trac.webkit.org/changeset/178570>
Comment 8 Daniel Bates 2015-01-15 19:29:15 PST
All reviewed patches have been landed.  Closing bug.