Bug 9141 – REGRESSION: loading PDF within a frame leads to ASSERT: !_haveAdditionalClip

• Status: NEW
• Component: Frames

[Trey Matteson]

Open the URL above.  Crashola:

   ASSERTION FAILED: !_haveAdditionalClip (/Volumes/Whopper/WebKit/WebKit/WebView/WebClipView.m:78 -[WebClipView setAdditionalClip:])

This is with rev 14619 on 10.4.6.  Works fine in stock 10.4.6 Safari.

From the backtrace (below) it appears to be the PDF content within the frame.  The PDF file loads fine with no frames.  A basic set of HTML frames (like some JavaDoc) works fine.

Note that PDFView is calling a scroll method (and thus display:) from within its drawRect:, which is a really bad idea (and has been the root of trouble in the past).


0   com.apple.WebKit         	0x00341f88 -[WebClipView setAdditionalClip:] + 116 (WebClipView.m:78)
1   com.apple.WebKit         	0x0036cf00 -[WebHTMLView drawSingleRect:] + 348 (WebHTMLView.m:2587)
2   com.apple.WebKit         	0x0036d348 -[WebHTMLView drawRect:] + 536 (WebHTMLView.m:2643)
3   com.apple.AppKit         	0x93765bf8 -[NSView _drawRect:clip:] + 2128
4   com.apple.AppKit         	0x9376499c -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] + 736
5   com.apple.WebKit         	0x00361b74 -[WebHTMLView(WebPrivate) _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] + 456 (WebHTMLView.m:823)
6   com.apple.AppKit         	0x9375e3f4 -[NSView _displayRectIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:] + 384
7   com.apple.AppKit         	0x937536e8 -[NSView displayIfNeeded] + 248
8   com.apple.AppKit         	0x9385a7d8 -[NSClipView _immediateScrollToPoint:] + 1384
9   com.apple.PDFKit         	0x9611edf0 -[PDFView constrainedScrollToPoint:] + 184
10  com.apple.PDFKit         	0x9611b134 -[PDFView reflectNewPageOn] + 780
11  com.apple.PDFKit         	0x96123fb4 -[PDFView drawRect:] + 116
12  com.apple.AppKit         	0x93765bf8 -[NSView _drawRect:clip:] + 2128
13  com.apple.AppKit         	0x937651b8 -[NSView _recursiveDisplayAllDirtyWithLockFocus:visRect:] + 404
14  com.apple.AppKit         	0x93767f00 _recursiveDisplayInRect2 + 84
15  com.apple.CoreFoundation 	0x907f33c4 CFArrayApplyFunction + 416
16  com.apple.AppKit         	0x937652cc -[NSView _recursiveDisplayAllDirtyWithLockFocus:visRect:] + 680
17  com.apple.AppKit         	0x93767f00 _recursiveDisplayInRect2 + 84
18  com.apple.CoreFoundation 	0x907f33c4 CFArrayApplyFunction + 416
19  com.apple.AppKit         	0x937652cc -[NSView _recursiveDisplayAllDirtyWithLockFocus:visRect:] + 680
20  com.apple.AppKit         	0x93767f00 _recursiveDisplayInRect2 + 84
21  com.apple.CoreFoundation 	0x907f33c4 CFArrayApplyFunction + 416
22  com.apple.AppKit         	0x937652cc -[NSView _recursiveDisplayAllDirtyWithLockFocus:visRect:] + 680
23  com.apple.AppKit         	0x93764780 -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] + 196
24  com.apple.AppKit         	0x93764d48 -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] + 1676
25  com.apple.AppKit         	0x9375e3f4 -[NSView _displayRectIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:] + 384
26  com.apple.AppKit         	0x9383e2d0 -[NSView displayIfNeededInRectIgnoringOpacity:] + 264
27  com.apple.AppKit         	0x9383e1b4 -[NSView displayRectIgnoringOpacity:] + 104
28  com.apple.WebCore        	0x019a404c WebCore::Widget::paint(WebCore::GraphicsContext*, WebCore::IntRect const&) + 516 (WidgetMac.mm:369)
29  com.apple.WebCore        	0x01a64fbc WebCore::RenderWidget::paint(WebCore::RenderObject::PaintInfo&, int, int) + 664 (RenderWidget.cpp:194)
30  com.apple.WebCore        	0x018d0ca8 WebCore::RenderBox::paint(WebCore::RenderObject::PaintInfo&, int, int) + 168 (RenderBox.cpp:268)
31  com.apple.WebCore        	0x018bf400 WebCore::RenderBlock::paintChildren(WebCore::RenderObject::PaintInfo&, int, int) + 820 (RenderBlock.cpp:1279)
32  com.apple.WebCore        	0x018c95a4 WebCore::RenderBlock::paintObject(WebCore::RenderObject::PaintInfo&, int, int) + 480 (RenderBlock.cpp:1335)
33  com.apple.WebCore        	0x018bef64 WebCore::RenderBlock::paint(WebCore::RenderObject::PaintInfo&, int, int) + 616 (RenderBlock.cpp:1254)
34  com.apple.WebCore        	0x018fbf80 WebCore::RenderLayer::paintLayer(WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::IntRect const&, bool, bool, WebCore::RenderObject*) + 1376 (RenderLayer.cpp:1162)
35  com.apple.WebCore        	0x018fc168 WebCore::RenderLayer::paintLayer(WebCore::RenderLayer*, WebCore::GraphicsContext*, WebCore::IntRect const&, bool, bool, WebCore::RenderObject*) + 1864 (RenderLayer.cpp:1185)
36  com.apple.WebCore        	0x018fc278 WebCore::RenderLayer::paint(WebCore::GraphicsContext*, WebCore::IntRect const&, bool, WebCore::RenderObject*) + 72 (RenderLayer.cpp:1068)
37  com.apple.WebCore        	0x017ef788 WebCore::Frame::paint(WebCore::GraphicsContext*, WebCore::IntRect const&) + 724 (Frame.cpp:2815)
38  com.apple.WebCore        	0x018357b8 -[WebCoreFrameBridge drawRect:] + 648 (DeprecatedPtrList.h:908)
39  com.apple.WebKit         	0x0036cff0 -[WebHTMLView drawSingleRect:] + 588 (WebHTMLView.m:2595)
40  com.apple.WebKit         	0x0036d348 -[WebHTMLView drawRect:] + 536 (WebHTMLView.m:2643)
41  com.apple.AppKit         	0x93765bf8 -[NSView _drawRect:clip:] + 2128
42  com.apple.AppKit         	0x9376499c -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] + 736
43  com.apple.WebKit         	0x00361b74 -[WebHTMLView(WebPrivate) _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] + 456 (WebHTMLView.m:823)
44  com.apple.AppKit         	0x93764d48 -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] + 1676
45  com.apple.AppKit         	0x93764d48 -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] + 1676
46  com.apple.AppKit         	0x93764d48 -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] + 1676
47  com.apple.AppKit         	0x93764d48 -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] + 1676
48  com.apple.AppKit         	0x93764d48 -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] + 1676
49  com.apple.AppKit         	0x93764d48 -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] + 1676
50  com.apple.AppKit         	0x93764d48 -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] + 1676
51  com.apple.AppKit         	0x937853e4 -[NSThemeFrame _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] + 192
52  com.apple.AppKit         	0x9375e3f4 -[NSView _displayRectIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:] + 384
53  com.apple.AppKit         	0x937536e8 -[NSView displayIfNeeded] + 248
54  com.apple.AppKit         	0x93753558 -[NSWindow displayIfNeeded] + 180
55  com.apple.Safari         	0x0001ac30 0x1000 + 105520
56  com.apple.AppKit         	0x93753404 _handleWindowNeedsDisplay + 200
57  com.apple.CoreFoundation 	0x907e373c __CFRunLoopDoObservers + 352
58  com.apple.CoreFoundation 	0x907e39dc __CFRunLoopRun + 420
59  com.apple.CoreFoundation 	0x907e347c CFRunLoopRunSpecific + 268
60  com.apple.HIToolbox      	0x9321d980 RunCurrentEventLoopInMode + 264
61  com.apple.HIToolbox      	0x9321d014 ReceiveNextEventCommon + 380
62  com.apple.HIToolbox      	0x9321ce80 BlockUntilNextEventMatchingListInMode + 96
63  com.apple.AppKit         	0x9371fe84 _DPSNextEvent + 384
64  com.apple.AppKit         	0x9371fb48 -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 116
65  com.apple.Safari         	0x00006df4 0x1000 + 24052
66  com.apple.AppKit         	0x9371c08c -[NSApplication run] + 472
67  com.apple.AppKit         	0x9380cbfc NSApplicationMain + 452

