Bug 70619 - REGRESSION (r97496-r97499): 6 fast/workers tests failing on SnowLeopard Intel Release (Tests)
Summary: REGRESSION (r97496-r97499): 6 fast/workers tests failing on SnowLeopard Intel...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.6
: P2 Normal
Assignee: David Levin
URL: http://build.webkit.org/results/SnowL...
Keywords: LayoutTestFailure, MakingBotsRed, Regression
Depends on: 71695
Blocks:
  Show dependency treegraph
 
Reported: 2011-10-21 10:46 PDT by Simon Fraser (smfr)
Modified: 2011-12-09 18:27 PST (History)
11 users (show)

See Also:


Attachments
Patch (1.57 KB, patch)
2011-12-09 14:14 PST, David Levin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2011-10-21 10:46:14 PDT
The following tests started failing on SnowLeopard Intel Release (Tests) between r97497 and r97499 (inclusive):

    fast/workers/shared-worker-lifecycle.html
    fast/workers/shared-worker-frame-lifecycle.html
    fast/workers/storage/interrupt-database.html
    fast/workers/worker-close-more.html
    fast/workers/dedicated-worker-lifecycle.html
    fast/workers/worker-lifecycle.html

http://trac.webkit.org/log/trunk?rev=97499&stop_rev=97497&limit=4

http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20(Tests)/r97496%20(33906)/results.html passed
http://build.webkit.org/results/SnowLeopard%20Intel%20Release%20(Tests)/r97499%20(33907)/results.html failed
Comment 1 Simon Fraser (smfr) 2011-10-21 10:46:34 PDT
Please can you try to make these workers tests less flakey? They cause so much bot redness.
Comment 2 David Levin 2011-10-21 11:01:28 PDT
Adding dslomov as fyi.

I think Drew added most of these except for the interrupt-database one which I think Michael owns.

I think everyone is booked at the moment but hopefully one of us will be able to look at this soon.
Comment 3 Simon Fraser (smfr) 2011-11-02 17:10:06 PDT
We need to get these workers tests skipped, or passing. Their flakiness is not acceptable.
Comment 4 David Levin 2011-11-02 17:14:41 PDT
Feel free to skip them unless someone volunteers to fix them (which doesn't look like it is happening at present).
Comment 5 Simon Fraser (smfr) 2011-11-02 17:16:51 PDT
Skipped in http://trac.webkit.org/changeset/99125
Comment 6 Andrew Wilson 2011-11-02 17:19:19 PDT
Yeah, if you look at the failures, they look to be DRT failures more than anything else.

Looking at fast/workers/resources/worker-lifecycle.js:

if (window.layoutTestController) {
    layoutTestController.dumpAsText();
    layoutTestController.waitUntilDone();
    waitUntilWorkerThreadsExit(runTests);
} else {
    log("NOTE: Test relies on window.layoutTestController to detect when worker threads have exited. Please run this test via DumpRenderTree");
    waitUntilWorkerThreadsExit(runTests);
}

function runTests()
{
    if (window.layoutTestController)
        log("PASS: workerThreadCount = " + layoutTestController.workerThreadCount);

I don't see how it's possible for the test output to be missing "PASS: workerThreadCount =" unless layoutTestController.workerThreadCount() just blocks.

So, yeah, it'd be nice for someone to diagnose why DRT is not working with workers on the bots, but it doesn't specifically look like a failure in the worker code.
Comment 7 David Levin 2011-11-02 17:22:59 PDT
I think all of these tests rely on waitUntilWorkerThreadsExit which to my memory is just plain flaky for jsc due to how garbage collection is done there.

I think Drew Wilson tried to fix this but it was usually a form of let's try this because it is hard to force the gc to happen and recognize that an object is not referred to anymore.

I bet they started failing due to all the gc changes that have been happening and I suspect that it isn't worth the trouble to keep them running on jsc. (I believe they work for v8 and this in turn will test the shared code in WebKit for this stuff which is the most important thing.)
Comment 8 Andrew Wilson 2011-11-02 17:27:30 PDT
Still not 100% clear why we're not getting *any* output, but maybe if we get a timeout we don't dump the current text contents of the DOM (clearly my WebKit-fu is getting rusty).
Comment 9 Simon Fraser (smfr) 2011-11-02 17:28:08 PDT
cc some JSC/GC folks.
Comment 10 David Levin 2011-12-07 14:53:58 PST
I bet this is the same as 71695.  If so, this is related to changing to NRWT.


After landing that fix, I'll unskip these and we'll see if they start passing, so I'll take this for now.
Comment 11 David Levin 2011-12-09 14:14:26 PST
Created attachment 118635 [details]
Patch
Comment 12 Alexey Proskuryakov 2011-12-09 14:23:46 PST
Comment on attachment 118635 [details]
Patch

This kind of change generally doesn't need review.
Comment 13 WebKit Review Bot 2011-12-09 18:26:59 PST
Comment on attachment 118635 [details]
Patch

Clearing flags on attachment: 118635

Committed r102510: <http://trac.webkit.org/changeset/102510>
Comment 14 WebKit Review Bot 2011-12-09 18:27:04 PST
All reviewed patches have been landed.  Closing bug.