Bug 62163

Summary: [chromium] PlatformContextSkia leaks SkDeviceFactory
Product: WebKit Reporter: Alok Priyadarshi <alokp>
Component: CanvasAssignee: Alok Priyadarshi <alokp>
Status: RESOLVED FIXED    
Severity: Normal CC: mdelaney7, reed, senorblanco, vangelis, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
proposed patch
none
proposed patch none

Alok Priyadarshi
Reported 2011-06-06 15:59:09 PDT
I found the leak while debugging a crash in PlatformContextSkia::setSharedGraphicsContext3D. Note that the factory created here: http://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/skia/PlatformContextSkia.cpp?rev=86182#L745 is never unref'd. chromium bug: http://code.google.com/p/chromium/issues/detail?id=84758
Attachments
proposed patch (2.75 KB, patch)
2011-06-06 16:09 PDT, Alok Priyadarshi 		no flags
DetailsFormatted DiffDiff
proposed patch (1.46 KB, patch)
2011-06-07 12:18 PDT, Alok Priyadarshi 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Alok Priyadarshi
Comment 1 2011-06-06 16:09:07 PDT
Created attachment 96146 [details] proposed patch
Mike Reed
Comment 2 2011-06-07 04:55:20 PDT
Seems ok, though I think a 1-liner fix could also just be from m_canvas->setDeviceFactory(factory); to m_canvas->setDeviceFactory(factory)->unref(); This matches the skia pattern, and matches the line above for the device.
Alok Priyadarshi
Comment 3 2011-06-07 07:53:31 PDT
(In reply to comment #2) > Seems ok, though I think a 1-liner fix could also just be > > from > m_canvas->setDeviceFactory(factory); > to > m_canvas->setDeviceFactory(factory)->unref(); > > This matches the skia pattern, and matches the line above for the device. Either way is fine with me. I used auto-ptr because there was a bunch of TODOs in the code to that effect and also the skia pattern does not work in case of GrTexture.
Alok Priyadarshi
Comment 4 2011-06-07 12:18:30 PDT
Created attachment 96280 [details] proposed patch Went with the one-line fix. This is probably not the right time make unnecessary changes.
Stephen White
Comment 5 2011-06-07 12:20:50 PDT
Comment on attachment 96280 [details] proposed patch Looks good. R=me.
WebKit Review Bot
Comment 6 2011-06-07 13:29:06 PDT
Comment on attachment 96280 [details] proposed patch Clearing flags on attachment: 96280 Committed r88258: <http://trac.webkit.org/changeset/88258>
WebKit Review Bot
Comment 7 2011-06-07 13:29:11 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.