Bug 43840 - fast/loader/recursive-before-unload-crash.html is flaky
Summary: fast/loader/recursive-before-unload-crash.html is flaky
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
: 41871 (view as bug list)
Depends on:
Blocks: 38928 50880
  Show dependency treegraph
 
Reported: 2010-08-11 02:05 PDT by Steve Block
Modified: 2010-12-23 00:17 PST (History)
7 users (show)

See Also:


Attachments
Patch (6.62 KB, patch)
2010-09-02 12:27 PDT, Eric Seidel (no email)
ap: review+
ap: commit-queue+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Steve Block 2010-08-11 02:05:18 PDT
This test has been observed to fail on the commit queue bot. See 
https://bugs.webkit.org/show_bug.cgi?id=43443#c5 and #c7.

The test was added in the fix for Bug 38928
Comment 1 Eric Seidel (no email) 2010-09-02 12:05:46 PDT
*** Bug 41871 has been marked as a duplicate of this bug. ***
Comment 2 Eric Seidel (no email) 2010-09-02 12:06:31 PDT
This remains our most flaky test.  We need to either skip it or fix it.  See Bug 41871 for an example of the failure.
Comment 3 Eric Seidel (no email) 2010-09-02 12:11:08 PDT
CQ false rejections in recent memory due to this test:

https://bugs.webkit.org/show_bug.cgi?id=45078#c3
https://bugs.webkit.org/show_bug.cgi?id=41812#c4
https://bugs.webkit.org/show_bug.cgi?id=41684#c5
https://bugs.webkit.org/show_bug.cgi?id=41582#c7
https://bugs.webkit.org/show_bug.cgi?id=39482#c14
https://bugs.webkit.org/show_bug.cgi?id=41317#c4
https://bugs.webkit.org/show_bug.cgi?id=38548#c32

In order to cause a false rejection a test has to fail, then not fail, then fail again!  So 2 out of 3!  A single failure won't cause a false rejection and will instead cause the cq to just spin again.  Flaky tests are the main cause of cq delays actually.
Comment 4 Eric Seidel (no email) 2010-09-02 12:12:01 PDT
Here is the failure diff:

--- /tmp/layout-test-results/fast/loader/recursive-before-unload-crash-expected.txt     2010-07-09 13:38:55.000000000 -0700
+++ /tmp/layout-test-results/fast/loader/recursive-before-unload-crash-actual.txt       2010-07-09 13:38:55.000000000 -0700
@@ -6,8 +6,8 @@
 main frame - didStartProvisionalLoadForFrame
 ALERT: Adding iframe
 frame "<!--framePath //<!--frame0-->-->" - didStartProvisionalLoadForFrame
-main frame - didCancelClientRedirectForFrame
 frame "<!--framePath //<!--frame0-->-->" - didFailProvisionalLoadWithError
+main frame - didCancelClientRedirectForFrame
 main frame - didFailProvisionalLoadWithError
 This test demonstrates a problem with our handling of the beforeunload event.
 If a script manages to try and navigate the frame from beforeunload - when a navigation is already pending - we end up blowing out the stack by recursively consulting the policy delegate then running onbeforeunloa
d repeatedly.
Comment 5 Eric Seidel (no email) 2010-09-02 12:21:17 PDT
BTW, this was added 4 months ago by http://trac.webkit.org/changeset/59384 for https://bugs.webkit.org/show_bug.cgi?id=38928.

http://trac.webkit.org/browser/trunk/LayoutTests/fast/loader/recursive-before-unload-crash.html

It's been flaky since at least 2010-06-07 (see the failure links above) and I believe it's been flaky since it landed.
Comment 6 Eric Seidel (no email) 2010-09-02 12:23:29 PDT
In https://bugs.webkit.org/show_bug.cgi?id=38928#c27 Brady talks about removing the callbacks from the test.  I'll see if I can prep a patch to do so.
Comment 7 Eric Seidel (no email) 2010-09-02 12:27:02 PDT
Created attachment 66395 [details]
Patch
Comment 8 Eric Seidel (no email) 2010-09-02 13:10:54 PDT
Committed r66681: <http://trac.webkit.org/changeset/66681>