[Messages] We never revoke the blob URL for the attachments

RESOLVED FIXED in 2.1 S8 (7Nov)

Status

defect
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: julienw, Assigned: azasypkin)

Tracking

unspecified
2.1 S8 (7Nov)
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.2+, b2g-v2.0 unaffected, b2g-v2.0M unaffected, b2g-v2.1 unaffected, b2g-v2.2 fixed)

Details

(Whiteboard: [p=1])

Attachments

(1 attachment)

Reporter

Description

5 years ago
Since bug 983172 landed, we now leak the blobs used for images. This is quite bad.

* We'll leak when we display a thread containing MMS with images and then going back to the thread list
* We'll leak when sending a MMS with an image. We'll leak both the resized image and the original image.
triage: regression
blocking-b2g: 2.2? → 2.2+
Reporter

Updated

5 years ago
Whiteboard: [p=1]
Reporter

Updated

5 years ago
Target Milestone: --- → 2.1 S8 (7Nov)
Assignee: nobody → azasypkin
Status: NEW → ASSIGNED
Hey Julien,

In this patch I revoke object URL for the following cases:

* Composer: when attachment is removed (except for the case when it's removed with backspace, noted that on Github);
* Composer: when attachment is replaced;
* Composer: when composer is cleared (like in Send Message or Go Back case);
* ThreadUI: when user leaves ThreadUI panel.

Thanks!
Attachment #8513727 - Flags: review?(felash)
Reporter

Comment 3

5 years ago
Comment on attachment 8513727 [details] [review]
GitHub pull request URL

r=me with the nit in ThreadUI that is unnecessary in my opinion. Please argument if you think otherwise :)
Attachment #8513727 - Flags: review?(felash) → review+
(In reply to Julien Wajsberg [:julienw] from comment #3)
> Comment on attachment 8513727 [details] [review]
> GitHub pull request URL
> 
> r=me with the nit in ThreadUI that is unnecessary in my opinion. Please
> argument if you think otherwise :)

Thanks for review! Added clarifying code comment as we agreed.

Master: https://github.com/mozilla-b2g/gaia/commit/4aedb73eef6eea1a6282d359b807b22b287ff269
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Comment 5

5 years ago
there is no STR's for this issue, and not enough information to verify if this is has landed.
QA Whiteboard: [QAnalyst-Triage?] [QAnalyst-verify-]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?] [QAnalyst-verify-] → [QAnalyst-Triage+] [QAnalyst-verify-]
Flags: needinfo?(ktucker)
You need to log in before you can comment on or make changes to this bug.