Bug 26568 – Repro crash animating GIF if previously used in a closed window's back/forward list

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Alice Liu]

<rdar://problem/6978362>

This bug reproduces in Safari 4 on Windows but not on Mac because on Mac we execute the platformWidget() code paths instead of calling hostWindow().

Steps to repro:
- Safari 4 on any Windows platform
- Set Safari to open new windows with an empty page
- Launch
- Open 2 empty windows
- Navigate to http://www.forum.skoda-club.ru/viewtopic.php?t=38976&postdays=0&postorder=asc&start=15&sid=9d258270773fafd77a27418fb1c8180b
- Navigate in *same* window to about:blank
- close window w/ about:blank
- *QUICKLY* load in second window: http://www.forum.skoda-club.ru/viewtopic.php?t=38976&postdays=0&postorder=asc&start=15&sid=9d258270773fafd77a27418fb1c8180b
---> crash

We delay destruction of the back/forward list, so a cached image can be left with a reference to a client that has a null page.  


STACK_TEXT:  
0018d2f0 6cc29352 0018d3d0 00000000 7df1f704 WebKit!WebCore::ScrollView::repaintContentRectangle+0x2f
0018d358 6cd66586 0018d3d0 00000000 7df1f704 WebKit!WebCore::FrameView::repaintContentRectangle+0x1f2
0018d398 6cbd1319 0018d3d0 00000000 6cbd1413 WebKit!WebCore::RenderView::repaintViewRectangle+0x66
0018d3a4 6cbd1413 7df1f704 0018d3d0 00000000 WebKit!WebCore::RenderObject::repaintUsingContainer+0x39
0018d3e0 6ce2df04 0018d408 00000000 7e1afa80 WebKit!WebCore::RenderObject::repaintRectangle+0x93
0018d480 6ce2bbae 7e1afa80 00000000 7f0276e8 WebKit!WebCore::RenderImage::imageChanged+0x2c4
0018d4a8 6ce2c0b7 00000000 00000001 6ceb9e6c WebKit!WebCore::CachedImage::notifyObservers+0x2e
0018d4b4 6ceb9e6c 7f0276e8 0018f5b8 7f0276e8 WebKit!WebCore::CachedImage::animationAdvanced+0x17
0018d52c 6cebfcfb 00000001 0018f5b8 7f0276e8 WebKit!WebCore::BitmapImage::startAnimation+0x26c
0018d598 6cc46180 0018f5b8 0018d5e0 0018d5d0 WebKit!WebCore::BitmapImage::draw+0x1b
0018d5f0 6cc45c01 41b00000 421c0000 00000002 WebKit!WebCore::GraphicsContext::drawImage+0x170
0018d62c 6cc45b5b 0018f5b8 00000002 00000000 WebKit!WebCore::GraphicsContext::drawImage+0x91
0018d650 6ce2e11d 0018d688 00000000 00000354 WebKit!WebCore::GraphicsContext::drawImage+0x2b
0018d6d4 6cefaed5 0018d750 00000354 00000037 WebKit!WebCore::RenderImage::paintReplaced+0x1cd
0018d72c 6ce8ce44 0018d750 00000354 00000037 WebKit!WebCore::RenderReplaced::paint+0x195
0018d77c 6cef2dcf 0018d7c8 0000032e 00000027 WebKit!WebCore::InlineBox::paint+0xd4
0018d7f4 6cef2dcf 0018d840 0000032e 00000027 WebKit!WebCore::InlineFlowBox::paint+0x40f
0018d86c 6cef2dcf 0018d8b8 0000032e 00000027 WebKit!WebCore::InlineFlowBox::paint+0x40f
0018d8e4 6cec59ea 0018d978 0000032e 00000027 WebKit!WebCore::InlineFlowBox::paint+0x40f
0018d904 6ceba67d 0018d978 0000032e 00000027 WebKit!WebCore::RootInlineBox::paint+0x1a
0018d9a4 6cdfa92f 7e83607c 7e836010 0018da60 WebKit!WebCore::RenderLineBoxList::paint+0x38d
0018d9c8 6cdfad35 0018da60 0000032e 0000032e WebKit!WebCore::RenderBlock::paintContents+0x3f
0018d9fc 6cdfa319 0018da60 0000032e 00000027 WebKit!WebCore::RenderBlock::paintObject+0xe5
0018da3c 6cdfaa7e 0018da60 0000032e 00000027 WebKit!WebCore::RenderBlock::paint+0xd9
0018da88 6cdfa93a 7e9f4308 0018db00 0000032d WebKit!WebCore::RenderBlock::paintChildren+0x13e
0018daa8 6cdfad35 0018dbd8 0000032d 0000032d WebKit!WebCore::RenderBlock::paintContents+0x4a
0018dadc 6cdfa319 0018dbd8 0000032d 00000026 WebKit!WebCore::RenderBlock::paintObject+0xe5
0018db1c 6ce0fca5 0018dbd8 0000032d 00000026 WebKit!WebCore::RenderBlock::paint+0xd9
0018db34 6ce0d2bd 0018dbd8 0000032d 00000026 WebKit!WebCore::RenderTableCell::paint+0x95
0018db8c 6ce0cdd4 0018dbd8 0000032d 00000026 WebKit!WebCore::RenderTableSection::paintObject+0x4cd
0018dbac 6ce09214 0018dbd8 00000300 00000004 WebKit!WebCore::RenderTableSection::paint+0x54
0018dcac 6cef3221 00000000 00000006 7f6b2ee4 WebKit!WebCore::RenderTable::paintObject+0x124
0018dd80 6cef2e06 0018de90 0000031a 00000006 WebKit!WebCore::InlineFlowBox::paintBoxDecorations+0xd1
0018dec0 6cdfabac 0018df78 0018df18 6cdf81f3 WebKit!WebCore::InlineFlowBox::paint+0x446
0018decc 6cdf81f3 00000000 0000031a 0018df78 WebKit!WebCore::RenderBlock::paintCaret+0x5c
0018dfa0 6cdfa93a 7e9f3dc4 0018e000 0000031a WebKit!WebCore::RenderBlock::overflowRect+0x203
0018dfc0 6cdfad35 0018e058 0000031a 0000031a WebKit!WebCore::RenderBlock::paintContents+0x4a
0018dff4 6cdfa319 0018e058 0000031a 00000006 WebKit!WebCore::RenderBlock::paintObject+0xe5
0018e034 6cdfaa7e 0018e058 0000031a 00000006 WebKit!WebCore::RenderBlock::paint+0xd9
0018e080 6cdfa93a 7e9f3d3c 0018e100 0000031a WebKit!WebCore::RenderBlock::paintChildren+0x13e
0018e0a0 6cdfad35 0018e138 0000031a 0000031a WebKit!WebCore::RenderBlock::paintContents+0x4a
0018e0d4 6cdfa319 0018e138 0000031a 00000006 WebKit!WebCore::RenderBlock::paintObject+0xe5
0018e114 6cdfaa7e 0018e138 0000031a 00000006 WebKit!WebCore::RenderBlock::paint+0xd9
0018e160 6cdfa93a 7e9f3c98 0018e200 00000318 WebKit!WebCore::RenderBlock::paintChildren+0x13e
0018e180 6cdfad35 0018e2b0 00000318 00000318 WebKit!WebCore::RenderBlock::paintContents+0x4a
0018e1b4 6cdfa319 0018e2b0 00000318 00000004 WebKit!WebCore::RenderBlock::paintObject+0xe5
0018e1f4 6ce0fca5 0018e2b0 00000318 00000004 WebKit!WebCore::RenderBlock::paint+0xd9
0018e20c 6ce0d2bd 0018e2b0 00000034 00000004 WebKit!WebCore::RenderTableCell::paint+0x95
0018e264 6ce0cdd4 0018e2b0 00000034 00000004 WebKit!WebCore::RenderTableSection::paintObject+0x4cd
0018e284 6ce09214 0018e2b0 00000000 00000004 WebKit!WebCore::RenderTableSection::paint+0x54
0018e370 6fcd4c58 80b395fb 0d292a48 00000001 WebKit!WebCore::RenderTable::paintObject+0x124
0018e3b4 6fcd4c39 00630000 00000000 6fcd4c58 msvcr80!free+0xec
0018e400 80027309 0d298fa8 6e89181d 07718558 msvcr80!free+0xcd

[Alice Liu]

Created attachment 31588 [details]
patch and manual test

Comment on attachment 31588 [details]
patch and manual test

The null checks are good in preventing crashes, but I wonder if it isn’t practical to augment them with ASSERTs, and to add code at a higher level that would stop this crash from happening—one possible place is RenderView::repaintViewRectangle(), but even better would be to change implementations of imageChanged() such that they don’t do any unnecessary work (such as computing a repaint rectangle) when the document is in the back/forward cache.

[Maciej Stachowiak]

Comment on attachment 31588 [details]
patch and manual test

I'm going to r+ notwithstanding Mitz's comments, because I think further improvement to fix the problem at a higher level can be done as a separate patch.

Is it possible to make an automated layout test for this? I believe layout tests have the power to open and navigate additional windows. Let's try to make the test into a fully automated LayoutTest if possible.

[Alice Liu]

The crash requires that the back/forward cache be enabled, which afaik is not enabled in the automated layout tests.

Fixed by Alice in <http://trac.webkit.org/changeset/44908>.