Closed Bug 887196 Opened 11 years ago Closed 10 years ago

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

Categories

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

ARM
Gonk (Firefox OS)
defect

Tracking

(blocking-b2g:-)

RESOLVED INVALID
blocking-b2g -

People

(Reporter: julienw, Unassigned)

References

Details

(Keywords: perf, Whiteboard: [c=progress p= s= u=])

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.
blocking-b2g: --- → leo?
Triage: We're waiting on Julien's needinfo response on bug 876467 before deciding whether to leo+ this bug.
Keywords: perf
Whiteboard: [c= ]
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.
Assignee: nobody → mbudzynski
Summary: [sms][mms] lazy display the attachments and/or the content as we scroll → [sms][mms] lazy display the attachments the content as we scroll
Depends on: messaging-perf
No longer depends on: 876467
No longer depends on: messaging-perf
We discussed this in triage today and the decision was that we'd like more concrete evidence of improvement here before blocking on it.
blocking-b2g: leo? → -
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.
We should divide this one into more patches, I'll create bugs about this and attach them to the meta bug.
Blocks: 868190
No longer blocks: 868190
Blocks: 882094
Depends on: 889899
Depends on: 889902
No longer blocks: 882094
Michal, could you please attach your work in progress ?
Flags: needinfo?(mbudzynski)
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.
Flags: needinfo?(mbudzynski)
Status: NEW → ASSIGNED
Michal, do you think you could finish this work when you're finished with other stuff ?
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.
Whiteboard: [c= ] → [c= p= s= u=]
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.
Assignee: mbudzynski → nobody
Priority: -- → P3
Summary: [sms][mms] lazy display the attachments the content as we scroll → [sms][mms] remove lazy-loaded images once content is off screen
Whiteboard: [c= p= s= u=] → [c=progress p= s= u=]
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
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.