Bug 13938

Summary: REGRESSION: Difficult to repro crash in RenderBlock::layoutBlock using iGoogle
Product: WebKit Reporter: Beth Dakin <bdakin>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bdakin, ddkilzer, hyatt, mitz
Priority: P1 Keywords: NeedsReduction, Regression
Version: 523.x (Safari 3)   
Hardware: Mac   
OS: OS X 10.4   

Beth Dakin
Reported 2007-05-30 12:45:56 PDT
We have had several reports of people crashing loading/playing with the new iGoogle Google homepage. Here is what we have learned about the crash so far: 1. It was introduced with http://trac.webkit.org/projects/webkit/changeset/21183 2. Antti caught this in the debugger once, and discovered that the problem is that we are triggering layout() from SelectionController()::caretRect() and end up over-popping layout state. He crashed in RenderView::layout() at ASSERT(!m_layoutState); Here is the relevant part of his debug stack trace: #0 0x0114587a in WebCore::RenderView::layout at RenderView.cpp:104 #1 0x010c4d6f in WebCore::FrameView::layout at FrameView.cpp:418 #2 0x010c7aad in WebCore::Document::updateLayout at Document.cpp:1054 #3 0x010c7a2b in WebCore::Document::updateLayout at Document.cpp:1046 #4 0x010d2efc in WebCore::Document::updateLayoutIgnorePendingStylesheets at Document.cpp:1080 #5 0x011e998b in WebCore::VisiblePosition::canonicalPosition at VisiblePosition.cpp:130 #6 0x011e9d4c in WebCore::VisiblePosition::init at VisiblePosition.cpp:58 #7 0x011e9f48 in WebCore::VisiblePosition::VisiblePosition at VisiblePosition.cpp:45 #8 0x011d9e54 in WebCore::SelectionController::layout at SelectionController.cpp:839 #9 0x011da04f in WebCore::SelectionController::caretRect at SelectionController.cpp:856 #10 0x011da360 in WebCore::SelectionController::recomputeCaretRect at SelectionController.cpp:896 #11 0x010b7475 in WebCore::Frame::selectionLayoutChanged at Frame.cpp:585 #12 0x010b7616 in WebCore::Frame::invalidateSelection at Frame.cpp:522 #13 0x010c4db7 in WebCore::FrameView::layout at FrameView.cpp:423 #14 0x0127518c in WebCore::RenderPart::updateWidgetPosition at RenderPart.cpp:115 #15 0x01146723 in WebCore::RenderView::updateWidgetPositions at RenderView.cpp:447 #16 0x0115c510 in WebCore::RenderLayer::scrollToOffset at RenderLayer.cpp:671 #17 0x0115e2dc in WebCore::RenderLayer::updateScrollInfoAfterLayout at RenderLayer.cpp:1170 #18 0x0113a89e in WebCore::RenderBlock::layoutBlock at RenderBlock.cpp:647 And here is the full stack trace that we have: Thread 0 Crashed (i386): >#0 com.apple.WebCore 0x9556eab8 WebCore::RenderBlock::layoutBlock(bool) + 2360 #1 com.apple.WebCore 0x955618c8 WebCore::RenderBlock::layout() + 40 #2 com.apple.WebCore 0x9556dc4e WebCore::RenderBlock::layoutBlockChildren(bool) + 814 #3 com.apple.WebCore 0x9556e892 WebCore::RenderBlock::layoutBlock(bool) + 1810 #4 com.apple.WebCore 0x955618c8 WebCore::RenderBlock::layout() + 40 #5 com.apple.WebCore 0x9556dc4e WebCore::RenderBlock::layoutBlockChildren(bool) + 814 #6 com.apple.WebCore 0x9556e892 WebCore::RenderBlock::layoutBlock(bool) + 1810 #7 com.apple.WebCore 0x955618c8 WebCore::RenderBlock::layout() + 40 #8 com.apple.WebCore 0x9555d3fb WebCore::RenderBlock::layoutInlineChildren(bool, int&, int&) + 2235 #9 com.apple.WebCore 0x9556e453 WebCore::RenderBlock::layoutBlock(bool) + 723 #10 com.apple.WebCore 0x955618c8 WebCore::RenderBlock::layout() + 40 #11 com.apple.WebCore 0x9556dc4e WebCore::RenderBlock::layoutBlockChildren(bool) + 814 #12 com.apple.WebCore 0x9556e892 WebCore::RenderBlock::layoutBlock(bool) + 1810 #13 com.apple.WebCore 0x955618c8 WebCore::RenderBlock::layout() + 40 #14 com.apple.WebCore 0x9556dc4e WebCore::RenderBlock::layoutBlockChildren(bool) + 814 #15 com.apple.WebCore 0x9556e892 WebCore::RenderBlock::layoutBlock(bool) + 1810 #16 com.apple.WebCore 0x955618c8 WebCore::RenderBlock::layout() + 40 #17 com.apple.WebCore 0x9556dc4e WebCore::RenderBlock::layoutBlockChildren(bool) + 814 #18 com.apple.WebCore 0x9556e892 WebCore::RenderBlock::layoutBlock(bool) + 1810 #19 com.apple.WebCore 0x955618c8 WebCore::RenderBlock::layout() + 40 #20 com.apple.WebCore 0x9556dc4e WebCore::RenderBlock::layoutBlockChildren(bool) + 814 #21 com.apple.WebCore 0x9556e892 WebCore::RenderBlock::layoutBlock(bool) + 1810 #22 com.apple.WebCore 0x955618c8 WebCore::RenderBlock::layout() + 40 #23 com.apple.WebCore 0x9556dc4e WebCore::RenderBlock::layoutBlockChildren(bool) + 814 #24 com.apple.WebCore 0x9556e892 WebCore::RenderBlock::layoutBlock(bool) + 1810 #25 com.apple.WebCore 0x955618c8 WebCore::RenderBlock::layout() + 40 #26 com.apple.WebCore 0x9556dc4e WebCore::RenderBlock::layoutBlockChildren(bool) + 814 #27 com.apple.WebCore 0x9556e892 WebCore::RenderBlock::layoutBlock(bool) + 1810 #28 com.apple.WebCore 0x955618c8 WebCore::RenderBlock::layout() + 40 #29 com.apple.WebCore 0x95579c48 WebCore::RenderView::layout() + 200 #30 com.apple.WebCore 0x954f25c6 WebCore::FrameView::layout(bool) + 422 #31 com.apple.WebCore 0x95851a62 WebCore::Timer<WebCore::FrameView>::fired() + 82 #32 com.apple.WebCore 0x95617e19 WebCore::TimerBase::fireTimers(double, WTF::Vector<WebCore::TimerBase*, 0ul> const&) + 137 #33 com.apple.WebCore 0x95617ed2 WebCore::TimerBase::sharedTimerFired() + 162 #34 com.apple.CoreFoundation 0x95ab1414 __CFRunLoopRun + 4180 #35 com.apple.CoreFoundation 0x95ab17b9 CFRunLoopRunSpecific + 553 #36 com.apple.CoreFoundation 0x95ab1848 CFRunLoopRunInMode + 88 #37 com.apple.HIToolbox 0x90628055 RunCurrentEventLoopInMode + 305 #38 com.apple.HIToolbox 0x9062da61 ReceiveNextEventCommon + 374 #39 com.apple.HIToolbox 0x9062db6d BlockUntilNextEventMatchingListInMode + 106 #40 com.apple.AppKit 0x932de79f _DPSNextEvent + 657 #41 com.apple.AppKit 0x932de0f2 -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 128 #42 com.apple.Safari 0x00006fde #43 com.apple.AppKit 0x932d8121 -[NSApplication run] + 795 #44 com.apple.AppKit 0x932cb480 NSApplicationMain + 663 #45 com.apple.Safari 0x00002e4f #46 com.apple.Safari 0x0004c159 #47 page zero 0x00000002
Attachments
Add attachment proposed patch, testcase, etc.
Beth Dakin
Comment 1 2007-05-30 12:46:37 PDT
<rdar://problem/5198882> CrashTracer: [USER] 7 crashes in Safari at com.apple.WebCore: WebCore::RenderBlock::layoutBlock + 2360
mitz
Comment 2 2007-05-30 13:04:15 PDT
Looks like a regression from <http://trac.webkit.org/projects/webkit/changeset/20936> which was a fix for bug 13375. Document::updateLayout() was changed to update the parent frame's layout. In the case of this bug, the parent frame is already in the middle of layout, so you get a nasty cycle (which is at least detected by the LayoutState assertion).
mitz
Comment 3 2007-05-31 10:01:22 PDT
So this bug turned out to be fixed already, but Beth added the midLayout guard around invalidateSelection() is <http://trac.webkit.org/projects/webkit/changeset/21905> and just now going to maps.google.com I hit the ASSERT(!d->midLayout), with this backtrace: 0 com.apple.WebCore 0x01101a68 WebCore::FrameView::layout(bool) + 100 (FrameView.cpp:290) 1 com.apple.WebCore 0x01106110 WebCore::Document::updateLayout() + 260 (Document.cpp:1060) 2 com.apple.WebCore 0x0111576c WebCore::Document::updateLayoutIgnorePendingStylesheets() + 200 (Document.cpp:1087) 3 com.apple.WebCore 0x012851b4 WebCore::VisiblePosition::canonicalPosition(WebCore::Position const&) + 104 (VisiblePosition.cpp:143) 4 com.apple.WebCore 0x01285730 WebCore::VisiblePosition::init(WebCore::Position const&, WebCore::EAffinity) + 68 (VisiblePosition.cpp:58) 5 com.apple.WebCore 0x012859a0 WebCore::VisiblePosition::VisiblePosition[in-charge](WebCore::Position const&, WebCore::EAffinity) + 60 (VisiblePosition .cpp:46) 6 com.apple.WebCore 0x0127173c WebCore::SelectionController::layout() + 648 (SelectionController.cpp:839) 7 com.apple.WebCore 0x01271960 WebCore::SelectionController::caretRect() const + 56 (SelectionController.cpp:858) 8 com.apple.WebCore 0x01271d84 WebCore::SelectionController::recomputeCaretRect() + 276 (SelectionController.cpp:896) 9 com.apple.WebCore 0x010eff04 WebCore::Frame::selectionLayoutChanged() + 52 (Frame.cpp:584) 10 com.apple.WebCore 0x010f01a8 WebCore::Frame::invalidateSelection() + 56 (Frame.cpp:523) 11 com.apple.WebCore 0x011023e8 WebCore::FrameView::layout(bool) + 2532 (FrameView.cpp:433) The FrameView being the same in frames #0 and #11. Also regarding the original problem (at iGoogle), if this is something that can legitimately happen within WebCore, isn't an ASSERT too harsh? I mean, does the iGoogle bug still need to be fixed some other way?
David Kilzer (:ddkilzer)
Comment 4 2007-06-04 09:36:51 PDT
Saw the same ASSERT(!d->midLayout) after accidentally hitting the "Yahoo" link in the Safari toolbar, then using Cmd-Left-Arrow to go back while the Yahoo main page (www.yahoo.com) was still loading. Oddly, Safari hung in such a way so that I couldn't attach via gdb or get a stack trace through CrashReporter.
David Kilzer (:ddkilzer)
Comment 5 2007-06-04 17:28:04 PDT
With a local debug build of WebKit r21908, I hit this EVERY time I go to http://maps.google.com/ now. It's really not that hard to reproduce now. :(
David Kilzer (:ddkilzer)
Comment 6 2007-06-04 17:30:44 PDT
This needs a new Radar to track; the one listed in Comment #1 has been closed.
Dave Hyatt
Comment 7 2007-06-04 18:01:50 PDT
It's just an assert.
David Kilzer (:ddkilzer)
Comment 8 2007-06-04 18:08:04 PDT
(In reply to comment #6) > This needs a new Radar to track; the one listed in Comment #1 has been closed. It's kind of hard to use Google Maps when it crashes all the time in a debug build, too! :)
David Kilzer (:ddkilzer)
Comment 9 2007-06-06 07:48:09 PDT
(In reply to comment #7) > It's just an assert. The assert was removed in r22022. http://trac.webkit.org/projects/webkit/changeset/22022 Is this bug fixed now?
mitz
Comment 10 2007-06-14 15:52:58 PDT
(In reply to comment #9) > Is this bug fixed now? Yes. It was actually fixed before it was filed.
Note You need to log in before you can comment on or make changes to this bug.