934001 - ImageView's setImageBitmap is evil

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Theme and Visual Design, defect)

Description

[Sriram Ramasubramanian [:sriram]]

Calling setImageBitmap(null) creates BitmapDrawables, BitmapDrawable$State, and associated Paint objects. That's so much memory for not showing anything! :O

Comment 1

[Sriram Ramasubramanian [:sriram]]

Attachment: Patch

Feels so light.

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

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

We should find a way to get this kind of code change documented so other people remember it. Maybe a simple post to mobile-dev-firefox? Just a short paragraph on why setImageBitmap(null) is not as good as setImageDrawbale(null).

I thought there was another issue with setImageBitmap(null)/setImageDrawable(null) too?

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

I also found this tidbit:
https://bugzilla.mozilla.org/show_bug.cgi?id=923218#c8

Comment 4

[Sriram Ramasubramanian [:sriram]]

https://hg.mozilla.org/integration/fx-team/rev/04066e075f59

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/04066e075f59

Comment 6

[Richard Newman [:rnewman]]

Do we want to uplift this? Seems trivial.

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

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

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Favicon refactor
User impact if declined: Memory and jank issues
Testing completed (on m-c, etc.): It's been on m-c for a while
Risk to taking this patch (and alternatives if risky): low, low risk
String or IDL/UUID changes made by this patch: none

Comment 8

[Kevin Brosnan [Ex-Mozilla]]

Aurora is 28 as of today. Is this good enough to take for beta?

Comment 9

[Richard Newman [:rnewman]]

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

IMO, yes. Totally safe one-liner.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

rnewman landed this.
https://hg.mozilla.org/releases/mozilla-beta/rev/10ecc8f4258e