887196 - [sms][mms] remove lazy-loaded images once content is off screen

Status: RESOLVED INVALID

(Firefox OS Graveyard :: Gaia::SMS, defect, P3)

Description

[Julien Wajsberg [:julienw]]

Right now, we're loading all attachments in a thread when we display it. Also, we're using data url (but this will probably change in Bug 876467).

This is quickly messy if the user scrolls back in history in a thread, because we never undisplay images that have been displayed.

This bug will build on top of Bug 876467 and will hopefully use David Flanagan's visibility_monitor.js to monitor this. (but maybe that lib is not suitable for that use case).

requesting leo+ for this so that we don't explode the phone in big threads.

Comment 1

[Mike Lee [:mlee]]

Triage: We're waiting on Julien's needinfo response on bug 876467 before deciding whether to leo+ this bug.

Comment 2

[Julien Wajsberg [:julienw]]

Michal is working on this to see if this helps.

Our profile in bug 876467 comment 14 implies that we need to move from iframes to standard div with images, and to actually not render the images until they're displayed, which is quite easy.

However, removing the images once they're scrolled off is probably less easy,but this is a necessity after our tests. So I think leo+ here would be a good idea.

Comment 3

[Andrew Overholt [:overholt]]

We discussed this in triage today and the decision was that we'd like more concrete evidence of improvement here before blocking on it.

Comment 4

[Julien Wajsberg [:julienw]]

Michal, I think we can do the iframe -> div/img change separately in another bug, this would make a smaller patch with big improvements. What do you think ? Maybe the dataurl -> blob change too.

Comment 5

[Michał Budzyński (:michalbe)]

We should divide this one into more patches, I'll create bugs about this and attach them to the meta bug.

Comment 6

[Julien Wajsberg [:julienw]]

Michal, could you please attach your work in progress ?

Comment 7

[Michał Budzyński (:michalbe)]

Yes, it's on my branch michalbe:mms-thumbnails. It fires events when the msg is visible and when it gets out of the screen, so everything what should be done is just add proper listeners in thread_ui for adding/removing the img from the DOM.

Comment 8

[Julien Wajsberg [:julienw]]

Michal, do you think you could finish this work when you're finished with other stuff ?

Comment 9

[Michał Budzyński (:michalbe)]

I'll work on it now, because I'm waiting for #908949 to land, and my other responsibilities are not Comms related so they can wait.

Comment 10

[Julien Wajsberg [:julienw]]

Michal is busy on Haida, so removing assignee.

I see my SMS app crashing when I have big attachments in my threads. Not sure this bug would help this though, maybe we don't free correctly memory in some other part of the code.

Comment 11

[Julien Wajsberg [:julienw]]

Not sure this is still needed:

* we use directly the image stored in the Gecko API DB, so the blob itself does not take much memory
* we use moz-sample-size so we don't create another blob
* gecko discard images' decoded data when it needs to reclaim memory

=> let's close it