Bug 13938 - REGRESSION: Difficult to repro crash in RenderBlock::layoutBlock using iGoogle
Summary: REGRESSION: Difficult to repro crash in RenderBlock::layoutBlock using iGoogle
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 523.x (Safari 3)
Hardware: Mac OS X 10.4
: P1 Normal
Assignee: Nobody
URL:
Keywords: NeedsReduction, Regression
Depends on:
Blocks:
 
Reported: 2007-05-30 12:45 PDT by Beth Dakin
Modified: 2007-06-14 15:52 PDT (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Beth Dakin 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
Comment 1 Beth Dakin 2007-05-30 12:46:37 PDT
<rdar://problem/5198882> CrashTracer: [USER] 7 crashes in Safari at com.apple.WebCore: WebCore::RenderBlock::layoutBlock + 2360
Comment 2 mitz 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).
Comment 3 mitz 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?
Comment 4 David Kilzer (:ddkilzer) 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.
Comment 5 David Kilzer (:ddkilzer) 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.  :(
Comment 6 David Kilzer (:ddkilzer) 2007-06-04 17:30:44 PDT
This needs a new Radar to track; the one listed in Comment #1 has been closed.
Comment 7 Dave Hyatt 2007-06-04 18:01:50 PDT
It's just an assert.
Comment 8 David Kilzer (:ddkilzer) 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!  :)

Comment 9 David Kilzer (:ddkilzer) 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?

Comment 10 mitz 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.