15976 2007-11-13 18:02:12 -0800 Throwing an exception within Database.transaction ASSERTs then crashes 2007-11-19 04:17:20 -0800 1 1 1 Unclassified WebKit DOM 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 aroben webkit-unassigned andersca beidson oldest_to_newest 61143 0 aroben 2007-11-13 18:02:12 -0800 Throwing an exception within Database.transaction ASSERTs then crashes. This can happen if there was an explicit throw statement, or if there was a type mismatch error or something similar. 61144 1 17251 aroben 2007-11-13 18:02:26 -0800 Created attachment 17251 testcase 61147 2 beidson 2007-11-13 18:37:02 -0800 I made an assumption that the transaction error callback would only be scheduled, therefore invoked, if it was actually set. The problem is when we make the transaction callback and it immediately invokes the transaction error callback, the assumption I made is no longer valid. Basically, it's just a bogus assert the logic should change to involve a null check. The spec already accounts for this 61149 3 17254 beidson 2007-11-13 18:46:12 -0800 Created attachment 17254 Proposed fix 61160 4 17254 aroben 2007-11-13 21:01:25 -0800 Comment on attachment 17254 Proposed fix + m_transactionError = new SQLError(0, "the SQLTransactionCallback was null or threw an exception"); Do we need to localize this string? Should "the" be capitalized? Where is this text used? It doesn't look like m_transactionError gets set everywhere deliverTransactionErrorCallback() is called -- do we need to do it elsewhere? What's the rule? r- until there's a regression test (you should be able to turn the attached testcase into a test pretty easily). 61163 5 beidson 2007-11-13 21:22:41 -0800 > Do we need to localize this string? no (not required, and we don't for any other DOM error messages) > Should "the" be capitalized? no (we don't for any other DOM error messages) > Where is this text used? Wherever the web developer chooses - it's an attribute on the SQLError they get, in addition to the error code (0, in this case) > It doesn't look like m_transactionError gets set everywhere deliverTransactionErrorCallback() is called -- do we need to do it elsewhere? What's the rule? Actually it was set everywhere else - The only other entry point to deliverTransactonErrorCallback() is through handleTransactionError(), and a quick audit of each site that calls that shows the error being set first. This was a unique case, jumping from one callback straight to the error callback. What the assertion was designed to catch! I overlooked the fact that this was easily layout-testable since I still haven't gotten to DRT infrastructure for a real database test suite, whereas this one is just a "crash or doesn't" test. I will add that and put a new patch up 61174 6 17260 beidson 2007-11-13 22:37:39 -0800 Created attachment 17260 Proposed fix w/ layout test 61175 7 17260 aroben 2007-11-13 22:40:11 -0800 Comment on attachment 17260 Proposed fix w/ layout test r=me We should probably add a way for DRT to clear databases created by tests. 61177 8 beidson 2007-11-13 22:44:47 -0800 Thanks for the review. I plan on augmenting DRT thouroughly when adding a database layout test suite, and that is one enhancement I had in mind! 61657 9 mrowe 2007-11-19 04:17:20 -0800 This was landed in r27784. 17251 2007-11-13 18:02:26 -0800 2007-11-13 18:02:26 -0800 testcase database.html text/html 211 aroben PHA+VGhpcyBwYWdlIHNob3VsZCBub3QgY3Jhc2ggV2ViS2l0LjwvcD4KPHNjcmlwdD4KICAgIHZh ciBkYiA9IG9wZW5EYXRhYmFzZSgiQnVnVGVzdCIsICIwLjEiLCAiVGVzdCBmb3IgYSBidWciLCAx KTsKICAgIHRyeSB7CiAgICAgICAgZGIudHJhbnNhY3Rpb24oZnVuY3Rpb24odHgpIHsgdGhyb3cg MCB9KTsKICAgIH0gY2F0Y2ggKGUpIHsKICAgIH0KPC9zY3JpcHQ+Cg== 17254 2007-11-13 18:46:12 -0800 2007-11-13 21:01:25 -0800 Proposed fix patch.txt text/plain 2117 beidson SW5kZXg6IHN0b3JhZ2UvU1FMVHJhbnNhY3Rpb24uY3BwCj09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIHN0b3JhZ2Uv U1FMVHJhbnNhY3Rpb24uY3BwCShyZXZpc2lvbiAyNzc3OSkKKysrIHN0b3JhZ2UvU1FMVHJhbnNh Y3Rpb24uY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0xNDUsOSArMTQ1LDEwIEBACiAgICAgICAgIHNo b3VsZERlbGl2ZXJFcnJvckNhbGxiYWNrID0gdHJ1ZTsKIAogICAgIC8vIFRyYW5zYWN0aW9uIFN0 ZXAgNSAtIElmIHRoZSB0cmFuc2FjdGlvbiBjYWxsYmFjayB3YXMgbnVsbCBvciByYWlzZWQgYW4g ZXhjZXB0aW9uLCBqdW1wIHRvIHRoZSBlcnJvciBjYWxsYmFjawotICAgIGlmIChzaG91bGREZWxp dmVyRXJyb3JDYWxsYmFjaykKKyAgICBpZiAoc2hvdWxkRGVsaXZlckVycm9yQ2FsbGJhY2spIHsK KyAgICAgICAgbV90cmFuc2FjdGlvbkVycm9yID0gbmV3IFNRTEVycm9yKDAsICJ0aGUgU1FMVHJh bnNhY3Rpb25DYWxsYmFjayB3YXMgbnVsbCBvciB0aHJldyBhbiBleGNlcHRpb24iKTsKICAgICAg ICAgZGVsaXZlclRyYW5zYWN0aW9uRXJyb3JDYWxsYmFjaygpOwotICAgIGVsc2UKKyAgICB9IGVs c2UKICAgICAgICAgc2NoZWR1bGVUb1J1blN0YXRlbWVudHMoKTsKIH0KIApAQCAtMjg5LDExICsy OTAsMTEgQEAKIAogdm9pZCBTUUxUcmFuc2FjdGlvbjo6ZGVsaXZlclRyYW5zYWN0aW9uRXJyb3JD YWxsYmFjaygpCiB7Ci0gICAgQVNTRVJUKG1fZXJyb3JDYWxsYmFjayk7CiAgICAgQVNTRVJUKG1f dHJhbnNhY3Rpb25FcnJvcik7Ci0gICAgCisKICAgICAvLyBUcmFuc2FjdGlvbiBTdGVwIDExIC0g SWYgdGhlIGNhbGxiYWNrIGRpZG4ndCByZXR1cm4gZmFsc2UsIHRoZW4gcm9sbGJhY2sgdGhlIHRy YW5zYWN0aW9uLgotICAgIGlmIChtX2Vycm9yQ2FsbGJhY2stPmhhbmRsZUV2ZW50KG1fdHJhbnNh Y3Rpb25FcnJvci5nZXQoKSkpCisgICAgLy8gVGhpcyBpbmNsdWRlcyB0aGUgY2FsbGJhY2sgbm90 IGV4aXN0aW5nLCByZXR1cm5pbmcgdHJ1ZSwgb3IgdGhyb3dpbmcgYW4gZXhjZXB0aW9uCisgICAg aWYgKCFtX2Vycm9yQ2FsbGJhY2sgfHwgbV9lcnJvckNhbGxiYWNrLT5oYW5kbGVFdmVudChtX3Ry YW5zYWN0aW9uRXJyb3IuZ2V0KCkpKQogICAgICAgICBtX3Nob3VsZENvbW1pdEFmdGVyRXJyb3JD YWxsYmFjayA9IGZhbHNlOwogCiAgICAgbV9uZXh0U3RlcCA9ICZTUUxUcmFuc2FjdGlvbjo6Y2xl YW51cEFmdGVyVHJhbnNhY3Rpb25FcnJvckNhbGxiYWNrOwpJbmRleDogQ2hhbmdlTG9nCj09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT0KLS0tIENoYW5nZUxvZwkocmV2aXNpb24gMjc3NzkpCisrKyBDaGFuZ2VMb2cJKHdvcmtp bmcgY29weSkKQEAgLTEsMyArMSwxNSBAQAorMjAwNy0xMS0xMyAgQnJhZHkgRWlkc29uICA8YmVp ZHNvbkBhcHBsZS5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisK KyAgICAgICAgaHR0cDovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTU5NzYgLSBB U1NFUlQvY3Jhc2ggd2hlbiBTUUxUcmFuc2FjdGlvbkNhbGxiYWNrIHRocm93cyBhbiBleGNlcHRp b24KKworICAgICAgICAqIHN0b3JhZ2UvU1FMVHJhbnNhY3Rpb24uY3BwOgorICAgICAgICAoV2Vi Q29yZTo6U1FMVHJhbnNhY3Rpb246OmRlbGl2ZXJUcmFuc2FjdGlvbkNhbGxiYWNrKTogTWFrZSBh IHRyYW5zYWN0aW9uIGVycm9yIGZvciB0aGUgY2FzZSB3aGVyZSB0aGUgCisgICAgICAgICAgU1FM VHJhbnNhY3Rpb25DYWxsYmFjayBmYWlscworICAgICAgICAoV2ViQ29yZTo6U1FMVHJhbnNhY3Rp b246OmRlbGl2ZXJUcmFuc2FjdGlvbkVycm9yQ2FsbGJhY2spOiBEb24ndCBhc3NlcnQgb24gdGhl IGVycm9yIGNhbGxiYWNrLCBidXQgbnVsbCBjaGVjayBpdAorICAgICAgICAgIGFuZCBtYWtlIHRo ZSBjb21taXQvcm9sbGJhY2sgZGVjaXNpb24gYWNjb3JkaW5nbHkKKwogMjAwNy0xMS0xMyAgQW5k ZXJzIENhcmxzc29uICA8YW5kZXJzY2FAYXBwbGUuY29tPgogCiAgICAgICAgIEZpeCBXaW5kb3dz IGJ1aWxkLgo= 17260 2007-11-13 22:37:39 -0800 2007-11-13 22:40:11 -0800 Proposed fix w/ layout test patch.txt text/plain 4077 beidson SW5kZXg6IFdlYkNvcmUvc3RvcmFnZS9TUUxUcmFuc2FjdGlvbi5jcHAKPT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0g V2ViQ29yZS9zdG9yYWdlL1NRTFRyYW5zYWN0aW9uLmNwcAkocmV2aXNpb24gMjc3ODEpCisrKyBX ZWJDb3JlL3N0b3JhZ2UvU1FMVHJhbnNhY3Rpb24uY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0xNDUs OSArMTQ1LDEwIEBACiAgICAgICAgIHNob3VsZERlbGl2ZXJFcnJvckNhbGxiYWNrID0gdHJ1ZTsK IAogICAgIC8vIFRyYW5zYWN0aW9uIFN0ZXAgNSAtIElmIHRoZSB0cmFuc2FjdGlvbiBjYWxsYmFj ayB3YXMgbnVsbCBvciByYWlzZWQgYW4gZXhjZXB0aW9uLCBqdW1wIHRvIHRoZSBlcnJvciBjYWxs YmFjawotICAgIGlmIChzaG91bGREZWxpdmVyRXJyb3JDYWxsYmFjaykKKyAgICBpZiAoc2hvdWxk RGVsaXZlckVycm9yQ2FsbGJhY2spIHsKKyAgICAgICAgbV90cmFuc2FjdGlvbkVycm9yID0gbmV3 IFNRTEVycm9yKDAsICJ0aGUgU1FMVHJhbnNhY3Rpb25DYWxsYmFjayB3YXMgbnVsbCBvciB0aHJl dyBhbiBleGNlcHRpb24iKTsKICAgICAgICAgZGVsaXZlclRyYW5zYWN0aW9uRXJyb3JDYWxsYmFj aygpOwotICAgIGVsc2UKKyAgICB9IGVsc2UKICAgICAgICAgc2NoZWR1bGVUb1J1blN0YXRlbWVu dHMoKTsKIH0KIApAQCAtMjg5LDExICsyOTAsMTEgQEAKIAogdm9pZCBTUUxUcmFuc2FjdGlvbjo6 ZGVsaXZlclRyYW5zYWN0aW9uRXJyb3JDYWxsYmFjaygpCiB7Ci0gICAgQVNTRVJUKG1fZXJyb3JD YWxsYmFjayk7CiAgICAgQVNTRVJUKG1fdHJhbnNhY3Rpb25FcnJvcik7CiAgICAgCiAgICAgLy8g VHJhbnNhY3Rpb24gU3RlcCAxMSAtIElmIHRoZSBjYWxsYmFjayBkaWRuJ3QgcmV0dXJuIGZhbHNl LCB0aGVuIHJvbGxiYWNrIHRoZSB0cmFuc2FjdGlvbi4KLSAgICBpZiAobV9lcnJvckNhbGxiYWNr LT5oYW5kbGVFdmVudChtX3RyYW5zYWN0aW9uRXJyb3IuZ2V0KCkpKQorICAgIC8vIFRoaXMgaW5j bHVkZXMgdGhlIGNhbGxiYWNrIG5vdCBleGlzdGluZywgcmV0dXJuaW5nIHRydWUsIG9yIHRocm93 aW5nIGFuIGV4Y2VwdGlvbgorICAgIGlmICghbV9lcnJvckNhbGxiYWNrIHx8IG1fZXJyb3JDYWxs YmFjay0+aGFuZGxlRXZlbnQobV90cmFuc2FjdGlvbkVycm9yLmdldCgpKSkKICAgICAgICAgbV9z aG91bGRDb21taXRBZnRlckVycm9yQ2FsbGJhY2sgPSBmYWxzZTsKIAogICAgIG1fbmV4dFN0ZXAg PSAmU1FMVHJhbnNhY3Rpb246OmNsZWFudXBBZnRlclRyYW5zYWN0aW9uRXJyb3JDYWxsYmFjazsK SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiAyNzc4MykKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTUgQEAKKzIwMDctMTEtMTMgIEJyYWR5IEVpZHNvbiAgPGJlaWRzb25AYXBwbGUu Y29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIGh0 dHA6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE1OTc2IC0gQVNTRVJUL2NyYXNo IHdoZW4gU1FMVHJhbnNhY3Rpb25DYWxsYmFjayB0aHJvd3MgYW4gZXhjZXB0aW9uCisKKyAgICAg ICAgKiBzdG9yYWdlL1NRTFRyYW5zYWN0aW9uLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OlNRTFRy YW5zYWN0aW9uOjpkZWxpdmVyVHJhbnNhY3Rpb25DYWxsYmFjayk6IE1ha2UgYSB0cmFuc2FjdGlv biBlcnJvciBmb3IgdGhlIGNhc2Ugd2hlcmUgdGhlIAorICAgICAgICAgIFNRTFRyYW5zYWN0aW9u Q2FsbGJhY2sgZmFpbHMKKyAgICAgICAgKFdlYkNvcmU6OlNRTFRyYW5zYWN0aW9uOjpkZWxpdmVy VHJhbnNhY3Rpb25FcnJvckNhbGxiYWNrKTogRG9uJ3QgYXNzZXJ0IG9uIHRoZSBlcnJvciBjYWxs YmFjaywgYnV0IG51bGwgY2hlY2sgaXQKKyAgICAgICAgICBhbmQgbWFrZSB0aGUgY29tbWl0L3Jv bGxiYWNrIGRlY2lzaW9uIGFjY29yZGluZ2x5CisKIDIwMDctMTEtMTMgIE9saXZlciBIdW50ICA8 b2xpdmVyQGFwcGxlLmNvbT4KIAogICAgICAgICBSZXZpZXdlZCBieSBBbmRlcnMuCkluZGV4OiBM YXlvdXRUZXN0cy9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gTGF5b3V0VGVzdHMvQ2hhbmdlTG9n CShyZXZpc2lvbiAyNzc4MykKKysrIExheW91dFRlc3RzL0NoYW5nZUxvZwkod29ya2luZyBjb3B5 KQpAQCAtMSwzICsxLDEzIEBACisyMDA3LTExLTEzICBCcmFkeSBFaWRzb24gIDxiZWlkc29uQGFw cGxlLmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAg ICBGaXggZm9yIGh0dHA6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE1OTc2CisK KyAgICAgICAgKiBzdG9yYWdlOiBBZGRlZCAtIFRoZXJlIHdpbGwgc29vbiBiZSBhbiBlbnRpcmUg c3VpdGUgaW4gaGVyZSEKKyAgICAgICAgKiBzdG9yYWdlL3RyYW5zYWN0aW9uX2NhbGxiYWNrX2V4 Y2VwdGlvbl9jcmFzaC1leHBlY3RlZC50eHQ6IEFkZGVkLgorICAgICAgICAqIHN0b3JhZ2UvdHJh bnNhY3Rpb25fY2FsbGJhY2tfZXhjZXB0aW9uX2NyYXNoLmh0bWw6IEFkZGVkLgorCiAyMDA3LTEx LTEzICBBZGFtIFJvYmVuICA8YXJvYmVuQGFwcGxlLmNvbT4KIAogICAgICAgICBBZGQgdGhlIGh0 dHAvdGVzdHMvbWVkaWEgZGlyZWN0b3J5IHRvIHRoZSBXaW5kb3dzIFNraXBwZWQgZmlsZQpJbmRl eDogTGF5b3V0VGVzdHMvc3RvcmFnZS90cmFuc2FjdGlvbl9jYWxsYmFja19leGNlcHRpb25fY3Jh c2guaHRtbAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09Ci0tLSBMYXlvdXRUZXN0cy9zdG9yYWdlL3RyYW5zYWN0aW9uX2Nh bGxiYWNrX2V4Y2VwdGlvbl9jcmFzaC5odG1sCShyZXZpc2lvbiAwKQorKysgTGF5b3V0VGVzdHMv c3RvcmFnZS90cmFuc2FjdGlvbl9jYWxsYmFja19leGNlcHRpb25fY3Jhc2guaHRtbAkocmV2aXNp b24gMCkKQEAgLTAsMCArMSwyMiBAQAorPGh0bWw+Cis8aGVhZD4KKzxzY3JpcHQ+CisKK2lmICh3 aW5kb3cubGF5b3V0VGVzdENvbnRyb2xsZXIpIHsKKyAgICBsYXlvdXRUZXN0Q29udHJvbGxlci5k dW1wQXNUZXh0KCk7CisgICAgbGF5b3V0VGVzdENvbnRyb2xsZXIud2FpdFVudGlsRG9uZSgpOwor fQorCit2YXIgZGIgPSBvcGVuRGF0YWJhc2UoIjE1OTc2VGVzdCIsICIxLjAiLCAiVGVzdCBmb3Ig aHR0cDovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTU5NzYiLCAxKTsKK2RiLnRy YW5zYWN0aW9uKGZ1bmN0aW9uKHR4KSB7IAorICAgIGlmICh3aW5kb3cubGF5b3V0VGVzdENvbnRy b2xsZXIpCisgICAgICAgIHNldFRpbWVvdXQoImxheW91dFRlc3RDb250cm9sbGVyLm5vdGlmeURv bmUoKSIsIDApOworICAgIHRocm93IDA7Cit9KTsKKworPC9zY3JpcHQ+Cis8L2hlYWQ+Cis8Ym9k eT4KK0lmIFdlYktpdCBkb2Vzbid0IGNyYXNoLCB0aGlzIHRlc3QgaGFzIHBhc3NlZAorPC9ib2R5 PgorPC9odG1sPgpJbmRleDogTGF5b3V0VGVzdHMvc3RvcmFnZS90cmFuc2FjdGlvbl9jYWxsYmFj a19leGNlcHRpb25fY3Jhc2gtZXhwZWN0ZWQudHh0Cj09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIExheW91dFRlc3Rz L3N0b3JhZ2UvdHJhbnNhY3Rpb25fY2FsbGJhY2tfZXhjZXB0aW9uX2NyYXNoLWV4cGVjdGVkLnR4 dAkocmV2aXNpb24gMCkKKysrIExheW91dFRlc3RzL3N0b3JhZ2UvdHJhbnNhY3Rpb25fY2FsbGJh Y2tfZXhjZXB0aW9uX2NyYXNoLWV4cGVjdGVkLnR4dAkocmV2aXNpb24gMCkKQEAgLTAsMCArMSwy IEBACitDT05TT0xFIE1FU1NBR0U6IGxpbmUgMDogdW5kZWZpbmVkCitJZiBXZWJLaXQgZG9lc24n dCBjcmFzaCwgdGhpcyB0ZXN0IGhhcyBwYXNzZWQK