Happened to me a few times a couple of weeks ago, but then I was unable to reproduce.

[Darin Adler]

I tried and could not reproduce.

[Trey Matteson]

I updated to rev 14741 tonight and did a fresh build and I still see it.   I bet this is the key:

fastmac:/<1>Whopper/WebKit: defaults read com.apple.Safari | grep PDF
    WebKitPDFDisplayMode = 1; 
    WebKitPDFScaleFactor = 1.681763; 

I removed the scale factor default, and the crash went away.  I believe this setting is remembered across all PDF loads.

[Alice Liu]

<rdar://problem/4575267>

[Alice Liu]

Trey, I added WebKitPDFDisplayMode and WebKitPDFScaleFactor at those values to my safari plist, and I was unable to reproduce the bug.  Is this still happening for you?  Did you try to replace the scale factor value after you removed it to see if it crashed?  

[Alexey Proskuryakov]

I could reproduce on 10.4.7 by running TOT as:

run-safari -WebKitPDFDisplayMode 1 -WebKitPDFScaleFactor 1.681763

[Trey Matteson]

Yes, still crashes on 10.4.6 with TOT as of a couple days ago.  

[Maciej Stachowiak]

*** Bug 9140 has been marked as a duplicate of this bug. ***

The assertion fails because the WebHTMLView in stack frame 5 is the same WebHTMLView from stack frame 43, i.e. -_recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView: is being reentered. The reason for that is apparently that the NSClipView (stack frame 8) calls -displayIfNeeded on its opaqueAncestor, which is the WebHTMLView's superview (a WebClipView).

