Bug 37954

Summary: Occasional crash in CoreGraphics when using accelerated compositing in Windows.
Product: WebKit Reporter: Andy Estes <aestes>
Component: Layout and RenderingAssignee: Andy Estes <aestes>
Status: RESOLVED FIXED    
Severity: Critical Keywords: InRadar
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows XP   
Bug Depends on: 37952    
Bug Blocks:    
Attachments:
Description Flags
patch mjs: review+

Andy Estes
Reported 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.
Attachments
patch (8.29 KB, patch)
2010-04-21 20:56 PDT, Andy Estes
mjs: review+
Andy Estes
Comment 1 2010-04-21 14:52:33 PDT
Andy Estes
Comment 2 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).
Andy Estes
Comment 3 2010-04-21 20:56:49 PDT
Andy Estes
Comment 4 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.
Maciej Stachowiak
Comment 5 2010-04-21 23:51:30 PDT
Comment on attachment 54020 [details] patch r=me Nice fix.
Andy Estes
Comment 6 2010-04-22 00:13:09 PDT
Committed revision 58067. Thanks Maciej!
Adam Roben (:aroben)
Comment 7 2010-04-22 08:14:24 PDT
Comment on attachment 54020 [details] patch Nice fix!
Note You need to log in before you can comment on or make changes to this bug.