Bug 30269 – Waiting for iframe load should use onload, not setTimeout

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Julie Parent]

Using setTimeout for iframe load leads to flaky tests, leads to unhappiness!

[Julie Parent]

Created attachment 41058 [details]
Re-writes some editing tests to use iframe load events rather than setTimeout

[Darin Adler]

Comment on attachment 41058 [details]
Re-writes some editing tests to use iframe load events rather than setTimeout

Assuming these tests all still pass, r=me

[Ojan Vafai]

> ===================================================================
> --- LayoutTests/editing/execCommand/paste-1.html	(revision 49408)
> +++ LayoutTests/editing/execCommand/paste-1.html	(working copy)
> @@ -3,7 +3,7 @@ if (window.layoutTestController)
>       layoutTestController.dumpEditingCallbacks();
>  </script>
>  <p>This tests cut/paste inside an editable iframe.  You should see 'foo bar baz' below.</p>
> -<iframe src="../resources/contenteditable-iframe-src.html"></iframe>
> +<iframe src="../resources/contenteditable-iframe-src.html" onload="foo();"></iframe>
>  
>  <script>
>  function foo() {
> @@ -21,5 +21,4 @@ function foo() {
>  }
>  if (window.layoutTestController)
>      window.layoutTestController.waitUntilDone();
> -window.setTimeout(foo, 100);

There's still theoretically a race condition here (and in the other tests), right? If the iframe loads before foo is defined or waitUntilDone is called? It's easy enough to fix by moving the iframe after the script tag right?

[Julie Parent]

Created attachment 41135 [details]
Updated to address Ojan's concern of iframe load happening before script is loaded.

[Ojan Vafai]

Yup. That's what I was asking for. I don't think it was a very likely race condition, but while you're fixing this up, may as well fix it.

[WebKit Commit Bot]

Comment on attachment 41135 [details]
Updated to address Ojan's concern of iframe load happening before script is loaded.

Rejecting patch 41135 from commit-queue.

Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--quiet', '--exit-after-n-failures=1']" exit_code: 1
Running build-dumprendertree
Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests
Testing 11430 test cases.
editing/selection/5136696.html -> failed

Exiting early after 1 failures. 4127 tests run.
66.95s total testing time

4126 test cases (99%) succeeded
1 test case (<1%) had incorrect layout
1 test case (<1%) had stderr output

[Julie Parent]

Created attachment 41143 [details]
Added rebaselines for editing/selection/5136696.  Moving the script tag removed an empty text node from the render tree.

Failed commit queue because of a whitespace change (removed a script tag with newline, now there is no empty text node in the render tree).  Rebaselined the test to remove the empty text node.

[Julie Parent]

Committed: http://trac.webkit.org/changeset/49592