Does the base class implementation of -[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:] have some reentrancy protection that WebHTMLView lacks?

For what it's worth, the outer call (in stack frame 43) has isVisibleRect:YES and topView:NO while the inner call has the opposite flags. The visibleView is different, too.

[John Sullivan]

If this has no obvious symptom on a release build, it should be demoted to p2 at least.

[Trey Matteson]

I bet the problem is not that various layers need reentrancy protection, but PDFView is doing something stupid/wrong by calling display from within its drawRect method.

I don't remember if this led to actual bad behavior on a release build.  I would guess yes, but that's just a guess.

[John Sullivan]

mitzpettel couldn't observe any actual bad behavior on a release build. I'm sure Trey's right that the problem is PDFKit's bad behavior. We could avoid the bad behavior by removing some of the code we wrote to make PDFs integrate better with WebKit, but I'd rather not to do that unless we have proof that it would prevent something bad in a release build.

Having a PDF as the source of a subframe should be quite rare on the internet, also, though of course it could be more common in local networks where everyone's using a browser that displays PDFs inline.

Demoting to P2/Major.

[Cameron Zwarich (cpst)]

This is fixed now, at least with ToT on Leopard. Should I add a layout test for it?

[Eric Seidel (no email)]

I"m not sure that this ancient bug is still valid.  However there is a new bug with a similar stack: bug 36930.