|Summary:||[Qt][WK2] http/tests/navigation/https-in-page-cache.html fails with timeout|
|Product:||WebKit||Reporter:||Csaba Osztrogonác <ossy>|
|Component:||Tools / Tests||Assignee:||Michael Brüning <michael.bruning>|
|Severity:||Critical||CC:||hausmann, menard, michael.bruning, ossy, webkit.review.bot, zoltan|
|Version:||528+ (Nightly build)|
|Bug Depends on:|
Description Csaba Osztrogonác 2012-04-10 06:45:10 PDT
This test was skipped long long time ago, and was unskipped by http://trac.webkit.org/changeset/113427, but unfortunately it fails on Qt5-WK2 platform (pass with Qt4.8-WK1 and Qt5-WK2) with notifyDone timeout: --- /home/webkitbuildbot/slaves/release64bitWebKit2_EC2/buildslave/qt-linux-64-release-webkit2/build/layout-test-results/http/tests/navigation/https-in-page-cache-expected.txt +++ /home/webkitbuildbot/slaves/release64bitWebKit2_EC2/buildslave/qt-linux-64-release-webkit2/build/layout-test-results/http/tests/navigation/https-in-page-cache-actual.txt @@ -1,7 +1,2 @@ -ALERT: This page is https and has the no-store cache-control directive. It should NOT go in to the page cache. -ALERT: The page was reloaded on back, not from the page cache. Good job. Running part 2 of the test. -ALERT: This page is https and has the no-cache cache-control directive. It should NOT go in to the page cache. -ALERT: The page was reloaded on back, not from the page cache. Good job. Running part 3 of the test. -ALERT: This page is https and should go in to the page cache. -ALERT: The page was restored from the page cache. Good job! +FAIL: Timed out waiting for notifyDone to be called
Comment 1 Csaba Osztrogonác 2012-04-10 06:53:52 PDT
Skipped by http://trac.webkit.org/changeset/113712 Please unskip it with the proper fix.
Comment 2 Michael Brüning 2012-05-14 01:24:23 PDT
Taking a look.
Comment 3 Michael Brüning 2012-05-14 03:45:34 PDT
Seems this is not specific to qt: https://bugs.webkit.org/show_bug.cgi?id=81622
Comment 6 Simon Hausmann 2012-05-14 12:39:15 PDT
Comment on attachment 141754 [details] Patch Good catch! I think your patch is correct in principle, the WK1 DRTs do something similar. There's one difference between WK1 and your patch that I think should be corrected: In WK1 (Mac and Qt) only SSL certificates for 127.0.0.1 and localhost are "blindly" accepted. Also I think the private C++ API can go directly into QQuickWebView, see the "Private C++-only API" comment. This isn't an experimental after all, we know it's private only. I suppose a comment next to the function could indicate that it's used by WTR, and perhaps it should be called setAllowAnyHTTPSCertificateForLocalHost() or so.
Comment 7 Simon Hausmann 2012-05-14 12:40:29 PDT
Change status to assigned to make it clear that Michael is working on this one :)
Comment 9 Michael Brüning 2012-05-14 16:51:34 PDT
Thanks for the review (and the bug state change ;)), Simon. The new version of the patch checks whether the hostname is either localhost or 127.0.0.1 before accepting the certificate. I hope this will also work on the bots (not sure if they are using the localhost/127.0.0.1 config)...
Comment 10 Simon Hausmann 2012-05-14 22:58:12 PDT
Comment on attachment 141811 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=141811&action=review r=me. Only one small nitpick to fix when landing :) > Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:565 > + && (hostname == QString::fromAscii("127.0.0.1") || hostname == QString::fromAscii("localhost"))) The string comparision should avoid allocating a new QString and instead use QStringLiteral, i.e. if (hostName == QStringLiteral("127.0.0.1"))
Comment 12 Michael Brüning 2012-05-15 00:05:35 PDT
Comment on attachment 141868 [details] Patch Thanks for the review, nitpick has bee fixed... It seems that webkit-patch uploade cleared the review flag :-/. Could you please r+ again :)
Comment 13 WebKit Review Bot 2012-05-15 01:05:31 PDT
Comment on attachment 141868 [details] Patch Rejecting attachment 141868 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1 ERROR: /mnt/git/webkit-commit-queue/Source/WebKit2/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: http://queues.webkit.org/results/12704341
Comment 14 Michael Brüning 2012-05-15 01:33:07 PDT
Created attachment 141890 [details] Patch with fixed reviewer in ChangeLog. Fixed reviewer in ChangeLog to please the commit bot :)
Comment 15 WebKit Review Bot 2012-05-15 02:38:27 PDT
Comment on attachment 141890 [details] Patch with fixed reviewer in ChangeLog. Clearing flags on attachment: 141890 Committed r117045: <http://trac.webkit.org/changeset/117045>
Comment 16 Michael Brüning 2012-05-15 08:08:25 PDT