Bug 63305 – [chromium] Divide by zero in TilingData

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Adrienne Walker]

There's a divide by zero showing up in Chromium crash reports: http://code.google.com/p/chromium/issues/detail?id=87153

This can only happen when there are no tiles because the layer size is zero.  I can see what could trigger this in code, but don't understand what state of events could cause this.  I'll upload a patch to prevent this crash, because we probably need that safety check anyway.

Callstack:

Thread 0 *CRASHED* ( EXCEPTION_INT_DIVIDE_BY_ZERO @ 0x610241a9 )
0x610241a9 	[chrome.dll 	- tilingdata.cpp:99] 	WebCore::TilingData::tileBounds(int)
0x6102420c 	[chrome.dll 	- tilingdata.cpp:112] 	WebCore::TilingData::tileBoundsWithBorder(int)
0x61032f95 	[chrome.dll 	- layertilerchromium.cpp:185] 	WebCore::LayerTilerChromium::tileLayerRect(WebCore::LayerTilerChromium::Tile const *)
0x61032cc7 	[chrome.dll 	- layertilerchromium.cpp:117] 	WebCore::LayerTilerChromium::createTile(int,int)
0x6103318e 	[chrome.dll 	- layertilerchromium.cpp:247] 	WebCore::LayerTilerChromium::prepareToUpdate(WebCore::IntRect const &)
0x619dd6f3 	[chrome.dll 	- contentlayerchromium.cpp:115] 	WebCore::ContentLayerChromium::paintContentsIfDirty(WebCore::IntRect const &)
0x61029b4b 	[chrome.dll 	- layerrendererchromium.cpp:430] 	WebCore::LayerRendererChromium::paintLayerContents(WTF::Vector<WTF::RefPtr<WebCore::CCLayerImpl>,0> const &)
0x610298ff 	[chrome.dll 	- layerrendererchromium.cpp:328] 	WebCore::LayerRendererChromium::updateLayers(WTF::Vector<WTF::RefPtr<WebCore::CCLayerImpl>,0> &)
0x61029650 	[chrome.dll 	- layerrendererchromium.cpp:239] 	WebCore::LayerRendererChromium::updateAndDrawLayers()
0x611a04e2 	[chrome.dll 	- webviewimpl.cpp:2547] 	WebKit::WebViewImpl::doComposite()
0x6119e02c 	[chrome.dll 	- webviewimpl.cpp:1148] 	WebKit::WebViewImpl::composite(bool)

[Adrienne Walker]

Created attachment 98449 [details]
Patch

[Hajime Morrita]

Comment on attachment 98449 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=98449&action=review

> Source/WebCore/platform/graphics/chromium/LayerTilerChromium.cpp:277
> +    if (!m_tilingData.totalSizeX() || !m_tilingData.totalSizeY() || m_updateRect.isEmpty() || !numTiles())

It looks numTiles() check is redundant because m_updateRect is empty. Is that right?
r+ anyway because being careful is no bad...

[Adrienne Walker]

(In reply to comment #2)
> (From update of attachment 98449 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=98449&action=review
> 
> > Source/WebCore/platform/graphics/chromium/LayerTilerChromium.cpp:277
> > +    if (!m_tilingData.totalSizeX() || !m_tilingData.totalSizeY() || m_updateRect.isEmpty() || !numTiles())
> 
> It looks numTiles() check is redundant because m_updateRect is empty. Is that right?
> r+ anyway because being careful is no bad...

You're quite right.  I just figured that an extra sanity check wouldn't hurt while I was making a similar change.

[Adrienne Walker]

Created attachment 99113 [details]
Patch

[Adrienne Walker]

(In reply to comment #4)
> Created an attachment (id=99113) [details]
> Patch

I'm uploading another patch, but this should be a quick review.

I took a look at a potential repro case for this issue from a bug that was duped into the linked Chromium bug.  However, I couldn't repro it locally.  It looked a lot like an issue I'd seen before, where doing a WebKit paint operation turned off compositing.  This bug could be triggered because the updateRect wasn't being cleared in reset.

I would like to leave the other safety checks in place, however, as they are still relevant even if the reset fix might remove the current root cause.

[James Robinson]

Comment on attachment 99113 [details]
Patch

OK

[Adrienne Walker]

Committed r90029: <http://trac.webkit.org/changeset/90029>