Bug 31200

Correction:

[Sorry about the comment spam, didn't mean to commit the previous one yet] Corrections and additions after even more testing: Actually different tests in http/tests/security/mixedContent start to fail, not just about-blank-iframe-in-main-frame.html. However, it's only ever 1 test that fails, and which test fails seems to be consistent for a given set of added tests. Furthermore, if you remove 1 test from the added test set, the next test in line in http/tests/security/mixedContent fails. It therefore seems this is caused strictly by the number of tests (rather than any content) that come before. And to even further add to the fun: if you add a LOT of layout tests (e.g., duplicating fast/block and fast/clip), everything runs through fine again. This leads me to believe that perhaps something in http/tests/security/mixedContent is sensitive to some sort of run-over of the layout count. I.e., a test in this directory fails if it happens to be exactly the, say, 1000th test.

The bot is officially red because of this issue now. I added new tests for a new feature, and this bug is officially exposed. We should try to find a quick resolution to avoid a red bot.

(In reply to comment #3) > The bot is officially red because of this issue now. I added new tests for a > new feature, and this bug is officially exposed. We should try to find a quick > resolution to avoid a red bot. Huh? Why is it ok for you to land a patch that turns the tree red? It seems fixing this bug is a re-requisite for landing that patch.

(In reply to comment #4) > (In reply to comment #3) > > The bot is officially red because of this issue now. I added new tests for a > > new feature, and this bug is officially exposed. We should try to find a quick > > resolution to avoid a red bot. > > Huh? Why is it ok for you to land a patch that turns the tree red? It seems > fixing this bug is a re-requisite for landing that patch. Presumably because she didn’t realize that would happen.

(In reply to comment #5) > (In reply to comment #4) > > (In reply to comment #3) > > > The bot is officially red because of this issue now. I added new tests for a > > > new feature, and this bug is officially exposed. We should try to find a quick > > > resolution to avoid a red bot. > > > > Huh? Why is it ok for you to land a patch that turns the tree red? It seems > > fixing this bug is a re-requisite for landing that patch. > > Presumably because she didn’t realize that would happen. Indeed I did not. Also, we can not have a bug that blocks adding new tests. That's insane.

*** Bug 31305 has been marked as a duplicate of this bug. ***

It's also blocking two patches in the commit queue, see bug 31305. :)

Printing out the error reveals: main frame - didFailProvisionalLoadWithError Error Domain=NSURLErrorDomain Code=-1202 UserInfo=0x10269f380 "The certificate for this server is invalid. You might be connecting to a server that is pretending to be “127.0.0.1” which could put your confidential information at risk." Underlying Error=(Error Domain=kCFErrorDomainCFNetwork Code=-1202 UserInfo=0x1026d3400 "The certificate for this server is invalid. You might be connecting to a server that is pretending to be “127.0.0.1” which could put your confidential information at risk.")

Ok, this is trivial to reproduce: run-webkit-tests --nthly=1 http/tests/security/mixedContent Every test fails in this manner. If you instead did: run-webkit-tests --nthly=2 http/tests/security/mixedContent you'd see an interesting pattern where every second test fails in the same way. This makes the problem relatively obvious: the first test in a given instance of DRT to use HTTPS gets extra frame load callbacks as a result of the self-signed SSL certificate. It tells the network layer to ignore the certificate errors and then retry. The new tests had the effect of shifting the point at which DRT was restarted, exposing this issue.

I have a fix for this.

Fixed in r50783.

Created attachment 42997 [details] follow-up fix for Tiger One patch per bug!

Comment on attachment 42997 [details] follow-up fix for Tiger > +#if BUILDING_ON_TIGER > + // Initialize internal NSuRLRequest data for setAllowsAnyHTTPSCertificate:forHost: to work properly. > + [[[[NSURLRequest alloc] init] autorelease] HTTPMethod]; > +#endif Bad spelling of NSURLRequest. Comment should use the phrase "work around for Tiger bug", I think.

Comment on attachment 42997 [details] follow-up fix for Tiger > Index: WebKitTools/DumpRenderTree/mac/DumpRenderTree.mm > =================================================================== > --- WebKitTools/DumpRenderTree/mac/DumpRenderTree.mm (revision 50817) > +++ WebKitTools/DumpRenderTree/mac/DumpRenderTree.mm (working copy) > @@ -604,6 +604,10 @@ void dumpRenderTree(int argc, const char > > // <http://webkit.org/b/31200> In order to prevent extra frame load delegate logging being generated if the first test to use SSL > // is set to log frame load delegate calls we ignore SSL certificate errors on localhost and 127.0.0.1. > +#if BUILDING_ON_TIGER > + // Initialize internal NSuRLRequest data for setAllowsAnyHTTPSCertificate:forHost: to work properly. > + [[[[NSURLRequest alloc] init] autorelease] HTTPMethod]; > +#endif You appear to have inserted some tabs here! And you’re missing an uppercase U in NSURLRequest.

> Comment should use the phrase "work around for > Tiger bug", I think. It's not an API, so while I'm not sure that its behavior on Tiger was formally a bug. But OK. Committed r50842