Bug 36934 - REGRESSION(56394): http/tests/xmlhttprequest/xmlhttprequest-50ms-download-dispatch.html fails on Tiger
Summary: REGRESSION(56394): http/tests/xmlhttprequest/xmlhttprequest-50ms-download-dis...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh OS X 10.4
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 18654
Blocks:
  Show dependency treegraph
 
Reported: 2010-03-31 23:53 PDT by Eric Seidel (no email)
Modified: 2010-04-05 10:30 PDT (History)
4 users (show)

See Also:


Attachments
Add more explicit comments to the test for debugging this issue (3.16 KB, patch)
2010-04-01 08:41 PDT, Julien Chaffraix
no flags Details | Formatted Diff | Diff
Patch (1.32 KB, patch)
2010-04-02 23:03 PDT, Adam Barth
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2010-03-31 23:53:33 PDT
REGRESSION(56394): http/tests/xmlhttprequest/xmlhttprequest-50ms-download-dispatch.html fails on Tiger

This test has been failing since it was committed.  It may just need Tiger-specific results checked in, but that is very unclear from the (not very helpful) pass/fail output.

http://build.webkit.org/results/Tiger%20Intel%20Release/r56893%20(10281)/http/tests/xmlhttprequest/xmlhttprequest-50ms-download-dispatch-diffs.txt

--- /Volumes/Data/WebKit-BuildSlave/tiger-intel-release/build/layout-test-results/http/tests/xmlhttprequest/xmlhttprequest-50ms-download-dispatch-expected.txt	2010-03-31 21:39:46.000000000 -0700
+++ /Volumes/Data/WebKit-BuildSlave/tiger-intel-release/build/layout-test-results/http/tests/xmlhttprequest/xmlhttprequest-50ms-download-dispatch-actual.txt	2010-03-31 21:39:46.000000000 -0700
@@ -5,8 +5,8 @@
 You should see a sequence of 5 PASSED.
 
 PASSED
+FAILED (count was 1)
 PASSED
 PASSED
-PASSED
-PASSED
+FAILED (count was 1)
Comment 1 Eric Seidel (no email) 2010-03-31 23:54:30 PDT
I see, the diff is just hard to read.  Tiger is simply failing 2 of the 5 subtests.  My "not very helpful" comment above was itself not very helpful.  What I meant was that the PASSED/FAILED messages were not self-documenting.
Comment 2 Julien Chaffraix 2010-04-01 08:37:36 PDT
(In reply to comment #1)
> I see, the diff is just hard to read.  Tiger is simply failing 2 of the 5
> subtests.  My "not very helpful" comment above was itself not very helpful. 
> What I meant was that the PASSED/FAILED messages were not self-documenting.

You are right about the message. I will attach a patch to have more explicit messages that will help find out what is failing. It is likely the 2 tests that fails are the 2 throttled ones, the other ones just make sure there is no throttling.

Thanks for filing this bug. I will be more careful with the Tiger build-bot next time.
Comment 3 Julien Chaffraix 2010-04-01 08:41:47 PDT
Created attachment 52299 [details]
Add more explicit comments to the test for debugging this issue
Comment 4 Eric Seidel (no email) 2010-04-01 11:22:41 PDT
Comment on attachment 52299 [details]
Add more explicit comments to the test for debugging this issue

We would need to update the expected results when checking this in if we didn't want failures.
Comment 5 Julien Chaffraix 2010-04-01 11:49:51 PDT
(In reply to comment #4)
> (From update of attachment 52299 [details])
> We would need to update the expected results when checking this in if we didn't
> want failures.

No, I only made the failures more clear.
The test does asynchronous XHRs which means that saying which one passed is not reliable 100% of the time. There is value in keeping it as such (like checking that the throttling will still work properly with several concurrent XHRs). That's why I did not want to update the expected results.
Comment 6 Eric Seidel (no email) 2010-04-01 11:57:26 PDT
Comment on attachment 52299 [details]
Add more explicit comments to the test for debugging this issue

OK.
Comment 7 WebKit Commit Bot 2010-04-01 12:23:45 PDT
Comment on attachment 52299 [details]
Add more explicit comments to the test for debugging this issue

Clearing flags on attachment: 52299

Committed r56928: <http://trac.webkit.org/changeset/56928>
Comment 8 WebKit Commit Bot 2010-04-01 12:23:52 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Julien Chaffraix 2010-04-01 20:15:38 PDT
Reopening this bug as it was not mean to be closed.

The Tiger build bot fails consistently 2 strict tests (my previous analysis was completely off):

FAILED (count was 1) for strict test 1
FAILED (count was 1) for strict test 3

Looking at http/tests/xmlhttprequest/xmlhttprequest-50ms-download-dispatch.html, the only strict test that passes is expecting 1 'progress' event. The throttled tests would also pass with only 1 event. Without more information, it seems like the network library buffers the input.

Increasing the time between chunks of data could help but we may miss some corner cases at the 50 ms limit (which would be difficult to get reliably on the bots). I will go ahead and increase it in the following days to make the bot happy.
Comment 10 Eric Seidel (no email) 2010-04-01 20:20:46 PDT
I do seem to remember Tiger's CFNetwork having some sort of buffer for loading http requests off the network.  But I could be remembering wrong.  AP or some other Apple person would know.
Comment 11 Alexey Proskuryakov 2010-04-01 20:44:17 PDT
Yes, our code that disables content sniffing (and thus buffering) only works on Leopard and later. Good catch!
Comment 12 Eric Seidel (no email) 2010-04-02 00:36:44 PDT
(In reply to comment #11)
> Yes, our code that disables content sniffing (and thus buffering) only works on
> Leopard and later. Good catch!

So I'm unclear from your remark.  Is there a code fix to make here, or just a test fix?
Comment 13 Alexey Proskuryakov 2010-04-02 08:27:17 PDT
I was answering your question in comment 10. Certainly XMLHttpRequest behavior is different on Tiger because of buffering, but I haven't investigated if test failures are caused by this difference.
Comment 14 Eric Seidel (no email) 2010-04-02 09:58:14 PDT
Adam Barth fixed another failing test in Tiger last night by moving it from using GET to POST. http://trac.webkit.org/changeset/56979   I'm not sure if the same workaround is useful in this context or not.
Comment 15 Julien Chaffraix 2010-04-02 15:17:22 PDT
(In reply to comment #14)
> Adam Barth fixed another failing test in Tiger last night by moving it from
> using GET to POST. http://trac.webkit.org/changeset/56979   I'm not sure if the
> same workaround is useful in this context or not.

I think this would apply here too. I tried such a change and it did not make the test fail. I will create a patch tonight (I cannot come back to that before).
Comment 16 Alexey Proskuryakov 2010-04-02 15:20:36 PDT
Oh, I think that we could perhaps use a non-text MIME type to disable buffering. My recollection is that CFNetwork only does content sniffing for types like text/plain, because that often stands for "unknown".
Comment 17 Adam Barth 2010-04-02 23:03:50 PDT
Created attachment 52483 [details]
Patch
Comment 18 Adam Barth 2010-04-02 23:15:12 PDT
Comment on attachment 52483 [details]
Patch

Clearing flags on attachment: 52483

Committed r57044: <http://trac.webkit.org/changeset/57044>
Comment 19 Adam Barth 2010-04-02 23:34:51 PDT
ap for the win.  Seems to have worked.  Welcome to a green Tiger bot.
Comment 20 Alexey Proskuryakov 2010-04-05 10:26:02 PDT
Great! Can this bug be marked as resolved now?
Comment 21 Eric Seidel (no email) 2010-04-05 10:30:36 PDT
Looks resolved to me.