Bug 62163 – [chromium] PlatformContextSkia leaks SkDeviceFactory

• Status: RESOLVED
• Resolution: FIXED
• Component: Canvas

[Alok Priyadarshi]

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

[Alok Priyadarshi]

Created attachment 96146 [details]
proposed patch

[Mike Reed]

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]

(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]

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 on attachment 96280 [details]
proposed patch

Looks good.  R=me.

[WebKit Review Bot]

Comment on attachment 96280 [details]
proposed patch

Clearing flags on attachment: 96280

Committed r88258: <http://trac.webkit.org/changeset/88258>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.