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)
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.
Reporter | ||
Updated•11 years ago
|
blocking-b2g: --- → leo?
Comment 1•11 years ago
|
||
Triage: We're waiting on Julien's needinfo response on bug 876467 before deciding whether to leo+ this bug.
Keywords: perf
Whiteboard: [c= ]
Reporter | ||
Comment 2•11 years ago
|
||
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
Reporter | ||
Updated•11 years ago
|
Reporter | ||
Updated•11 years ago
|
Blocks: messaging-perf
No longer depends on: messaging-perf
Comment 3•11 years ago
|
||
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? → -
Reporter | ||
Comment 4•11 years ago
|
||
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•11 years ago
|
||
We should divide this one into more patches, I'll create bugs about this and attach them to the meta bug.
Reporter | ||
Comment 6•11 years ago
|
||
Michal, could you please attach your work in progress ?
Flags: needinfo?(mbudzynski)
Comment 7•11 years ago
|
||
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)
Updated•11 years ago
|
Status: NEW → ASSIGNED
Reporter | ||
Comment 8•11 years ago
|
||
Michal, do you think you could finish this work when you're finished with other stuff ?
Comment 9•11 years ago
|
||
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.
Updated•11 years ago
|
Whiteboard: [c= ] → [c= p= s= u=]
Reporter | ||
Comment 10•11 years ago
|
||
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
Updated•11 years ago
|
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=]
Reporter | ||
Comment 11•10 years ago
|
||
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.
Description
•