802491 - ScrollbarLayer's direct-allocation buffers get leaked

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Toolbar, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

Attachment: Patch

Found this while testing pushkar's patch for bug 779511. The BufferedCairoImage class doesn't accept ownership of the direct-allocation buffer passed in during construction or via the setBuffer call (i.e. it doesn't set the mNeedToFreeBuffer boolean and therefore doesn't free the buffer). However, all users of BufferedCairoImage assume that it takes ownership of the buffer, and so they don't free it either! Therefore the buffer gets leaked. Thankfully this only affects the ScrollbarLayer instances by default since TextLayer is never instantiated unless the frame rate pref is enabled, and PluginLayer passes in a null buffer.

Attached patch basically makes BufferedCairoImage always accept ownership of the buffer, which removes the guesswork from the caller code and makes it behave consistently (from one point of view at least).

Comment 1

[Chris Lord [:cwiiis]]

Comment on attachment 672184 [details] [diff] [review]
Patch

Review of attachment 672184 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me, I assume that all users of this class have been checked.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/integration/mozilla-inbound/rev/078e92572b63

Comment 3

[Kartikaya Gupta (email:[e-mail])]

For future reference, I had pushed this to try along with the patch from bug 802495, try run is at https://tbpl.mozilla.org/?tree=Try&rev=766f2d373c6c

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/078e92572b63