Bug 32009

Summary:

Crash in RenderBlock::startDelayUpdateScrollInfo

Product:

WebKit

Reporter:

Shinichiro Hamaji <hamaji>

Component:

Layout and Rendering

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED DUPLICATE

Severity:

Normal

CC:

hyatt, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

OS:

OS X 10.5

Attachments:

Description	Flags
Patch v1	mjs: review-

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-	Details Formatted Diff Diff
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.