Bug 36688

Summary: [Qt] LayoutTests/http/tests/appcache/auth.html failed and skipped
Product: WebKit Reporter: Chang Shu <cshu>
Component: New BugsAssignee: Chang Shu <cshu>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, commit-queue, eric, kling, laszlo.gombos, markus, ossy, webkit.review.bot
Priority: P3 Keywords: Qt
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Bug Depends on:    
Bug Blocks: 34712    
Attachments:
Description Flags
Patch to unskip auth.html from Skipped.
none
fix patch
kling: review+, kling: commit-queue-
fix patch 2 none

Description Chang Shu 2010-03-26 18:29:11 PDT
The above test failed on Qt Linux. The test result shows time out.
Comment 1 Chang Shu 2010-03-30 11:22:36 PDT
I have investigated the issue and it looks to me the problem is under QNetwork layer.
The test case will send several http requests in a row. The first one to "setup.php" contains the login/password info and it went through. The second request to "iframe.php" does not contain user/password in its url and the QNetwork layer is supposed to fetch the credential from its cache. However, I didn't see the credential was saved to cache before this. I think after the successful return of the 1st request, the credential should be saved.
Comment 2 Chang Shu 2010-04-16 12:17:04 PDT
I reported a bug in Qt:
http://bugreports.qt.nokia.com/browse/QTBUG-9996
Comment 3 Chang Shu 2010-09-24 12:53:26 PDT
the bug is fixed in Qt4.7. Once the QtWebKit buildbot starts using Qt4.7, we can close this bug.
Comment 4 Markus Goetz 2010-11-03 03:49:56 PDT
Hey,
Can i then close the associated bug in the Qt bugtracker?
http://bugreports.qt.nokia.com/browse/QTBUG-9996
Thanks
Comment 5 krithigassree.sambamurthy 2010-12-21 13:42:43 PST
Created attachment 77147 [details]
Patch to unskip auth.html from Skipped.

Appcache layout test case auth.html can be removed from the Skipped list. Test case passes since switching to Qt4.7 in the buildbot.
Comment 6 WebKit Commit Bot 2010-12-22 08:24:21 PST
The commit-queue encountered the following flaky tests while processing attachment 77147 [details]:

http/tests/local/stylesheet-and-script-load-order-media-print.html bug 51470 (author: koivisto@iki.fi)
The commit-queue is continuing to process your patch.
Comment 7 WebKit Commit Bot 2010-12-22 08:25:48 PST
Comment on attachment 77147 [details]
Patch to unskip auth.html from Skipped.

Clearing flags on attachment: 77147

Committed r74481: <http://trac.webkit.org/changeset/74481>
Comment 8 WebKit Commit Bot 2010-12-22 08:25:54 PST
All reviewed patches have been landed.  Closing bug.
Comment 9 Laszlo Gombos 2010-12-22 08:47:08 PST
It seems to me that running http/tests/appcache/auth.html makes the http/tests/misc/401-alternative-content.php flaky.
Comment 10 WebKit Review Bot 2010-12-22 10:35:13 PST
http://trac.webkit.org/changeset/74481 might have broken GTK Linux 64-bit Debug
The following tests are not passing:
http/tests/local/stylesheet-and-script-load-order-media-print.html
Comment 11 Chang Shu 2010-12-22 12:47:12 PST
The patch was reverted by http://trac.webkit.org/changeset/74486
as it causes regression. A re-work is going on.
Comment 12 Eric Seidel (no email) 2010-12-23 00:40:32 PST
What was the regression?  The stylesheet test failure?  If so, please dup bug 51470 against this one?
Comment 13 Csaba Osztrogonác 2011-01-03 10:42:00 PST
*** Bug 51829 has been marked as a duplicate of this bug. ***
Comment 14 Csaba Osztrogonác 2011-01-03 10:44:26 PST
(In reply to comment #13)
> *** Bug 51829 has been marked as a duplicate of this bug. ***

After http://trac.webkit.org/changeset/74896 http/tests/misc/401-alternative-content.php fails always if I run all of the tests, but pass if I run only this test. So it must be a Qt-DRT sideeffect bug, and not flakyness as Laszlo said.

pretty diff: http://build.webkit.org/results/Qt%20Linux%20Release/r74900%20%2825916%29/http/tests/misc/401-alternative-content-pretty-diff.html
Comment 15 Chang Shu 2011-01-28 07:27:30 PST
Created attachment 80449 [details]
fix patch
Comment 16 Andreas Kling 2011-01-28 08:33:23 PST
Comment on attachment 80449 [details]
fix patch

View in context: https://bugs.webkit.org/attachment.cgi?id=80449&action=review

r=me, with one adjustment:

> Tools/DumpRenderTree/qt/DumpRenderTreeQt.cpp:560
> +    if (isHttpTest(url)) {

The helper method is a bit misleading, I think we should simply inline the check here, and also make it more specific, i.e:
if (url.scheme() == "http" || url.scheme() == "https")
Comment 17 Chang Shu 2011-01-28 08:46:41 PST
Laszlo suggested to do network cleanup for all tests, including non-http. This clean up the code. I ran part of the tests so far and no failures and no performance overhead observed. Andreas, do you prefer this change?
Comment 18 Andreas Kling 2011-01-28 09:06:51 PST
(In reply to comment #17)
> Laszlo suggested to do network cleanup for all tests, including non-http. This clean up the code. I ran part of the tests so far and no failures and no performance overhead observed. Andreas, do you prefer this change?

If it has no tangible performance overhead, sure.
Comment 19 Chang Shu 2011-01-28 09:06:52 PST
Created attachment 80456


patch based on Kling's comments.
Comment 20 Chang Shu 2011-01-28 09:08:24 PST
Created attachment 80457 [details]
fix patch 2
Comment 21 WebKit Commit Bot 2011-01-28 11:16:35 PST
Comment on attachment 80457 [details]
fix patch 2

Clearing flags on attachment: 80457

Committed r76958: <http://trac.webkit.org/changeset/76958>
Comment 22 WebKit Commit Bot 2011-01-28 11:16:40 PST
All reviewed patches have been landed.  Closing bug.