Bug 7951 – REGRESSION: Safari crashes when printing a google map w/directions

• Status: RESOLVED
• Resolution: FIXED
• Component: Printing

[Trey Matteson]

Go to the URL above.  Print.  Hit Preview.  Crash with the backtrace below.

The bug is a little finnicky to repro because it relies on reuse of some freed memory.  On a different version of the app/OS, you may need to try a different source/destination for the directions, or try printing a few times to get it to croak.

#0  0x01b29f70 in WebCore::DataRef<WebCore::StyleSurroundData>::operator-> (this=0x1c) at /Volumes/Whopper/WebKit/WebCore/rendering/DataRef.h:54
#1  0x01b2a08c in WebCore::RenderStyle::borderRight (this=0x0) at /Volumes/Whopper/WebKit/WebCore/rendering/render_style.h:1132
#2  0x019e29bc in WebCore::RenderTableCell::collapsedLeftBorder (this=0x122dd55c, rtl=false) at /Volumes/Whopper/WebKit/WebCore/rendering/RenderTableCell.cpp:238
#3  0x019e3d6c in WebCore::RenderTableCell::borderLeft (this=0x122dd55c) at /Volumes/Whopper/WebKit/WebCore/rendering/RenderTableCell.cpp:478
#4  0x019626f8 in WebCore::RenderBlock::calcMinMaxWidth (this=0x122dd55c) at /Volumes/Whopper/WebKit/WebCore/rendering/RenderBlock.cpp:2709
#5  0x019e5838 in WebCore::RenderTableCell::calcMinMaxWidth (this=0x122dd55c) at /Volumes/Whopper/WebKit/WebCore/rendering/RenderTableCell.cpp:88
#6  0x019ba488 in WebCore::RenderObject::recalcMinMaxWidths (this=0x122dd55c) at /Volumes/Whopper/WebKit/WebCore/rendering/RenderObject.cpp:2284
#7  0x019ba2a0 in WebCore::RenderObject::recalcMinMaxWidths (this=0x1260482c) at /Volumes/Whopper/WebKit/WebCore/rendering/RenderObject.cpp:2269
#8  0x019ba2a0 in WebCore::RenderObject::recalcMinMaxWidths (this=0x1260315c) at /Volumes/Whopper/WebKit/WebCore/rendering/RenderObject.cpp:2269
#9  0x019ba2a0 in WebCore::RenderObject::recalcMinMaxWidths (this=0x122fa5ac) at /Volumes/Whopper/WebKit/WebCore/rendering/RenderObject.cpp:2269
#10 0x019e7f0c in WebCore::RenderTable::recalcMinMaxWidths (this=0x122fa5ac) at /Volumes/Whopper/WebKit/WebCore/rendering/RenderTable.cpp:552
#11 0x019ba2a0 in WebCore::RenderObject::recalcMinMaxWidths (this=0x122aa49c) at /Volumes/Whopper/WebKit/WebCore/rendering/RenderObject.cpp:2269
#12 0x019ba2a0 in WebCore::RenderObject::recalcMinMaxWidths (this=0x124b1eec) at /Volumes/Whopper/WebKit/WebCore/rendering/RenderObject.cpp:2269
#13 0x019ba2a0 in WebCore::RenderObject::recalcMinMaxWidths (this=0x1249ec5c) at /Volumes/Whopper/WebKit/WebCore/rendering/RenderObject.cpp:2269
#14 0x019ba2a0 in WebCore::RenderObject::recalcMinMaxWidths (this=0x122a208c) at /Volumes/Whopper/WebKit/WebCore/rendering/RenderObject.cpp:2269
#15 0x019ba2a0 in WebCore::RenderObject::recalcMinMaxWidths (this=0x106e32bc) at /Volumes/Whopper/WebKit/WebCore/rendering/RenderObject.cpp:2269
#16 0x019ba2a0 in WebCore::RenderObject::recalcMinMaxWidths (this=0x1247474c) at /Volumes/Whopper/WebKit/WebCore/rendering/RenderObject.cpp:2269
#17 0x0197b99c in WebCore::RenderCanvas::layout (this=0x1247474c) at /Volumes/Whopper/WebKit/WebCore/rendering/RenderCanvas.cpp:112
#18 0x0189fff4 in WebCore::FrameView::layout (this=0xd46da10) at /Volumes/Whopper/WebKit/WebCore/page/FrameView.cpp:415
#19 0x01879084 in WebCore::Frame::forceLayout (this=<incomplete type>) at /Volumes/Whopper/WebKit/WebCore/page/Frame.cpp:2925
#20 0x018c2f78 in -[WebCoreFrameBridge forceLayoutAdjustingViewSize:] (self=<incomplete type>, _cmd=0x90a1380c, flag=1 '\001') at /Volumes/Whopper/WebKit/WebCore/kwq/DeprecatedPtrList.h:891
#21 0x0037a5f0 in -[WebHTMLView layoutToMinimumPageWidth:maximumPageWidth:adjustingViewSize:] (self=0x122ae230, _cmd=0x90a1f200, minPageWidth=0, maxPageWidth=0, adjustViewSize=1 '\001') at /Volumes/Whopper/WebKit/WebKit/WebView/WebHTMLView.m:2306
#22 0x0037f1c8 in -[WebHTMLView _setPrinting:minimumPageWidth:maximumPageWidth:adjustViewSize:] (self=0x122ae230, _cmd=0x909f87f4, printing=0 '\0', minPageWidth=0, maxPageWidth=0, adjustViewSize=1 '\001') at /Volumes/Whopper/WebKit/WebKit/WebView/WebHTMLView.m:3060
#23 0x0037f774 in -[WebHTMLView _endPrintMode] (self=0x122ae230, _cmd=0x414a84) at /Volumes/Whopper/WebKit/WebKit/WebView/WebHTMLView.m:3128
#24 0x003801e0 in -[WebHTMLView endDocument] (self=0x122ae230, _cmd=0x90a115d0) at /Volumes/Whopper/WebKit/WebKit/WebView/WebHTMLView.m:3241

