Bug 14899 – !d->m_view->needsLayout() in Frame::paint() (Causes assert)

• Component: Layout and Rendering
• Status: RESOLVED FIXED

ASSERTION FAILED: d->m_view && !d->m_view->needsLayout()
(WebCore/page/Frame.cpp:1383 void WebCore::Frame::paint(WebCore::GraphicsContext*, const WebCore::IntRect&))

Backtrace:
0   com.apple.WebCore             	0x01a39948 WebCore::Frame::paint(WebCore::GraphicsContext*, WebCore::IntRect const&) + 720 (Frame.cpp:1383)
1   com.apple.WebCore             	0x01a8d290 -[WebCoreFrameBridge drawRect:] + 372 (WebCoreFrameBridge.mm:425)
2   com.apple.WebKit              	0x001d375c -[WebHTMLView drawSingleRect:] + 1028
3   com.apple.WebKit              	0x001d3b84 -[WebHTMLView drawRect:] + 552
4   com.apple.AppKit              	0x95779178 -[NSView _drawRect:clip:] + 2892
5   com.apple.AppKit              	0x95778310 -[NSView _recursiveDisplayAllDirtyWithLockFocus:visRect:] + 860
6   com.apple.WebKit              	0x001ca168 -[WebHTMLView(WebPrivate) _recursiveDisplayAllDirtyWithLockFocus:visRect:] + 420
7   com.apple.CoreFoundation      	0x943b64fc CFArrayApplyFunction + 348
8   com.apple.AppKit              	0x957784c0 -[NSView _recursiveDisplayAllDirtyWithLockFocus:visRect:] + 1292
9   com.apple.CoreFoundation      	0x943b64fc CFArrayApplyFunction + 348
10  com.apple.AppKit              	0x957784c0 -[NSView _recursiveDisplayAllDirtyWithLockFocus:visRect:] + 1292
11  com.apple.CoreFoundation      	0x943b64fc CFArrayApplyFunction + 348
12  com.apple.AppKit              	0x957784c0 -[NSView _recursiveDisplayAllDirtyWithLockFocus:visRect:] + 1292
13  com.apple.CoreFoundation      	0x943b64fc CFArrayApplyFunction + 348
14  com.apple.AppKit              	0x957784c0 -[NSView _recursiveDisplayAllDirtyWithLockFocus:visRect:] + 1292
15  com.apple.CoreFoundation      	0x943b64fc CFArrayApplyFunction + 348
16  com.apple.AppKit              	0x957784c0 -[NSView _recursiveDisplayAllDirtyWithLockFocus:visRect:] + 1292
17  com.apple.CoreFoundation      	0x943b64fc CFArrayApplyFunction + 348
18  com.apple.AppKit              	0x957784c0 -[NSView _recursiveDisplayAllDirtyWithLockFocus:visRect:] + 1292
19  com.apple.AppKit              	0x95777284 -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] + 640
20  com.apple.AppKit              	0x95777ce8 -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] + 3300
21  com.apple.AppKit              	0x95777ce8 -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] + 3300
22  com.apple.AppKit              	0x9577680c -[NSThemeFrame _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] + 264
23  com.apple.AppKit              	0x95775d10 -[NSView _displayRectIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:] + 2172
24  com.apple.AppKit              	0x95774cc4 -[NSWindow displayIfNeeded] + 188

I think it is generally preferred to use separate ASSERTs instead of a conjunction.

[Matt Lilek]

You can reproduce this by starting the new iWeb with ToT WebKit and adding a HTML Snippet web widget thingy

