32009 2009-11-30 23:17:21 -0800 Crash in RenderBlock::startDelayUpdateScrollInfo 2009-12-28 19:04:58 -0800 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) PC OS X 10.5 RESOLVED DUPLICATE 32172 P2 Normal --- 1 hamaji webkit-unassigned hyatt webkit.review.bot oldest_to_newest 167517 0 hamaji 2009-11-30 23:17:21 -0800 After Bug 15135 (this was my change, sorry), the following HTML causes crash or assertion failure. <style> .test { overflow-x: overlay; width: 50; display: -webkit-box; } </style> <script> if (window.layoutTestController) layoutTestController.dumpAsText(); </script> <body> <div class="test"> <input type=file class="test"> </div> No crash means PASS </body> This is the stacktrace: (gdb) bt #0 0x0000000001dd7401 in WebCore::RenderBlock::startDelayUpdateScrollInfo () at /usr/local/google/chrome-webkit/src/third_party/WebKit/WebCore/rendering/RenderBlock.cpp:605 #1 0x0000000001e1fdf9 in WebCore::RenderFlexibleBox::layoutHorizontalBox ( this=0x7fffe8062f08, relayoutChildren=false) at /usr/local/google/chrome-webkit/src/third_party/WebKit/WebCore/rendering/RenderFlexibleBox.cpp:336 #2 0x0000000001e2131c in WebCore::RenderFlexibleBox::layoutBlock ( this=0x7fffe8062f08, relayoutChildren=false) at /usr/local/google/chrome-webkit/src/third_party/WebKit/WebCore/rendering/RenderFlexibleBox.cpp:242 #3 0x0000000001de0f25 in WebCore::RenderBlock::layout (this=0x7fffe8062f08) at /usr/local/google/chrome-webkit/src/third_party/WebKit/WebCore/rendering/RenderBlock.cpp:648 #4 0x0000000001de4c4f in WebCore::RenderObject::layoutIfNeeded ( this=0x7fffe8062f08) at /usr/local/google/chrome-webkit/src/third_party/WebKit/WebCore/rendering/RenderObject.h:496 #5 0x0000000001dfdf11 in WebCore::RenderBlock::layoutInlineChildren ( this=0x7fffe8062788, relayoutChildren=true, repaintTop=@0x438249cc, repaintBottom=@0x438249c8) at /usr/local/google/chrome-webkit/src/third_party/WebKit/WebCore/rendering/---Type <return> to continue, or q <return> to quit--- RenderBlockLineLayout.cpp:865 #6 0x0000000001de15a9 in WebCore::RenderBlock::layoutBlock ( this=0x7fffe8062788, relayoutChildren=true) at /usr/local/google/chrome-webkit/src/third_party/WebKit/WebCore/rendering/RenderBlock.cpp:722 #7 0x0000000001e3b523 in WebCore::RenderLayer::updateScrollInfoAfterLayout ( this=0x7fffe80628c8) at /usr/local/google/chrome-webkit/src/third_party/WebKit/WebCore/rendering/RenderLayer.cpp:1872 #8 0x0000000001de1cd8 in WebCore::RenderBlock::finishDelayUpdateScrollInfo () at /usr/local/google/chrome-webkit/src/third_party/WebKit/WebCore/rendering/RenderBlock.cpp:622 #9 0x0000000001e20cb4 in WebCore::RenderFlexibleBox::layoutHorizontalBox ( this=0x7fffe805f318, relayoutChildren=false) at /usr/local/google/chrome-webkit/src/third_party/WebKit/WebCore/rendering/RenderFlexibleBox.cpp:558 ... This crash happens because finishDelayUpdateScrollInfo() triggers layout of another flexible box and it calls startDelayUpdateScrollInfo(). This function modifies a global variable gDelayUpdateScrollInfoSet before finishDelayUpdateScrollInfo() finalize the global variable. This bug can be easily fixed by evacuating the global variable into local variable before we trigger other layouts. Chromium side: http://code.google.com/p/chromium/issues/detail?id=27085 167518 1 44056 hamaji 2009-11-30 23:19:47 -0800 Created attachment 44056 Patch v1 167523 2 webkit.review.bot 2009-11-30 23:24:31 -0800 style-queue ran check-webkit-style on attachment 44056 without any errors. 169628 3 hamaji 2009-12-08 01:37:56 -0800 Ping? I think this change is not so difficult to review because this is basically a fix for wrong ownership of obejcts, and not related to complex layout stuff. Thanks! 175362 4 44056 mjs 2009-12-28 18:24:40 -0800 Comment on attachment 44056 Patch v1 I suggest reducing the comment to one line: // updateScrollInfoAfterLayout() may modify gDelayedUpdateScrollInfoSet, so save a copy This idiom is common in WebKit so excessive explanation is not needed. Fix that and I'll gladly r+ this patch. 175374 5 hamaji 2009-12-28 19:04:58 -0800 Thanks for the review. However, I've just noticed the almost identical change was done in Bug 32172... *** This bug has been marked as a duplicate of bug 32172 *** 44056 2009-11-30 23:19:47 -0800 2009-12-28 18:24:40 -0800 Patch v1 bug-32009-20091201161945.patch text/plain 3570 hamaji ZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL0NoYW5nZUxvZyBiL0xheW91dFRlc3RzL0NoYW5nZUxv ZwppbmRleCBhNzg0ODY4Li5mZjY4ZDU0IDEwMDY0NAotLS0gYS9MYXlvdXRUZXN0cy9DaGFuZ2VM b2cKKysrIGIvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTMgQEAKKzIwMDktMTEt MzAgIFNoaW5pY2hpcm8gSGFtYWppICA8aGFtYWppQGNocm9taXVtLm9yZz4KKworICAgICAgICBS ZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBDcmFzaCBpbiBSZW5kZXJCbG9j azo6c3RhcnREZWxheVVwZGF0ZVNjcm9sbEluZm8KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtp dC5vcmcvc2hvd19idWcuY2dpP2lkPTMyMDA5CisKKyAgICAgICAgKiBmYXN0L2ZsZXhib3gvbmVz dGVkLW92ZXJmbG93LWZsZXgtaGFuZy1leHBlY3RlZC50eHQ6IENvcGllZCBmcm9tIExheW91dFRl c3RzL2Zhc3QvY3NzLWdlbmVyYXRlZC1jb250ZW50L2Fic29sdXRlLXBvc2l0aW9uLWluc2lkZS1p bmxpbmUtZXhwZWN0ZWQudHh0LgorICAgICAgICAqIGZhc3QvZmxleGJveC9uZXN0ZWQtb3ZlcmZs b3ctZmxleC1oYW5nLmh0bWw6IEFkZGVkLgorCiAyMDA5LTExLTMwICBFbnJpY2EgQ2FzdWNjaSAg PGVucmljYUBhcHBsZS5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgRGFyaW4gQWRsZXIuCmRp ZmYgLS1naXQgYS9MYXlvdXRUZXN0cy9mYXN0L2ZsZXhib3gvbmVzdGVkLW92ZXJmbG93LWZsZXgt aGFuZy1leHBlY3RlZC50eHQgYi9MYXlvdXRUZXN0cy9mYXN0L2ZsZXhib3gvbmVzdGVkLW92ZXJm bG93LWZsZXgtaGFuZy1leHBlY3RlZC50eHQKbmV3IGZpbGUgbW9kZSAxMDA2NDQKaW5kZXggMDAw MDAwMC4uNzY5NjQyMQotLS0gL2Rldi9udWxsCisrKyBiL0xheW91dFRlc3RzL2Zhc3QvZmxleGJv eC9uZXN0ZWQtb3ZlcmZsb3ctZmxleC1oYW5nLWV4cGVjdGVkLnR4dApAQCAtMCwwICsxIEBACitO byBjcmFzaCBtZWFucyBQQVNTCmRpZmYgLS1naXQgYS9MYXlvdXRUZXN0cy9mYXN0L2ZsZXhib3gv bmVzdGVkLW92ZXJmbG93LWZsZXgtaGFuZy5odG1sIGIvTGF5b3V0VGVzdHMvZmFzdC9mbGV4Ym94 L25lc3RlZC1vdmVyZmxvdy1mbGV4LWhhbmcuaHRtbApuZXcgZmlsZSBtb2RlIDEwMDY0NAppbmRl eCAwMDAwMDAwLi5iZTEzY2ZmCi0tLSAvZGV2L251bGwKKysrIGIvTGF5b3V0VGVzdHMvZmFzdC9m bGV4Ym94L25lc3RlZC1vdmVyZmxvdy1mbGV4LWhhbmcuaHRtbApAQCAtMCwwICsxLDI1IEBACis8 aHRtbD4KKzxoZWFkPgorPHRpdGxlPkJ1ZyAzMjAwOSAtIENyYXNoIGluIFJlbmRlckJsb2NrOjpz dGFydERlbGF5VXBkYXRlU2Nyb2xsSW5mbzwvdGl0bGU+Cis8L2hlYWQ+CisKKzxzdHlsZT4KKy50 ZXN0IHsKKyAgICBvdmVyZmxvdy14OiBvdmVybGF5OworICAgIHdpZHRoOiA1MDsKKyAgICBkaXNw bGF5OiAtd2Via2l0LWJveDsKK30KKzwvc3R5bGU+CisKKzxzY3JpcHQ+CitpZiAod2luZG93Lmxh eW91dFRlc3RDb250cm9sbGVyKQorICAgIGxheW91dFRlc3RDb250cm9sbGVyLmR1bXBBc1RleHQo KTsKKzwvc2NyaXB0PgorCis8Ym9keT4KKyAgPGRpdiBjbGFzcz0idGVzdCI+CisgICAgPGlucHV0 IHR5cGU9ZmlsZSBjbGFzcz0idGVzdCI+CisgIDwvZGl2PgorICBObyBjcmFzaCBtZWFucyBQQVNT Cis8L2JvZHk+Cis8L2h0bWw+CmRpZmYgLS1naXQgYS9XZWJDb3JlL0NoYW5nZUxvZyBiL1dlYkNv cmUvQ2hhbmdlTG9nCmluZGV4IGViOGZlYjYuLjUwMDJmNGEgMTAwNjQ0Ci0tLSBhL1dlYkNvcmUv Q2hhbmdlTG9nCisrKyBiL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTUgQEAKKzIwMDkt MTEtMzAgIFNoaW5pY2hpcm8gSGFtYWppICA8aGFtYWppQGNocm9taXVtLm9yZz4KKworICAgICAg ICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBDcmFzaCBpbiBSZW5kZXJC bG9jazo6c3RhcnREZWxheVVwZGF0ZVNjcm9sbEluZm8KKyAgICAgICAgaHR0cHM6Ly9idWdzLndl YmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTMyMDA5CisKKyAgICAgICAgVGVzdDogZmFzdC9mbGV4 Ym94L25lc3RlZC1vdmVyZmxvdy1mbGV4LWhhbmcuaHRtbAorCisgICAgICAgICogcmVuZGVyaW5n L1JlbmRlckJsb2NrLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OlJlbmRlckJsb2NrOjpmaW5pc2hE ZWxheVVwZGF0ZVNjcm9sbEluZm8pOgorCiAyMDA5LTExLTMwICBGdW1pdG9zaGkgVWthaSAgPHVr YWlAY2hyb21pdW0ub3JnPgogCiAgICAgICAgIFVucmV2aWV3ZWQgQ2hyb21pdW0gYnVpbGQgZml4 IGludHJvZHVjZWQgYnkgcjUxMjEyCmRpZmYgLS1naXQgYS9XZWJDb3JlL3JlbmRlcmluZy9SZW5k ZXJCbG9jay5jcHAgYi9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJCbG9jay5jcHAKaW5kZXggN2Y3 MGIwNy4uMmI1Yzk0NiAxMDA2NDQKLS0tIGEvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyQmxvY2su Y3BwCisrKyBiL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlckJsb2NrLmNwcApAQCAtNjE2LDE1ICs2 MTYsMTggQEAgdm9pZCBSZW5kZXJCbG9jazo6ZmluaXNoRGVsYXlVcGRhdGVTY3JvbGxJbmZvKCkK ICAgICBpZiAoZ0RlbGF5VXBkYXRlU2Nyb2xsSW5mbyA9PSAwKSB7CiAgICAgICAgIEFTU0VSVChn RGVsYXllZFVwZGF0ZVNjcm9sbEluZm9TZXQpOwogCi0gICAgICAgIGZvciAoRGVsYXllZFVwZGF0 ZVNjcm9sbEluZm9TZXQ6Oml0ZXJhdG9yIGl0ID0gZ0RlbGF5ZWRVcGRhdGVTY3JvbGxJbmZvU2V0 LT5iZWdpbigpOyBpdCAhPSBnRGVsYXllZFVwZGF0ZVNjcm9sbEluZm9TZXQtPmVuZCgpOyArK2l0 KSB7CisgICAgICAgIC8vIFRoZSB1cGRhdGVTY3JvbGxJbmZvQWZ0ZXJMYXlvdXQoKSBtYXkgdHJp Z2dlciBvdGhlciBsYXlvdXRzCisgICAgICAgIC8vIG9mIGZsZXhpYmxlIGJveCBhbmQgYXR0ZW1w dCB0byBtb2RpZnkgZ0RlbGF5ZWRVcGRhdGVTY3JvbGxJbmZvU2V0LgorICAgICAgICAvLyBTbywg d2UgbmVlZCB0byBldmFjdWF0ZSBpdCB0byBhIGxvY2FsIHZhcmlhYmxlLgorICAgICAgICBPd25Q dHI8RGVsYXllZFVwZGF0ZVNjcm9sbEluZm9TZXQ+IGRlbGF5ZWRVcGRhdGVTY3JvbGxJbmZvU2V0 KGdEZWxheWVkVXBkYXRlU2Nyb2xsSW5mb1NldCk7CisgICAgICAgIGdEZWxheWVkVXBkYXRlU2Ny b2xsSW5mb1NldCA9IDA7CisKKyAgICAgICAgZm9yIChEZWxheWVkVXBkYXRlU2Nyb2xsSW5mb1Nl dDo6aXRlcmF0b3IgaXQgPSBkZWxheWVkVXBkYXRlU2Nyb2xsSW5mb1NldC0+YmVnaW4oKTsgaXQg IT0gZGVsYXllZFVwZGF0ZVNjcm9sbEluZm9TZXQtPmVuZCgpOyArK2l0KSB7CiAgICAgICAgICAg ICBSZW5kZXJCbG9jayogYmxvY2sgPSAqaXQ7CiAgICAgICAgICAgICBpZiAoYmxvY2stPmhhc092 ZXJmbG93Q2xpcCgpKSB7CiAgICAgICAgICAgICAgICAgYmxvY2stPmxheWVyKCktPnVwZGF0ZVNj cm9sbEluZm9BZnRlckxheW91dCgpOwogICAgICAgICAgICAgfQogICAgICAgICB9Ci0KLSAgICAg ICAgZGVsZXRlIGdEZWxheWVkVXBkYXRlU2Nyb2xsSW5mb1NldDsKLSAgICAgICAgZ0RlbGF5ZWRV cGRhdGVTY3JvbGxJbmZvU2V0ID0gMDsKICAgICB9CiB9CiAK