[Trey Matteson]

Created attachment 7267 [details]
partial reduction

[Trey Matteson]

I created an ugly "reduction" for this bug, which amounted to some de-obfuscation of the page, commenting out certain parts, and NOP'ing some elements by adding "xx" to their names.  The main goal was to reduce the number of table cells, since they were at the heart of the bug.  The ugly techniques stemmed from the fact that when I actually removed code the bug would change places, presumably because the heap access sequence would change.

[Trey Matteson]

I believe this is a regression from stock 10.4.5 (at least I can't make it crash).  It would be interesting to hear from an expert in the code if there is any idea why this would have regressed.

[Chris Petersen]

I can't reproduce this issue under 10.4.5 with WebKit-SVN-r13461. 

Trey: Does this still happen for you with the latest Nightly build ?

[Trey Matteson]

Created attachment 7269 [details]
proposed patch

[Trey Matteson]

Hmm, no, it doesn't happen for me on TOT.  On the other hand, I believe I found a clear logic error in the code, whose effects I observed in gdb, and which when fixed did definitely make the problem go away (all that before I updated to TOT).  I will consult the table experts as to whether another related fix went in recently, and whether my change makes sense.

The bug was so sensitive to the conditions of heap usage that it's quite possible that the change I got when I updated masked it.

[Trey Matteson]

More info on why this now works on TOT:  I tried running with MallocScribble, and still did not see a crash.  I then debugged, and found a big difference from the code path I was previously seeing.  During the relayouts during printing, we no longer do a full decent of recalcMinMaxWidths() through the render tree.  The topmost block has m_recalcMinMax false, so we never get into the recursion that was previously running into the freed table cells.

I believe my proposed change is still an improvement in safety, but I don't know the conditions now needed to force this issue to cause a crash.

[Dave Hyatt]

Comment on attachment 7269 [details]
proposed patch

I'd like to understand this problem a bit better.  recalcMinMaxWidths is supposed to just be the generic non-virtual workhorse that calls the virtual calcMinMaxWidth methods.

Could the cells not force a section recalc when needed?  

It's also not clear to me what causes this problem, since I thought the recalc was done when cells got added/removed.  Is printing causing a change in the cell composition of the table?

[Dave Hyatt]

Comment on attachment 7269 [details]
proposed patch

r=me

[Dave Hyatt]

Comment on attachment 7269 [details]
proposed patch

Sorry to flip-flop.  I do think doing this in the cell is slightly better because

(a) You avoid making recalcMinMaxWidths virtual

and

(b) Your current patch does the section recalc even when the table may not need to recompute its min/max width (or cells may not etc.)

[Trey Matteson]

Created attachment 7290 [details]
patch incorporating review comments

Same idea, this time with the cells requesting that the sections be made up to date.

[Dave Hyatt]

Comment on attachment 7290 [details]
patch incorporating review comments

r=me

[Darin Adler]

Landed back on 2006-04-04.