Bug 139685

Summary:

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

Product:

WebKit

Reporter:

Daniel Bates <dbates>

Component:

Tools / Tests

Assignee:

Daniel Bates <dbates>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ap, ddkilzer, simon.fraser

Priority:

Keywords:

InRadar, Regression

Version:

528+ (Nightly build)

Hardware:

iPhone / iPad

OS:

iOS 8.1

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none

Daniel Bates

Reported 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()".

Attachments
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
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Daniel Bates

Comment 1 2014-12-16 11:12:06 PST

Created attachment 243368 [details] Patch

Alexey Proskuryakov

Comment 2 2014-12-16 11:30:21 PST

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 3 2014-12-16 18:58:52 PST

Comment on attachment 243368 [details] Patch This fix is incorrect.

Radar WebKit Bug Importer

Comment 4 2014-12-17 11:12:45 PST

<rdar://problem/19281317>

Daniel Bates

Comment 5 2015-01-15 17:26:37 PST

Created attachment 244736 [details] Patch

Daniel Bates

Comment 6 2015-01-15 17:27:34 PST

Created attachment 244737 [details] Patch