RESOLVED DUPLICATE of bug 32172 32009
Crash in RenderBlock::startDelayUpdateScrollInfo 
https://bugs.webkit.org/show_bug.cgi?id=32009
Summary Crash in RenderBlock::startDelayUpdateScrollInfo
Shinichiro Hamaji
Reported 2009-11-30 23:17:21 PST
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
Attachments
Patch v1 (3.49 KB, patch)
2009-11-30 23:19 PST, Shinichiro Hamaji 		mjs: review-
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Shinichiro Hamaji
Comment 1 2009-11-30 23:19:47 PST
Created attachment 44056 [details] Patch v1
WebKit Review Bot
Comment 2 2009-11-30 23:24:31 PST
style-queue ran check-webkit-style on attachment 44056 [details] without any errors.
Shinichiro Hamaji
Comment 3 2009-12-08 01:37:56 PST
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!
Maciej Stachowiak
Comment 4 2009-12-28 18:24:40 PST
Comment on attachment 44056 [details] 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.
Shinichiro Hamaji
Comment 5 2009-12-28 19:04:58 PST
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 ***
Note You need to log in before you can comment on or make changes to this bug.