110019 2013-02-16 08:52:07 -0800 REGRESSION(r143076): Crash when calling removeNamedItem or removeNamedItemNS with a non-existent attribute of newly created element 2013-02-16 15:49:06 -0800 1 1 1 Unclassified WebKit DOM 528+ (Nightly build) All All RESOLVED FIXED Regression P1 Normal --- 0 peter webkit-unassigned bfulgham cmarcelo kling ojan.autocc peter webkit.review.bot oldest_to_newest 834732 0 188713 peter 2013-02-16 08:52:07 -0800 Created attachment 188713 HTML document (based on the Acid3 test) that demonstrates the issue Currently WebCore crashes when either removeNamedItem or removeNamedItemNS are called with a non-existent attribute of a newly created element. This bug causes a crash in test 67 of Acid3. Attached is a small HTML document (based on the Acid3 test) that demonstrates the issue. Tested with latest nightly on both Windows and OSX. 834761 1 188720 peter 2013-02-16 10:45:57 -0800 Created attachment 188720 Patch 834768 2 188720 bfulgham 2013-02-16 11:44:33 -0800 Comment on attachment 188720 Patch This seems like a reasonable check. I'm surprised it wasn't needed previously. Is it possible that some external logic is supposed to be preventing a call into this routine if the mode has no attributed? 834769 3 peter 2013-02-16 11:55:12 -0800 (In reply to comment #2) It may have been changeset 143076 (https://trac.webkit.org/changeset/143076) that broke it -- ElementData is no longer implicitly created when .attributes is referenced. 834775 4 188720 kling 2013-02-16 12:46:37 -0800 Comment on attachment 188720 Patch Good catch! Looks like our NamedNodeMap test coverage isn't what it should be. :/ Let's improve that by adding a layout test with this patch. 834807 5 188720 kling 2013-02-16 13:36:25 -0800 Comment on attachment 188720 Patch I didn't realize we already have a copy of ACID3 in-tree that catches these bugs. r=me as we don't need a new test and this looks good. Thanks! 834855 6 bfulgham 2013-02-16 15:42:48 -0800 (In reply to comment #5) > (From update of attachment 188720 [details]) > I didn't realize we already have a copy of ACID3 in-tree that catches these bugs. > r=me as we don't need a new test and this looks good. Thanks! If we have a test in the tree that covers this logic, why didn't we see it before Peter did? Could we indicate which test covers this behavior in the ChangeLog? 834857 7 kling 2013-02-16 15:45:37 -0800 (In reply to comment #6) > (In reply to comment #5) > > (From update of attachment 188720 [details] [details]) > > I didn't realize we already have a copy of ACID3 in-tree that catches these bugs. > > r=me as we don't need a new test and this looks good. Thanks! > > If we have a test in the tree that covers this logic, why didn't we see it before Peter did? Could we indicate which test covers this behavior in the ChangeLog? http/tests/misc/acid3.html is crashing on the bots ATM. I'll land this patch and add a comment to the ChangeLog about it. 834862 8 kling 2013-02-16 15:49:06 -0800 Committed r143115: <http://trac.webkit.org/changeset/143115> 188713 2013-02-16 08:52:07 -0800 2013-02-16 08:58:58 -0800 HTML document (based on the Acid3 test) that demonstrates the issue test.html text/plain 771 peter PCFET0NUWVBFIGh0bWw+DQo8aHRtbD4NCjxoZWFkPjwvaGVhZD4NCjxib2R5Pg0KPHNjcmlwdD4N CmZ1bmN0aW9uIGFzc2VydChjb25kaXRpb24sIG1lc3NhZ2UpIHsNCiAgaWYgKCFjb25kaXRpb24p DQogICAgZG9jdW1lbnQud3JpdGUobWVzc2FnZSArICc8YnI+Jyk7DQp9DQpmdW5jdGlvbiB0ZXN0 IChmKSB7DQogIHZhciBtc2cgPSAnd3JvbmcgZXhjZXB0aW9uIHJhaXNlZCc7DQogIHRyeSB7DQog ICAgZigpOw0KICAgIG1zZyA9ICdubyBleGNlcHRpb24gcmFpc2VkJzsNCiAgfSBjYXRjaCAoZSkg ew0KICAgIGlmICgnY29kZScgaW4gZSkgew0KICAgICAgaWYgKGUuY29kZSA9PSA4KQ0KICAgICAg ICBtc2cgPSAnJzsNCiAgICAgIGVsc2UNCiAgICAgICAgbXNnICs9ICc7IGNvZGUgPSAnICsgZS5j b2RlOw0KICAgIH0NCiAgfQ0KICBhc3NlcnQobXNnID09ICcnLCAid2hlbiBjYWxsaW5nIHJlbW92 ZU5hbWVkSXRlbU5TIGluIGEgbm9uIGV4aXN0ZW50IGF0dHJpYnV0ZTogIiArIG1zZyk7DQogIHJl dHVybiA1Ow0KfQ0KKGZ1bmN0aW9uICgpIHsNCiAgdmFyIHAgPSBkb2N1bWVudC5jcmVhdGVFbGVt ZW50KCJwIik7DQogIHRlc3QoZnVuY3Rpb24oKSB7cC5hdHRyaWJ1dGVzLnJlbW92ZU5hbWVkSXRl bSgiYWJzZW50Iik7fSk7DQogIHRlc3QoZnVuY3Rpb24oKSB7cC5hdHRyaWJ1dGVzLnJlbW92ZU5h bWVkSXRlbU5TKCJodHRwOi8vd3d3LmV4YW1wbGUuY29tLyIsICJhYnNlbnQiKTt9KQ0KfSkoKTsN Cjwvc2NyaXB0Pg0KPC9ib2R5Pg0KPC9odG1sPg0K 188720 2013-02-16 10:45:57 -0800 2013-02-16 15:45:07 -0800 Patch bug-110019-20130216184218.patch text/plain 2028 peter SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDE0MzEwNikKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE2IEBACisyMDEzLTAyLTE2ICBQZXRlciBO ZWxzb24gIDxwZXRlckBwZXRlcmRuLmNvbT4KKworICAgICAgICBDcmFzaCB3aGVuIGNhbGxpbmcg cmVtb3ZlTmFtZWRJdGVtIG9yIHJlbW92ZU5hbWVkSXRlbU5TIHdpdGggYSBub24tZXhpc3RlbnQg YXR0cmlidXRlIG9mIG5ld2x5IGNyZWF0ZWQgZWxlbWVudAorICAgICAgICBodHRwczovL2J1Z3Mu d2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTEwMDE5CisKKyAgICAgICAgUmV2aWV3ZWQgYnkg Tk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgTm93IGNoZWNrcyBFbGVtZW50OjpoYXNBdHRyaWJ1 dGVzKCkgYmVmb3JlIGNhbGxpbmcgRWxlbWVudDo6Z2V0QXR0cmlidXRlSXRlbUluZGV4KCkuCisK KyAgICAgICAgKiBkb20vTmFtZWROb2RlTWFwLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6Ok5hbWVk Tm9kZU1hcDo6cmVtb3ZlTmFtZWRJdGVtKToKKyAgICAgICAgKFdlYkNvcmU6Ok5hbWVkTm9kZU1h cDo6cmVtb3ZlTmFtZWRJdGVtTlMpOgorCiAyMDEzLTAyLTE2ICBTaGVyaWZmIEJvdCAgPHdlYmtp dC5yZXZpZXcuYm90QGdtYWlsLmNvbT4KIAogICAgICAgICBVbnJldmlld2VkLCByb2xsaW5nIG91 dCByMTQyNzM0LgpJbmRleDogU291cmNlL1dlYkNvcmUvZG9tL05hbWVkTm9kZU1hcC5jcHAKPT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvZG9tL05hbWVkTm9kZU1hcC5jcHAJKHJldmlzaW9u IDE0MzEwMikKKysrIFNvdXJjZS9XZWJDb3JlL2RvbS9OYW1lZE5vZGVNYXAuY3BwCSh3b3JraW5n IGNvcHkpCkBAIC02Miw3ICs2Miw3IEBAIFBhc3NSZWZQdHI8Tm9kZT4gTmFtZWROb2RlTWFwOjpn ZXROYW1lZEkKIAogUGFzc1JlZlB0cjxOb2RlPiBOYW1lZE5vZGVNYXA6OnJlbW92ZU5hbWVkSXRl bShjb25zdCBBdG9taWNTdHJpbmcmIG5hbWUsIEV4Y2VwdGlvbkNvZGUmIGVjKQogewotICAgIHNp emVfdCBpbmRleCA9IG1fZWxlbWVudC0+Z2V0QXR0cmlidXRlSXRlbUluZGV4KG5hbWUsIHNob3Vs ZElnbm9yZUF0dHJpYnV0ZUNhc2UobV9lbGVtZW50KSk7CisgICAgc2l6ZV90IGluZGV4ID0gbV9l bGVtZW50LT5oYXNBdHRyaWJ1dGVzKCkgPyBtX2VsZW1lbnQtPmdldEF0dHJpYnV0ZUl0ZW1JbmRl eChuYW1lLCBzaG91bGRJZ25vcmVBdHRyaWJ1dGVDYXNlKG1fZWxlbWVudCkpIDogbm90Rm91bmQ7 CiAgICAgaWYgKGluZGV4ID09IG5vdEZvdW5kKSB7CiAgICAgICAgIGVjID0gTk9UX0ZPVU5EX0VS UjsKICAgICAgICAgcmV0dXJuIDA7CkBAIC03Miw3ICs3Miw3IEBAIFBhc3NSZWZQdHI8Tm9kZT4g TmFtZWROb2RlTWFwOjpyZW1vdmVOYW0KIAogUGFzc1JlZlB0cjxOb2RlPiBOYW1lZE5vZGVNYXA6 OnJlbW92ZU5hbWVkSXRlbU5TKGNvbnN0IEF0b21pY1N0cmluZyYgbmFtZXNwYWNlVVJJLCBjb25z dCBBdG9taWNTdHJpbmcmIGxvY2FsTmFtZSwgRXhjZXB0aW9uQ29kZSYgZWMpCiB7Ci0gICAgc2l6 ZV90IGluZGV4ID0gbV9lbGVtZW50LT5nZXRBdHRyaWJ1dGVJdGVtSW5kZXgoUXVhbGlmaWVkTmFt ZShudWxsQXRvbSwgbG9jYWxOYW1lLCBuYW1lc3BhY2VVUkkpKTsKKyAgICBzaXplX3QgaW5kZXgg PSBtX2VsZW1lbnQtPmhhc0F0dHJpYnV0ZXMoKSA/IG1fZWxlbWVudC0+Z2V0QXR0cmlidXRlSXRl bUluZGV4KFF1YWxpZmllZE5hbWUobnVsbEF0b20sIGxvY2FsTmFtZSwgbmFtZXNwYWNlVVJJKSkg OiBub3RGb3VuZDsKICAgICBpZiAoaW5kZXggPT0gbm90Rm91bmQpIHsKICAgICAgICAgZWMgPSBO T1RfRk9VTkRfRVJSOwogICAgICAgICByZXR1cm4gMDsK