(In reply to comment #2)
> You can reproduce this by starting the new iWeb with ToT WebKit and adding a
> HTML Snippet web widget thingy

Can you check if d->m_view is 0 (which I think is harmless) or d->m_view->needsLayout() is true (just break up the ASSERT)?

[Matt Lilek]

!d->m_view->needsLayout() is causing this to assert

[David Kilzer (:ddkilzer)]

<rdar://problem/5400975>

[Matt Lilek]

I'm seeing this consistently when clicking through <http://www.tuaw.com/photos/pixelmator-first-look/>

[David Kilzer (:ddkilzer)]

I'm seeing the same assertion clicking around NASCAR.com.  Haven't come up with a way to reproduce it, but clicking between the "Races" and "Schedule" links two or three times seems to do it.

Used a local debug build of WebKit r25142 with Safari 3 Public Beta v. 3.0.3 (522.12.1) on Mac OS X 10.4.10 (8R218).

Console output:

ASSERTION FAILED: d->m_view && !d->m_view->needsLayout()
(/path/to/WebKit/WebCore/page/Frame.cpp:1394 void WebCore::Frame::paint(WebCore::GraphicsContext*, const WebCore::IntRect&))
Segmentation fault

Stack trace:

Exception:  EXC_BAD_ACCESS (0x0001)
Codes:      KERN_INVALID_ADDRESS (0x0001) at 0xbbadbeef

Thread 0 Crashed:
0   com.apple.WebCore              	0x010f3378 WebCore::Frame::paint(WebCore::GraphicsContext*, WebCore::IntRect const&) + 788 (Frame.cpp:1394)
1   com.apple.WebCore              	0x01121608 -[WebCoreFrameBridge drawRect:] + 372 (WebCoreFrameBridge.mm:426)
2   com.apple.WebKit               	0x003525dc -[WebHTMLView drawSingleRect:] + 760 (WebHTMLView.mm:2820)
3   com.apple.WebKit               	0x00352a88 -[WebHTMLView drawRect:] + 540 (WebHTMLView.mm:2875)
4   com.apple.AppKit               	0x937e7858 -[NSView _drawRect:clip:] + 2128
5   com.apple.AppKit               	0x937e65fc -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] + 736
6   com.apple.WebKit               	0x003490fc -[WebHTMLView(WebPrivate) _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] + 468 (WebHTMLView.mm:1060)
7   com.apple.AppKit               	0x937e69a8 -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] + 1676
8   com.apple.AppKit               	0x937e69a8 -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] + 1676
9   com.apple.AppKit               	0x937e69a8 -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] + 1676
10  com.apple.AppKit               	0x937e69a8 -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] + 1676
11  com.apple.AppKit               	0x937e69a8 -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] + 1676
12  com.apple.AppKit               	0x937e69a8 -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] + 1676
13  com.apple.AppKit               	0x937e69a8 -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] + 1676
14  com.apple.AppKit               	0x937e69a8 -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] + 1676
15  com.apple.AppKit               	0x93807044 -[NSThemeFrame _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] + 192
16  com.apple.AppKit               	0x937e0054 -[NSView _displayRectIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:] + 384
17  com.apple.AppKit               	0x937d5348 -[NSView displayIfNeeded] + 248
18  com.apple.AppKit               	0x937d51b8 -[NSWindow displayIfNeeded] + 180
19  com.apple.Safari               	0x000133d4 0x1000 + 74708
20  com.apple.AppKit               	0x937d5064 _handleWindowNeedsDisplay + 200
21  com.apple.CoreFoundation       	0x907dd76c __CFRunLoopDoObservers + 352
22  com.apple.CoreFoundation       	0x907dda0c __CFRunLoopRun + 420
23  com.apple.CoreFoundation       	0x907dd4ac CFRunLoopRunSpecific + 268
24  com.apple.HIToolbox            	0x9329bb20 RunCurrentEventLoopInMode + 264
25  com.apple.HIToolbox            	0x9329b1b4 ReceiveNextEventCommon + 380
26  com.apple.HIToolbox            	0x9329b020 BlockUntilNextEventMatchingListInMode + 96
27  com.apple.AppKit               	0x937a1ae4 _DPSNextEvent + 384
28  com.apple.AppKit               	0x937a17a8 -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 116
29  com.apple.Safari               	0x00006770 0x1000 + 22384
30  com.apple.AppKit               	0x9379dcec -[NSApplication run] + 472
31  com.apple.AppKit               	0x9388e87c NSApplicationMain + 452
32  com.apple.Safari               	0x0000244c 0x1000 + 5196
33  com.apple.Safari               	0x0004f1b0 0x1000 + 319920

[David Kilzer (:ddkilzer)]

Races URL: http://www.nascar.com/races/cup/
Schedule URL: http://www.nascar.com/races/cup/2007/data/schedule.html

Created attachment 16028 [details]
Possible fix

I am unable to reproduce the bug with the URLs given by Matt and Dave, but this patch fixes <http://ynet.co.il> for me. Could you guys see if it fixes your reproducible cases?

As for the patch itself, it maintains the behavior of copying the width and height attributes in the DOM from the <embed> to the enclosing <object>, just avoids doing it during layout. I don't know if the attribute copying is necessary in the first place (maybe it's a WinIE thing; Firefox doesn't do it).

[David Kilzer (:ddkilzer)]

(In reply to comment #9)
> Created an attachment (id=16028) [edit]
> Possible fix
> 
> I am unable to reproduce the bug with the URLs given by Matt and Dave, but this
> patch fixes <http://ynet.co.il> for me. Could you guys see if it fixes your
> reproducible cases?

I'm running with the patch now, but I had trouble reproducing the crash (without this patch) an hour after it happened on nascar.com.  Will keep trying!

[Matt Lilek]

This patch seems to fix it for me.

Created attachment 16065 [details]
Fix one reproducible instance of the ASSERT

Includes a manual test.

Since the bug was reported before updateWidget could be called under layout() (r25128), I am not sure that the patch addresses the original bug, but it does fix a reproducible bug with the same symptom.

[Darin Adler]

Comment on attachment 16065 [details]
Fix one reproducible instance of the ASSERT

+    String width = getAttribute(widthAttr);
+    String height = getAttribute(heightAttr);

It'd be slightly more efficient to use const AtomicString& for these. This would avoid a little bit of reference count churn. If we changed setAttribute to take const AtomicString& we could also save re-hashing the string values to make them Atomic again.

But that's very minor and I like the patch as-is.

r=me

[Andrew Wellington]

Landed in r25197