31200 2009-11-06 00:28:07 -0800 Tests in http/tests/security/mixedContent start to fail when new tests are added 2009-11-11 14:09:33 -0800 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) Mac OS X 10.5 RESOLVED FIXED P2 Normal --- 29972 31301 1 rolandsteiner mrowe abarth ap bdakin beidson darin eric mrowe oldest_to_newest 161102 0 rolandsteiner 2009-11-06 00:28:07 -0800 Discovered in r50495: when adding layout tests to the test set, http/tests/security/mixedContent/about-blank-iframe-in-main-frame.html fails as it has additional frame load delegate callbacks in the output. To reproduce (as of writing this bug report): add more than 12 layout tests to the test set that run before http/tests/security/mixedContent/about-blank-iframe-in-main-frame.html (i.e., are before it in lexical order). Apart from the tests in fast/ruby that initially caused this, it also reproduced when duplicating fast/lists, fast/clip or fast/block/basic. Tested on 2 different Mac Pros (only seems to occur on Mac). [CC'ing abarth, ap, brady, as discussed with bdash on IRC] 161103 1 rolandsteiner 2009-11-06 00:43:22 -0800 Correction: 161110 2 rolandsteiner 2009-11-06 01:40:48 -0800 [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. 162253 3 bdakin 2009-11-10 15:32:49 -0800 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. 162261 4 abarth 2009-11-10 15:56:56 -0800 (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. 162264 5 mrowe 2009-11-10 15:58:18 -0800 (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. 162266 6 bdakin 2009-11-10 16:02:04 -0800 (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. 162268 7 eric 2009-11-10 16:02:21 -0800 *** Bug 31305 has been marked as a duplicate of this bug. *** 162270 8 eric 2009-11-10 16:04:08 -0800 It's also blocking two patches in the commit queue, see bug 31305. :) 162292 9 mrowe 2009-11-10 17:00:22 -0800 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.") 162298 10 mrowe 2009-11-10 17:04:57 -0800 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. 162300 11 mrowe 2009-11-10 17:15:10 -0800 I have a fix for this. 162302 12 mrowe 2009-11-10 17:29:50 -0800 Fixed in r50783. 162665 13 42997 ap 2009-11-11 13:56:53 -0800 Created attachment 42997 follow-up fix for Tiger One patch per bug! 162666 14 42997 darin 2009-11-11 13:58:42 -0800 Comment on attachment 42997 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. 162668 15 42997 mrowe 2009-11-11 13:59:13 -0800 Comment on attachment 42997 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. 162674 16 ap 2009-11-11 14:09:33 -0800 > 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 42997 2009-11-11 13:56:53 -0800 2009-11-11 13:59:13 -0800 follow-up fix for Tiger TigerSSL.txt text/plain 1578 ap SW5kZXg6IFdlYktpdFRvb2xzL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJLaXRUb29scy9D aGFuZ2VMb2cJKHJldmlzaW9uIDUwODE3KQorKysgV2ViS2l0VG9vbHMvQ2hhbmdlTG9nCSh3b3Jr aW5nIGNvcHkpCkBAIC0xLDMgKzEsMTMgQEAKKzIwMDktMTEtMTEgIEFsZXhleSBQcm9za3VyeWFr b3YgIDxhcEBhcHBsZS5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISku CisKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTMxMjAw CisgICAgICAgIFRlc3RzIGluIGh0dHAvdGVzdHMvc2VjdXJpdHkvbWl4ZWRDb250ZW50IHN0YXJ0 IHRvIGZhaWwgd2hlbiBuZXcgdGVzdHMgYXJlIGFkZGVkCisKKyAgICAgICAgKiBEdW1wUmVuZGVy VHJlZS9tYWMvRHVtcFJlbmRlclRyZWUubW06IChkdW1wUmVuZGVyVHJlZSk6IE1hZ2ljYWxseSBt YWtlIHRoZSB3b3JrYXJvdW5kCisgICAgICAgIHdvcmsgb24gVGlnZXIsIHRvby4KKwogMjAwOS0x MS0xMSAgS2VubmV0aCBSb2hkZSBDaHJpc3RpYW5zZW4gIDxrZW5uZXRoQHdlYmtpdC5vcmc+CiAK ICAgICAgICAgUmV2aWV3ZWQgYnkgU2ltb24gSGF1c21hbm4uCkluZGV4OiBXZWJLaXRUb29scy9E dW1wUmVuZGVyVHJlZS9tYWMvRHVtcFJlbmRlclRyZWUubW0KPT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gV2ViS2l0 VG9vbHMvRHVtcFJlbmRlclRyZWUvbWFjL0R1bXBSZW5kZXJUcmVlLm1tCShyZXZpc2lvbiA1MDgx NykKKysrIFdlYktpdFRvb2xzL0R1bXBSZW5kZXJUcmVlL21hYy9EdW1wUmVuZGVyVHJlZS5tbQko d29ya2luZyBjb3B5KQpAQCAtNjA0LDYgKzYwNCwxMCBAQCB2b2lkIGR1bXBSZW5kZXJUcmVlKGlu dCBhcmdjLCBjb25zdCBjaGFyCiAKICAgICAvLyA8aHR0cDovL3dlYmtpdC5vcmcvYi8zMTIwMD4g SW4gb3JkZXIgdG8gcHJldmVudCBleHRyYSBmcmFtZSBsb2FkIGRlbGVnYXRlIGxvZ2dpbmcgYmVp bmcgZ2VuZXJhdGVkIGlmIHRoZSBmaXJzdCB0ZXN0IHRvIHVzZSBTU0wKICAgICAvLyBpcyBzZXQg dG8gbG9nIGZyYW1lIGxvYWQgZGVsZWdhdGUgY2FsbHMgd2UgaWdub3JlIFNTTCBjZXJ0aWZpY2F0 ZSBlcnJvcnMgb24gbG9jYWxob3N0IGFuZCAxMjcuMC4wLjEuCisjaWYgQlVJTERJTkdfT05fVElH RVIKKwkvLyBJbml0aWFsaXplIGludGVybmFsIE5TdVJMUmVxdWVzdCBkYXRhIGZvciBzZXRBbGxv d3NBbnlIVFRQU0NlcnRpZmljYXRlOmZvckhvc3Q6IHRvIHdvcmsgcHJvcGVybHkuCisJW1tbW05T VVJMUmVxdWVzdCBhbGxvY10gaW5pdF0gYXV0b3JlbGVhc2VdIEhUVFBNZXRob2RdOworI2VuZGlm CiAgICAgW05TVVJMUmVxdWVzdCBzZXRBbGxvd3NBbnlIVFRQU0NlcnRpZmljYXRlOllFUyBmb3JI b3N0OkAibG9jYWxob3N0Il07CiAgICAgW05TVVJMUmVxdWVzdCBzZXRBbGxvd3NBbnlIVFRQU0Nl cnRpZmljYXRlOllFUyBmb3JIb3N0OkAiMTI3LjAuMC4xIl07CiAK