39389 2010-05-19 16:40:25 -0700 REGRESSION(57081): Renderer crash in WebCore::HTMLElement::isContentEditable() const 2010-05-20 15:55:24 -0700 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) PC OS X 10.5 RESOLVED FIXED InRadar P1 Normal --- 1 ojan webkit-unassigned enrica hyatt jamesr mitz morrita simon.fraser oldest_to_newest 228052 0 56530 ojan 2010-05-19 16:40:25 -0700 Created attachment 56530 test case This is a very frequent crash see on youtube and YUI's gridbuilder (hit the "show code" button on http://developer.yahoo.com/yui/grids/builder/). 228055 1 ojan 2010-05-19 16:43:15 -0700 The issue seems to be in HTMLElement::isContentEditable. renderer() points to something sane before document()->updateStyleIfNeeded() and to garbage after. 228060 2 jamesr 2010-05-19 16:46:56 -0700 Stack (from a ToT WebKit debug build on Snow Leopard): Program received signal EXC_BAD_ACCESS, Could not access memory. Reason: KERN_INVALID_ADDRESS at address: 0xfffffffe0000600d 0x0000000101a40e20 in WTF::RefPtr<WebCore::RenderStyle>::get (this=0xfffffffe0000600d) at RefPtr.h:60 60 T* get() const { return m_ptr; } (gdb) bt #0 0x0000000101a40e20 in WTF::RefPtr<WebCore::RenderStyle>::get (this=0xfffffffe0000600d) at RefPtr.h:60 #1 0x0000000101051c7d in WebCore::RenderObject::style (this=0xfffffffe00006005) at RenderObject.h:598 #2 0x000000010136071e in WebCore::HTMLElement::isContentEditable (this=0x11723a960) at /usr/local/home/jamesr/WebKit/WebCore/html/HTMLElement.cpp:634 #3 0x00000001017283d4 in WebCore::Node::isContentEditable (this=0x117214520) at /usr/local/home/jamesr/WebKit/WebCore/dom/Node.cpp:643 #4 0x0000000101727ba1 in WebCore::Node::rootEditableElement (this=0x117214520) at /usr/local/home/jamesr/WebKit/WebCore/dom/Node.cpp:1520 #5 0x0000000101359423 in WebCore::editableRootForPosition (p=@0x7fff5fbfea80) at /usr/local/home/jamesr/WebKit/WebCore/editing/htmlediting.cpp:214 #6 0x0000000101a8aee5 in WebCore::VisibleSelection::rootEditableElement (this=0x10701dfd0) at /usr/local/home/jamesr/WebKit/WebCore/editing/VisibleSelection.cpp:569 #7 0x000000010127fd73 in WebCore::SelectionController::rootEditableElement (this=0x10701dfc0) at SelectionController.h:51 #8 0x00000001012c3b20 in WebCore::Frame::notifyRendererOfSelectionChange (this=0x10701da00, userTriggered=true) at /usr/local/home/jamesr/WebKit/WebCore/page/Frame.cpp:535 #9 0x0000000101278c90 in WebCore::EventHandler::handleMouseReleaseEvent (this=0x10701e140, event=@0x7fff5fbfec70) at /usr/local/home/jamesr/WebKit/WebCore/page/EventHandler.cpp:703 #10 0x00000001012790c7 in WebCore::EventHandler::handleMouseReleaseEvent (this=0x10701e140, mouseEvent=@0x7fff5fbfed60) at /usr/local/home/jamesr/WebKit/WebCore/page/EventHandler.cpp:1566 #11 0x00000001012813f5 in WebCore::EventHandler::mouseUp (this=0x10701e140, event=0x1172a1fd0) at /usr/local/home/jamesr/WebKit/WebCore/page/mac/EventHandlerMac.mm:530 #12 0x00000001003857e8 in -[WebHTMLView mouseUp:] (self=0x1172080b0, _cmd=0x7fff8743c914, event=0x1172a1fd0) at /usr/local/home/jamesr/WebKit/WebKit/mac/WebView/WebHTMLView.mm:3713 #13 0x00007fff86e36fc9 in -[NSWindow sendEvent:] () #14 0x0000000100048de3 in ?? () #15 0x00007fff86d6c676 in -[NSApplication sendEvent:] () #16 0x00000001000318dc in ?? () #17 0x00007fff86d030be in -[NSApplication run] () #18 0x00007fff86cfbd8c in NSApplicationMain () #19 0x00000001000016f4 in ?? () Current language: auto; currently c++ 228083 3 jamesr 2010-05-19 17:04:11 -0700 +cc some folks with knowledge of this area. It seems that the basic problem here is that the editing code is calling deep into a subtree that gets detached by the Document::updateStyleIfNeeded() call. I think the fix is to move the updateStyle call further down the callstack, especially as some bits down there (like htmlediting.cpp's editableRootForPosition() queries information might be affected by style. 228138 4 56546 jamesr 2010-05-19 18:39:34 -0700 Created attachment 56546 Patch 228139 5 jamesr 2010-05-19 18:40:23 -0700 Proposed patch up for review. Do you have any idea when this might have regressed, Ojan? 228140 6 ojan 2010-05-19 18:46:51 -0700 (In reply to comment #5) > Proposed patch up for review. Do you have any idea when this might have regressed, Ojan? No idea. Haven't tried looking at nightlies. 228168 7 56546 simon.fraser 2010-05-19 21:08:01 -0700 Comment on attachment 56546 Patch Does this allow you to remove the call to updateRendering () in isContentEditable()? Can you add assertions to ensure that updateRendering() doesn't happen during layout? 228172 8 jamesr 2010-05-19 21:17:51 -0700 Removing that call breaks some execCommand() tests. Hopefully that can be fixed easily as well but it'd be a different patch. 228224 9 56546 eric 2010-05-20 00:33:27 -0700 Comment on attachment 56546 Patch OK. 228495 10 56546 jamesr 2010-05-20 12:06:31 -0700 Comment on attachment 56546 Patch Gonna land by hand 228499 11 56546 jamesr 2010-05-20 12:18:46 -0700 Comment on attachment 56546 Patch Clearing flags on attachment: 56546 Committed r59856: <http://trac.webkit.org/changeset/59856> 228500 12 jamesr 2010-05-20 12:18:53 -0700 All reviewed patches have been landed. Closing bug. 228526 13 jamesr 2010-05-20 13:22:56 -0700 I'm starting a bisect now to see when this regressed. 228608 14 jamesr 2010-05-20 15:50:41 -0700 Surprise surprise, this particular test case was broken by http://trac.webkit.org/changeset/57081 since that patch eliminated a style recalc that would have papered over this issue. However I think the editing code was broken before r57081 anyway since it poked into the render tree without making any effort to see if styles were up to date. It is likely that there is a way to get this code to crash before r57081 (although it might be tricky). 228610 15 jamesr 2010-05-20 15:51:11 -0700 Also note that r57081 fixed a bunch of other crashes :) 228616 16 enrica 2010-05-20 15:55:24 -0700 <rdar://problem/8010983> 56530 2010-05-19 16:40:25 -0700 2010-05-19 16:40:25 -0700 test case scratch.html text/html 158 ojan PHNjcmlwdD4KZnVuY3Rpb24gZG9UaGluZygpIHsKICB0YS5mb2N1cygpOwogIHRhLnN0eWxlLmRp c3BsYXkgPSAibm9uZSI7Cn0KPC9zY3JpcHQ+Cjx0ZXh0YXJlYSBpZD0idGEiPmE8L3RleHRhcmVh Pgo8c3BhbiBvbmNsaWNrPSJkb1RoaW5nKCkiPkNsaWNrIG1lPC9zcGFuPgo= 56546 2010-05-19 18:39:34 -0700 2010-05-20 12:18:46 -0700 Patch bug-39389-20100519183933.patch text/plain 4268 jamesr ZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL0NoYW5nZUxvZyBiL0xheW91dFRlc3RzL0NoYW5nZUxv ZwppbmRleCA4ZDRkMDhhNjk0ZGI2NDRjOTRmMzVmYzU1OGZhMWEyOWRlYTBkNDVlLi5lOTYxODRk NDZkMDdlNTUyNDk2MDBlMzQxYWZmYTMxNTg5ZmQzZjliIDEwMDY0NAotLS0gYS9MYXlvdXRUZXN0 cy9DaGFuZ2VMb2cKKysrIGIvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTMgQEAK KzIwMTAtMDUtMTkgIEphbWVzIFJvYmluc29uICA8amFtZXNyQGNocm9taXVtLm9yZz4KKworICAg ICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBUZXN0cyBwdXR0aW5n IGZvY3VzIGludG8gYSB0ZXh0YXJlYSBhbmQgc2V0dGluZyBkaXNwbGF5Om5vbmUKKyAgICAgICAg aHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTM5Mzg5CisKKyAgICAgICAg KiBlZGl0aW5nL3NlbGVjdGlvbi9mb2N1cy1hbmQtZGlzcGxheS1ub25lLWV4cGVjdGVkLnR4dDog QWRkZWQuCisgICAgICAgICogZWRpdGluZy9zZWxlY3Rpb24vZm9jdXMtYW5kLWRpc3BsYXktbm9u ZS5odG1sOiBBZGRlZC4KKwogMjAxMC0wNS0xOSAgTWFydGluIFJvYmluc29uICA8bXJvYmluc29u QGlnYWxpYS5jb20+CiAKICAgICAgICAgTm90IHJldmlld2VkLgpkaWZmIC0tZ2l0IGEvTGF5b3V0 VGVzdHMvZWRpdGluZy9zZWxlY3Rpb24vZm9jdXMtYW5kLWRpc3BsYXktbm9uZS1leHBlY3RlZC50 eHQgYi9MYXlvdXRUZXN0cy9lZGl0aW5nL3NlbGVjdGlvbi9mb2N1cy1hbmQtZGlzcGxheS1ub25l LWV4cGVjdGVkLnR4dApuZXcgZmlsZSBtb2RlIDEwMDY0NAppbmRleCAwMDAwMDAwMDAwMDAwMDAw MDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwLi41ZWM3MzMwNmRkNGRmOGVhNzA4ZjU2YjUyZDUxNGQ0 NTBkMDU1NTA5Ci0tLSAvZGV2L251bGwKKysrIGIvTGF5b3V0VGVzdHMvZWRpdGluZy9zZWxlY3Rp b24vZm9jdXMtYW5kLWRpc3BsYXktbm9uZS1leHBlY3RlZC50eHQKQEAgLTAsMCArMSwyIEBACitD bGljayBtZQorVG8gcnVuIG1hbnVhbGx5LCBjbGljayB0aGUgJ0NsaWNrIG1lJyB0ZXh0LiBUZXN0 IHBhc3NlcyBpZiBpdCBkb2VzIG5vdCBjcmFzaC4KZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL2Vk aXRpbmcvc2VsZWN0aW9uL2ZvY3VzLWFuZC1kaXNwbGF5LW5vbmUuaHRtbCBiL0xheW91dFRlc3Rz L2VkaXRpbmcvc2VsZWN0aW9uL2ZvY3VzLWFuZC1kaXNwbGF5LW5vbmUuaHRtbApuZXcgZmlsZSBt b2RlIDEwMDY0NAppbmRleCAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAw Li5iZjU1M2M3ODYyNmVjODQ5YWE5NzJiNzc0ZWY5MjBkZTY5ZWEwYjk1Ci0tLSAvZGV2L251bGwK KysrIGIvTGF5b3V0VGVzdHMvZWRpdGluZy9zZWxlY3Rpb24vZm9jdXMtYW5kLWRpc3BsYXktbm9u ZS5odG1sCkBAIC0wLDAgKzEsNDMgQEAKKzwhRE9DVFlQRSBodG1sPgorPGh0bWw+Cis8Ym9keSBv bmxvYWQ9InJ1blRlc3QoKSI+Cis8c2NyaXB0PiAKK2Z1bmN0aW9uIGZvY3VzQW5kU2V0RGlzcGxh eU5vbmUoKSB7CisgIHZhciB0YSA9IGRvY3VtZW50LmdldEVsZW1lbnRCeUlkKCJ0YSIpOworICB0 YS5mb2N1cygpOworICB0YS5zdHlsZS5kaXNwbGF5ID0gIm5vbmUiOworfQorCitmdW5jdGlvbiBn ZXRQb3NpdGlvbk9mTm9kZShpZCkKK3sKKyAgICB2YXIgbiA9IGRvY3VtZW50LmdldEVsZW1lbnRC eUlkKGlkKTsKKyAgICB2YXIgcG9zID0ge3g6IDAsIHk6IDB9OworCisgICAgd2hpbGUgKG4pIHsK KyAgICAgICAgcG9zLnggKz0gbi5vZmZzZXRMZWZ0ICsgbi5jbGllbnRMZWZ0OworICAgICAgICBw b3MueSArPSBuLm9mZnNldFRvcCArIG4uY2xpZW50VG9wOworICAgICAgICBuID0gbi5vZmZzZXRQ YXJlbnQ7CisgICAgfQorICAgIHJldHVybiBwb3M7Cit9CisKK2Z1bmN0aW9uIHJ1blRlc3QoKSB7 CisgIGlmICh3aW5kb3cubGF5b3V0VGVzdENvbnRyb2xsZXIpIHsKKyAgICBsYXlvdXRUZXN0Q29u dHJvbGxlci5kdW1wQXNUZXh0KCkKKyAgICB2YXIgcG9zID0gZ2V0UG9zaXRpb25PZk5vZGUoImNs aWNrVGFyZ2V0Iik7CisgICAgLy8gUmVwZWF0IHRoZSB0ZXN0IGZvciBiZXR0ZXIgcmVwcm9kdWNh YmlsaXR5CisgICAgdmFyIGk7CisgICAgZm9yIChpPTA7IGk8MTA7ICsraSkgeworICAgICAgZG9j dW1lbnQuZ2V0RWxlbWVudEJ5SWQoInRhIikuc3R5bGUuZGlzcGxheT0iIjsKKyAgICAgIGV2ZW50 U2VuZGVyLm1vdXNlTW92ZVRvKHBvcy54ICsgNSwgcG9zLnkgKyA1KTsKKyAgICAgIGV2ZW50U2Vu ZGVyLm1vdXNlRG93bigpOworICAgICAgZXZlbnRTZW5kZXIubW91c2VVcCgpOworICAgIH0KKyAg fQorfQorPC9zY3JpcHQ+IAorPHRleHRhcmVhIGlkPSJ0YSI+YTwvdGV4dGFyZWE+IAorPHNwYW4g b25jbGljaz0iZm9jdXNBbmRTZXREaXNwbGF5Tm9uZSgpIiBpZD0iY2xpY2tUYXJnZXQiPkNsaWNr IG1lPC9zcGFuPjxicj4KK1RvIHJ1biBtYW51YWxseSwgY2xpY2sgdGhlICdDbGljayBtZScgdGV4 dC4gIFRlc3QgcGFzc2VzIGlmIGl0IGRvZXMgbm90IGNyYXNoLgorPC9ib2R5PgorPC9odG1sPgpk aWZmIC0tZ2l0IGEvV2ViQ29yZS9DaGFuZ2VMb2cgYi9XZWJDb3JlL0NoYW5nZUxvZwppbmRleCA2 NDI2OTg1ZTg1OGM1MmY0MjE2OGJmYTRkM2YwMjU0MTBhNzY0NjNhLi42YjdlOGZiOTMzODc2Y2Y2 NWNkMDQxNjJkZDdmNTg3NDIxYWY2NWU3IDEwMDY0NAotLS0gYS9XZWJDb3JlL0NoYW5nZUxvZwor KysgYi9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIyIEBACisyMDEwLTA1LTE5ICBKYW1l cyBSb2JpbnNvbiAgPGphbWVzckBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkg Tk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgRW5zdXJlcyBzdHlsZXMgYXJlIHVwLXRvLWRhdGUg YmVmb3JlIGRldGVybWluaW5nIHNlbGVjdGlvbgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0 Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MzkzODkKKworICAgICAgICBUaGlzIGFkZHMgYSBjYWxsIHRv IGRvY3VtZW50Ojp1cGRhdGVTdHlsZUlmTmVlZGVkKCkgdG8KKyAgICAgICAgRnJhbWU6Om5vdGlm eVJlbmRlcmVyT2ZTZWxlY3Rpb25DaGFuZ2UoKS4gV2l0aG91dCB0aGlzIGNhbGwsIGlmIHRoZXJl CisgICAgICAgIGlzIGEgcGVuZGluZyBzdHlsZSB1cGRhdGUgdGhhdCB3aWxsIGRldGFjaCBwYXJ0 IG9mIHRoZSBET00gYW5kIHRoZQorICAgICAgICB0aGUgc2VsZWN0aW9uIGlzIHdpdGhpbiB0aGF0 IHBhcnQgb2YgdGhlIERPTSB3aGVuCisgICAgICAgIG5vdGlmeVJlbmRlcmVyT2ZTZWxldGlvbkNo YW5nZSBpcyBjYWxsZWQsIHRoZSByb290RWRpdGFibGVFbGVtZW50CisgICAgICAgIGFsZ29yaXRo bSBlbmRzIHVwIHdhbGtpbmcgdGhyb3VnaCBkZXRhY2hlZCBwYXJ0cyBvZiB0aGUgcmVuZGVyIHRy ZWUuCisKKyAgICAgICAgVGVzdDogZWRpdGluZy9zZWxlY3Rpb24vZm9jdXMtYW5kLWRpc3BsYXkt bm9uZS5odG1sCisKKyAgICAgICAgKiBwYWdlL0ZyYW1lLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6 OkZyYW1lOjpub3RpZnlSZW5kZXJlck9mU2VsZWN0aW9uQ2hhbmdlKToKKwogMjAxMC0wNS0xOSAg S2V2aW4gV2F0dGVycyAgPGtldmlud2F0dGVyc0BnbWFpbC5jb20+CiAKICAgICAgICAgUmV2aWV3 ZWQgYnkgS2V2aW4gT2xsaXZpZXIuCmRpZmYgLS1naXQgYS9XZWJDb3JlL3BhZ2UvRnJhbWUuY3Bw IGIvV2ViQ29yZS9wYWdlL0ZyYW1lLmNwcAppbmRleCBhODJhN2ZjNjBiNWQ1NTVhMzZlODFiOGY0 ZjI1M2JhMjU3NzU3ZTFmLi5kYTc1MzlkOTU4ZmMzMGVmZTk4MGI4ODRlMmE4OGFlZWYwNWE0ZmZj IDEwMDY0NAotLS0gYS9XZWJDb3JlL3BhZ2UvRnJhbWUuY3BwCisrKyBiL1dlYkNvcmUvcGFnZS9G cmFtZS5jcHAKQEAgLTUzMiw2ICs1MzIsOSBAQCBTdHJpbmcgRnJhbWU6Om1hdGNoTGFiZWxzQWdh aW5zdEVsZW1lbnQoY29uc3QgVmVjdG9yPFN0cmluZz4mIGxhYmVscywgRWxlbWVudCogZQogdm9p ZCBGcmFtZTo6bm90aWZ5UmVuZGVyZXJPZlNlbGVjdGlvbkNoYW5nZShib29sIHVzZXJUcmlnZ2Vy ZWQpCiB7CiAgICAgUmVuZGVyT2JqZWN0KiByZW5kZXJlciA9IDA7CisKKyAgICBkb2N1bWVudCgp LT51cGRhdGVTdHlsZUlmTmVlZGVkKCk7CisKICAgICBpZiAoc2VsZWN0aW9uKCktPnJvb3RFZGl0 YWJsZUVsZW1lbnQoKSkKICAgICAgICAgcmVuZGVyZXIgPSBzZWxlY3Rpb24oKS0+cm9vdEVkaXRh YmxlRWxlbWVudCgpLT5zaGFkb3dBbmNlc3Rvck5vZGUoKS0+cmVuZGVyZXIoKTsKIAo=