Bug 37954 - Occasional crash in CoreGraphics when using accelerated compositing in Windows.
Summary: Occasional crash in CoreGraphics when using accelerated compositing in Windows.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows XP
: P2 Critical
Assignee: Andy Estes
URL:
Keywords: InRadar
Depends on: 37952
Blocks:
  Show dependency treegraph
 
Reported: 2010-04-21 14:52 PDT by Andy Estes
Modified: 2010-04-22 08:14 PDT (History)
0 users

See Also:


Attachments
patch (8.29 KB, patch)
2010-04-21 20:56 PDT, Andy Estes
mjs: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andy Estes 2010-04-21 14:52:10 PDT
When viewing pages rendered in accelerated compositing mode using the Windows port, there is occasionally a crash in the underlying CoreGraphics library used by WebKit on Windows.  Methods to repeat the crash include resizing the browser window, repeatedly changing focus between the browser and another application, and dragging items into the WebView.  The crash appears to be intermittent.
Comment 1 Andy Estes 2010-04-21 14:52:33 PDT
<rdar://problem/7806666>
Comment 2 Andy Estes 2010-04-21 14:54:33 PDT
Diagnosis from Steve Falkenburg:

This CGDataProviderRef pulls its data from m_backingStoreBitmap. Note this is deleted on a timer after a period of inactivity. I'm thinking this may be the cause of the bug. If the m_backingStoreBitmap is freed and then the WKCACFLayerRenderer::renderTimerFired(Timer<WKCACFLayerRenderer>*) timer fires, we will crash. One possible easy fix would be to protect the call to deleteBackingStoreSoon() if isAcceleratedCompositing() returns true in WebView::paint(). I haven't tested this theory (or fix).
Comment 3 Andy Estes 2010-04-21 20:56:49 PDT
Created attachment 54020 [details]
patch
Comment 4 Andy Estes 2010-04-21 21:02:57 PDT
Here is a description of a solution from Adam Roben, which is what I implemented in the patch:

I mentioned to Steve that one way to fix the problem with deleteBackingStoreSoon() is to add a RefCounted wrapper around an HBITMAP, and use that to hold the WebView's backing store bitmap. Then we can ref that wrapper object in updateRootLayerContents and call CGDataProviderCreateWithData (not CreateWithCFData), passing the wrapper object as the info pointer and a function that just calls deref() on the wrapper as the release callback. That should allow the bitmap to live as long as the CGImageRef needs it.
Comment 5 Maciej Stachowiak 2010-04-21 23:51:30 PDT
Comment on attachment 54020 [details]
patch

r=me

Nice fix.
Comment 6 Andy Estes 2010-04-22 00:13:09 PDT
Committed revision 58067.  Thanks Maciej!
Comment 7 Adam Roben (:aroben) 2010-04-22 08:14:24 PDT
Comment on attachment 54020 [details]
patch

Nice fix!