Bug 30598

Summary:

LayoutTests/editing/selection/editable-html-element.html is flaky on Chrome bots

Product:

WebKit

Reporter:

Evan Martin <evan>

Component:

HTML Editing

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

eric, jparent, mjs, ojan

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
guess at a fix for the test	none
results when the test fails	none

Evan Martin

Reported 2009-10-20 13:44:06 PDT

This test has had only mixed success on the Chromium builders. http://src.chromium.org/viewvc/chrome/trunk/src/webkit/tools/layout_tests/flakiness_dashboard.html#tests=LayoutTests%2Fediting%2Fselection%2Feditable-html-element.html The symptom when it fails is that runTest() is not defined. I haven't been able to reproduce the failure locally, but the JS experts suggest that perhaps these floating <script> tags are creating the body element too early.

Attachments
guess at a fix for the test (1.42 KB, patch) 2009-10-20 13:46 PDT, Evan Martin	no flags	Details Formatted Diff Diff
results when the test fails (2.78 KB, text/plain) 2009-10-21 09:48 PDT, Eric Seidel (no email)	no flags	Details
View All Add attachment proposed patch, testcase, etc.

Evan Martin

Comment 1 2009-10-20 13:46:20 PDT

Created attachment 41521 [details] guess at a fix for the test

Evan Martin

Comment 2 2009-10-20 16:34:10 PDT

The test in question, for those following at home http://trac.webkit.org/browser/trunk/LayoutTests/editing/selection/editable-html-element.html

Maciej Stachowiak

Comment 3 2009-10-20 16:42:44 PDT

I think it's almost impossible for this patch to affect the outcome of the test. <script> inside <html> will create an implicit <head>, and <body> will implicitly close the <head>.

Evan Martin

Comment 4 2009-10-20 16:42:58 PDT

Comment on attachment 41521 [details] guess at a fix for the test maciej looked at this on IRC and suggested some other ideas. removing review bits.

Evan Martin

Comment 5 2009-10-20 16:47:27 PDT

Chrome-side bug: http://code.google.com/p/chromium/issues/detail?id=25355

Eric Seidel (no email)

Comment 6 2009-10-21 09:48:36 PDT

Created attachment 41571 [details] results when the test fails

Eric Seidel (no email)

Comment 7 2009-10-21 09:49:04 PDT

I got those results by going to: http://src.chromium.org/viewvc/chrome/trunk/src/webkit/tools/layout_tests/flakiness_dashboard.html#tests=LayoutTests%2Fediting%2Fselection%2Feditable-html-element.html and clicking on one of the red bars and downloading the layout-test-results.zip

Eric Seidel (no email)

Comment 8 2009-10-21 09:56:41 PDT

It looks like the only difference in results is the extra "runTest() is not defined". See the mac results: http://trac.webkit.org/browser/trunk/LayoutTests/platform/mac/editing/selection/editable-html-element-expected.txt?format=txt I wonder if this could be some sort of v8 bug where it's logging an non-sensical error.

Eric Seidel (no email)

Comment 9 2009-10-21 09:57:08 PDT

Could the runTest() not defined log be coming from a previous test?

Evan Martin

Comment 10 2009-10-21 10:00:48 PDT

Wow, I fail. Eric is implicitly noting that runTest is clearly still running because the rest of the test output is still ok. There's just the extra line.

Eric Seidel (no email)

Comment 11 2009-10-21 10:12:17 PDT

I'm not sure if the output is "still OK", but it does look identical. I did a quick visual-inspection diff, I did not run "diff" against the two output files to confirm that they're exactly the same besides that line.

Julie Parent

Comment 12 2009-10-21 10:32:50 PDT

Another data point: LayoutTests/editing/selection/drag-to-contenteditable-iframe.html is doing the same thing. It fails with the extra runTest is not defined message, but clearly it is running properly since the rest of the output is ok.

Evan Martin

Comment 13 2009-10-21 11:11:52 PDT

I had diffed the fail/pass tests before, and that (In reply to comment #11) > I'm not sure if the output is "still OK", but it does look identical. I did a > quick visual-inspection diff, I did not run "diff" against the two output files > to confirm that they're exactly the same besides that line. Yes, it's identical (I had run diff before when trying to figure out what was going wrong) other than the extra line.

Ojan Vafai

Comment 14 2009-10-21 14:26:34 PDT

There seems to be two bugs here. 1. Errors that occur after a notifyDone call get output to the next test run in TestShell. Will fix this in TestShell. 2. There's a race between document teardown and an images onload handler. Namely, the image's onload handler is called after the runTest function has be destroyed. It's not clear to me whether this is a V8 or a WebCore bug. Here's what's going on: LayoutTests/editing/selection/drag-to-contenteditable-iframe.html drags an image with onload="runTest()" into a contentEditable region, creating a new image element with it's own onload handler. There's a race between the document teardown and the load event of the dragged in image. The reason LayoutTests/editing/selection/editable-html-element.html is involved is that it happens to run after drag-to-contenteditable-iframe.html. Since the racy onload handler fires after we've called notifyDone, the logged error gets inserted into editable-html-element.html's results.

Ojan Vafai

Comment 15 2009-10-21 14:44:42 PDT

OK. Nevermind. There's just the TestShell bug. runTest is undefined because it's run in the context of the iframe.

Note You need to log in before you can comment on or make changes to this bug.