Bug 43840

Summary: fast/loader/recursive-before-unload-crash.html is flaky
Product: WebKit Reporter: Steve Block <steveblock>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, ap, beidson, commit-queue, dglazkov, eric, jamesr
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 38928, 50880    
Attachments:
Description Flags
Patch ap: review+, ap: commit-queue+

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>