24636 2009-03-16 17:52:09 -0700 Fix crash of Chromium port in use of BackForwardList in SVG images 2010-02-24 17:38:06 -0800 1 1 1 Unclassified WebKit Platform 528+ (Nightly build) PC Windows XP RESOLVED DUPLICATE 24398 P2 Normal --- 0 hclam webkit-unassigned dglazkov maruel oldest_to_newest 113964 0 hclam 2009-03-16 17:52:09 -0700 Crash filed against Chromium: http://code.google.com/p/chromium/issues/detail?id=6869 When a SVG image is created, a WebCore::Page is created with BackForwardList but BackForwardList::m_client is never initialized. During eviction of the cached SVG image, m_client->close() is called in BackForwardList::close() resulting in a crash. Since the WebCore::Page in SVGImage doesn't have links to the frame, there's no BackForwardListClient implementation available, so we just don't care these requests to BackForwardList and don't delegate the requests when m_client is NULL. 113965 1 28673 hclam 2009-03-16 17:52:54 -0700 Created attachment 28673 patch 114206 2 hclam 2009-03-18 11:15:13 -0700 Review in Chromium: http://codereview.chromium.org/42265/show 114332 3 fishd 2009-03-19 10:29:44 -0700 maruel already fixed this: http://trac.webkit.org/changeset/41824 114333 4 hclam 2009-03-19 10:32:45 -0700 "if (m_client)" is only performed on close(), this check should be added to other functions as well. 114337 5 28755 hclam 2009-03-19 10:53:35 -0700 Created attachment 28755 adding if (m_client) check to other methods 114338 6 28755 hclam 2009-03-19 10:56:16 -0700 Comment on attachment 28755 adding if (m_client) check to other methods Since maruel has patched the class and initialize m_client as NULL, we should avoid using m_client in this case in all methods too. 114340 7 fishd 2009-03-19 11:11:38 -0700 This was discussed. Please see: https://bugs.webkit.org/show_bug.cgi?id=24398#c6 115362 8 28755 eric 2009-03-26 11:32:04 -0700 Comment on attachment 28755 adding if (m_client) check to other methods I would have just added an ASSERT(m_client) before these calls in all cases: - return m_client->backListCount(); + if (m_client) + return m_client->backListCount(); + ASSERT_NOT_REACHED(); + return 0; No need to work hard to crash only in debug mode. The changelog should mention the bug url. Also is there no way to test this? There should be a layout test if at all possible. 193577 9 hclam 2010-02-24 17:38:06 -0800 *** This bug has been marked as a duplicate of bug 24398 *** 28673 2009-03-16 17:52:54 -0700 2009-03-19 10:53:35 -0700 patch webkit-patch text/plain 2806 hclam SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiAxMTcxOSkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMjAgQEAKKzIwMDktMDMtMTYgIEFscGhhIExhbSAgPGhjbGFtQGNocm9taXVtLm9y Zz4KKworICAgICAgICBSZXZpZXcgYnkgPworCisgICAgICAgIEJ1ZyBmaXggZm9yIHBsYXRmb3Jt IENocm9taXVtIGZvciB1c2Ugb2YgQmFja0ZvcndhcmRMaXN0IGluCisgICAgICAgIFNWRyBpbWFn ZXMuIEluaXRpYWxpemUgQmFja0ZvcndhcmRMaXN0OjptX2NsaWVudCBhcyBOVUxMIGluCisgICAg ICAgIGN0b3IuIEF2b2lkIHVzaW5nIG1fY2xpZW50IGlmIGl0J3MgTlVMTC4KKworICAgICAgICAq IGhpc3RvcnkvQmFja0ZvcndhcmRMaXN0Q2hyb21pdW0uY3BwOgorICAgICAgICAoV2ViQ29yZTo6 QmFja0ZvcndhcmRMaXN0OjpCYWNrRm9yd2FyZExpc3QpOgorICAgICAgICAoV2ViQ29yZTo6QmFj a0ZvcndhcmRMaXN0Ojpnb1RvSXRlbSk6CisgICAgICAgIChXZWJDb3JlOjpCYWNrRm9yd2FyZExp c3Q6OmJhY2tJdGVtKToKKyAgICAgICAgKFdlYkNvcmU6OkJhY2tGb3J3YXJkTGlzdDo6YmFja0xp c3RDb3VudCk6CisgICAgICAgIChXZWJDb3JlOjpCYWNrRm9yd2FyZExpc3Q6OmZvcndhcmRMaXN0 Q291bnQpOgorICAgICAgICAoV2ViQ29yZTo6QmFja0ZvcndhcmRMaXN0OjppdGVtQXRJbmRleCk6 CisgICAgICAgIChXZWJDb3JlOjpCYWNrRm9yd2FyZExpc3Q6OmNsb3NlKToKKwogMjAwOS0wMy0x NCAgR3JlZyBCb2xzaW5nYSAgPGJvbHNpbmdhQGFwcGxlLmNvbT4KIAogICAgICAgICA8cmRhcjov L3Byb2JsZW0vNjY4MzQ2NT4KSW5kZXg6IFdlYkNvcmUvaGlzdG9yeS9CYWNrRm9yd2FyZExpc3RD aHJvbWl1bS5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PQotLS0gV2ViQ29yZS9oaXN0b3J5L0JhY2tGb3J3YXJkTGlz dENocm9taXVtLmNwcAkocmV2aXNpb24gMTE3MTkpCisrKyBXZWJDb3JlL2hpc3RvcnkvQmFja0Zv cndhcmRMaXN0Q2hyb21pdW0uY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0zOCw2ICszOCw3IEBACiAK IEJhY2tGb3J3YXJkTGlzdDo6QmFja0ZvcndhcmRMaXN0KFBhZ2UqIHBhZ2UpCiAgICAgOiBtX3Bh Z2UocGFnZSkKKyAgICAsIG1fY2xpZW50KDApCiAgICAgLCBtX2NhcGFjaXR5KERlZmF1bHRDYXBh Y2l0eSkKICAgICAsIG1fY2xvc2VkKHRydWUpCiAgICAgLCBtX2VuYWJsZWQodHJ1ZSkKQEAgLTU1 LDEyICs1NiwxNCBAQAogICAgIGlmIChtX2NhcGFjaXR5ID09IDAgfHwgIW1fZW5hYmxlZCkKICAg ICAgICAgcmV0dXJuOwogIAotICAgIG1fY2xpZW50LT5hZGRJdGVtKHBycEl0ZW0pOworICAgIGlm IChtX2NsaWVudCkKKyAgICAgICAgbV9jbGllbnQtPmFkZEl0ZW0ocHJwSXRlbSk7CiB9CiAKIHZv aWQgQmFja0ZvcndhcmRMaXN0Ojpnb1RvSXRlbShIaXN0b3J5SXRlbSogaXRlbSkKIHsKLSAgICBt X2NsaWVudC0+Z29Ub0l0ZW0oaXRlbSk7CisgICAgaWYgKG1fY2xpZW50KQorICAgICAgICBtX2Ns aWVudC0+Z29Ub0l0ZW0oaXRlbSk7CiB9CiAKIEhpc3RvcnlJdGVtKiBCYWNrRm9yd2FyZExpc3Q6 OmJhY2tJdGVtKCkKQEAgLTc3LDcgKzgwLDEwIEBACiAKIEhpc3RvcnlJdGVtKiBCYWNrRm9yd2Fy ZExpc3Q6OmN1cnJlbnRJdGVtKCkKIHsKLSAgICByZXR1cm4gbV9jbGllbnQtPmN1cnJlbnRJdGVt KCk7CisgICAgaWYgKG1fY2xpZW50KQorICAgICAgICByZXR1cm4gbV9jbGllbnQtPmN1cnJlbnRJ dGVtKCk7CisgICAgQVNTRVJUX05PVF9SRUFDSEVEKCk7CisgICAgcmV0dXJuIDA7CiB9CiAKIGlu dCBCYWNrRm9yd2FyZExpc3Q6OmNhcGFjaXR5KCkKQEAgLTEwNywxNyArMTEzLDI2IEBACiAKIGlu dCBCYWNrRm9yd2FyZExpc3Q6OmJhY2tMaXN0Q291bnQoKQogewotICAgIHJldHVybiBtX2NsaWVu dC0+YmFja0xpc3RDb3VudCgpOworICAgIGlmIChtX2NsaWVudCkKKyAgICAgICAgcmV0dXJuIG1f Y2xpZW50LT5iYWNrTGlzdENvdW50KCk7CisgICAgQVNTRVJUX05PVF9SRUFDSEVEKCk7CisgICAg cmV0dXJuIDA7CiB9CiAKIGludCBCYWNrRm9yd2FyZExpc3Q6OmZvcndhcmRMaXN0Q291bnQoKQog ewotICAgIHJldHVybiBtX2NsaWVudC0+Zm9yd2FyZExpc3RDb3VudCgpOworICAgIGlmIChtX2Ns aWVudCkKKyAgICAgICAgcmV0dXJuIG1fY2xpZW50LT5mb3J3YXJkTGlzdENvdW50KCk7CisgICAg QVNTRVJUX05PVF9SRUFDSEVEKCk7CisgICAgcmV0dXJuIDA7CiB9CiAKIEhpc3RvcnlJdGVtKiBC YWNrRm9yd2FyZExpc3Q6Oml0ZW1BdEluZGV4KGludCBpbmRleCkKIHsKLSAgICByZXR1cm4gbV9j bGllbnQtPml0ZW1BdEluZGV4KGluZGV4KTsKKyAgICBpZiAobV9jbGllbnQpCisgICAgICAgIHJl dHVybiBtX2NsaWVudC0+aXRlbUF0SW5kZXgoaW5kZXgpOworICAgIEFTU0VSVF9OT1RfUkVBQ0hF RCgpOworICAgIHJldHVybiAwOwogfQogCiBIaXN0b3J5SXRlbVZlY3RvciYgQmFja0ZvcndhcmRM aXN0OjplbnRyaWVzKCkKQEAgLTEyOCw3ICsxNDMsOCBAQAogCiB2b2lkIEJhY2tGb3J3YXJkTGlz dDo6Y2xvc2UoKQogewotICAgIG1fY2xpZW50LT5jbG9zZSgpOworICAgIGlmIChtX2NsaWVudCkK KyAgICAgICAgbV9jbGllbnQtPmNsb3NlKCk7CiAgICAgbV9wYWdlID0gMDsKICAgICBtX2Nsb3Nl ZCA9IHRydWU7CiB9Cg== 28755 2009-03-19 10:53:35 -0700 2009-03-26 11:32:04 -0700 adding if (m_client) check to other methods webkit-patch text/plain 2291 hclam SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiAxMTcxOSkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMjAgQEAKKzIwMDktMDMtMTYgIEFscGhhIExhbSAgPGhjbGFtQGNocm9taXVtLm9y Zz4KKworICAgICAgICBSZXZpZXcgYnkgPworCisgICAgICAgIEJ1ZyBmaXggZm9yIHBsYXRmb3Jt IENocm9taXVtIGZvciB1c2Ugb2YgQmFja0ZvcndhcmRMaXN0IGluCisgICAgICAgIFNWRyBpbWFn ZXMuIEF2b2lkIHVzaW5nIG1fY2xpZW50IGlmIGl0J3MgTlVMTC4KKworICAgICAgICAqIGhpc3Rv cnkvQmFja0ZvcndhcmRMaXN0Q2hyb21pdW0uY3BwOgorICAgICAgICAoV2ViQ29yZTo6QmFja0Zv cndhcmRMaXN0Ojpnb1RvSXRlbSk6CisgICAgICAgIChXZWJDb3JlOjpCYWNrRm9yd2FyZExpc3Q6 OmJhY2tJdGVtKToKKyAgICAgICAgKFdlYkNvcmU6OkJhY2tGb3J3YXJkTGlzdDo6YmFja0xpc3RD b3VudCk6CisgICAgICAgIChXZWJDb3JlOjpCYWNrRm9yd2FyZExpc3Q6OmZvcndhcmRMaXN0Q291 bnQpOgorICAgICAgICAoV2ViQ29yZTo6QmFja0ZvcndhcmRMaXN0OjppdGVtQXRJbmRleCk6CisK IDIwMDktMDMtMTQgIEdyZWcgQm9sc2luZ2EgIDxib2xzaW5nYUBhcHBsZS5jb20+CiAKICAgICAg ICAgPHJkYXI6Ly9wcm9ibGVtLzY2ODM0NjU+CkluZGV4OiBXZWJDb3JlL2hpc3RvcnkvQmFja0Zv cndhcmRMaXN0Q2hyb21pdW0uY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvaGlzdG9yeS9CYWNr Rm9yd2FyZExpc3RDaHJvbWl1bS5jcHAJKHJldmlzaW9uIDExNzE5KQorKysgV2ViQ29yZS9oaXN0 b3J5L0JhY2tGb3J3YXJkTGlzdENocm9taXVtLmNwcAkod29ya2luZyBjb3B5KQpAQCAtNTUsMTIg KzU2LDE0IEBACiAgICAgaWYgKG1fY2FwYWNpdHkgPT0gMCB8fCAhbV9lbmFibGVkKQogICAgICAg ICByZXR1cm47CiAgCi0gICAgbV9jbGllbnQtPmFkZEl0ZW0ocHJwSXRlbSk7CisgICAgaWYgKG1f Y2xpZW50KQorICAgICAgICBtX2NsaWVudC0+YWRkSXRlbShwcnBJdGVtKTsKIH0KIAogdm9pZCBC YWNrRm9yd2FyZExpc3Q6OmdvVG9JdGVtKEhpc3RvcnlJdGVtKiBpdGVtKQogewotICAgIG1fY2xp ZW50LT5nb1RvSXRlbShpdGVtKTsKKyAgICBpZiAobV9jbGllbnQpCisgICAgICAgIG1fY2xpZW50 LT5nb1RvSXRlbShpdGVtKTsKIH0KIAogSGlzdG9yeUl0ZW0qIEJhY2tGb3J3YXJkTGlzdDo6YmFj a0l0ZW0oKQpAQCAtNzcsNyArODAsMTAgQEAKIAogSGlzdG9yeUl0ZW0qIEJhY2tGb3J3YXJkTGlz dDo6Y3VycmVudEl0ZW0oKQogewotICAgIHJldHVybiBtX2NsaWVudC0+Y3VycmVudEl0ZW0oKTsK KyAgICBpZiAobV9jbGllbnQpCisgICAgICAgIHJldHVybiBtX2NsaWVudC0+Y3VycmVudEl0ZW0o KTsKKyAgICBBU1NFUlRfTk9UX1JFQUNIRUQoKTsKKyAgICByZXR1cm4gMDsKIH0KIAogaW50IEJh Y2tGb3J3YXJkTGlzdDo6Y2FwYWNpdHkoKQpAQCAtMTA3LDE3ICsxMTMsMjYgQEAKIAogaW50IEJh Y2tGb3J3YXJkTGlzdDo6YmFja0xpc3RDb3VudCgpCiB7Ci0gICAgcmV0dXJuIG1fY2xpZW50LT5i YWNrTGlzdENvdW50KCk7CisgICAgaWYgKG1fY2xpZW50KQorICAgICAgICByZXR1cm4gbV9jbGll bnQtPmJhY2tMaXN0Q291bnQoKTsKKyAgICBBU1NFUlRfTk9UX1JFQUNIRUQoKTsKKyAgICByZXR1 cm4gMDsKIH0KIAogaW50IEJhY2tGb3J3YXJkTGlzdDo6Zm9yd2FyZExpc3RDb3VudCgpCiB7Ci0g ICAgcmV0dXJuIG1fY2xpZW50LT5mb3J3YXJkTGlzdENvdW50KCk7CisgICAgaWYgKG1fY2xpZW50 KQorICAgICAgICByZXR1cm4gbV9jbGllbnQtPmZvcndhcmRMaXN0Q291bnQoKTsKKyAgICBBU1NF UlRfTk9UX1JFQUNIRUQoKTsKKyAgICByZXR1cm4gMDsKIH0KIAogSGlzdG9yeUl0ZW0qIEJhY2tG b3J3YXJkTGlzdDo6aXRlbUF0SW5kZXgoaW50IGluZGV4KQogewotICAgIHJldHVybiBtX2NsaWVu dC0+aXRlbUF0SW5kZXgoaW5kZXgpOworICAgIGlmIChtX2NsaWVudCkKKyAgICAgICAgcmV0dXJu IG1fY2xpZW50LT5pdGVtQXRJbmRleChpbmRleCk7CisgICAgQVNTRVJUX05PVF9SRUFDSEVEKCk7 CisgICAgcmV0dXJuIDA7CiB9CiAKIEhpc3RvcnlJdGVtVmVjdG9yJiBCYWNrRm9yd2FyZExpc3Q6 OmVudHJpZXMoKQo=