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
Skipped by http://trac.webkit.org/changeset/113712 Please unskip it with the proper fix.
Taking a look.
Seems this is not specific to qt: https://bugs.webkit.org/show_bug.cgi?id=81622
Created attachment 141744 [details] Patch
Created attachment 141754 [details] Patch
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.
Change status to assigned to make it clear that Michael is working on this one :)
Created attachment 141811 [details] Patch
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 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"))
Created attachment 141868 [details] Patch
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 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
Created attachment 141890 [details] Patch with fixed reviewer in ChangeLog. Fixed reviewer in ChangeLog to please the commit bot :)
Comment on attachment 141890 [details] Patch with fixed reviewer in ChangeLog. Clearing flags on attachment: 141890 Committed r117045: <http://trac.webkit.org/changeset/117045>
Resolving fixed...