Bug 1157954 causes 8MiB regression in heap-unclassified in AWSY

RESOLVED FIXED in Firefox 40

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: erahm, Assigned: seth)

Tracking

unspecified
mozilla40
Points:
---

Firefox Tracking Flags

(firefox40 fixed)

Details

(Whiteboard: [MemShrink][awsy])

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
+++ This bug was initially created as a clone of Bug #1157954 +++

It looks like about 8MiB of image data is no longer reported and gets lumped into heap-unclassified.
(Reporter)

Comment 1

4 years ago
Created attachment 8600400 [details]
memory before regression
(Reporter)

Comment 2

4 years ago
Created attachment 8600401 [details]
memory after regression
(Assignee)

Comment 3

4 years ago
Looks an awful lot like we're just not reporting source data anymore. It must've gotten busted in bug 1157954.
(Assignee)

Comment 4

4 years ago
Created attachment 8600549 [details] [diff] [review]
Report image source size again in about:memory

Sure enough, we mistakenly stopped reporting image source size in bug 1157954.
This patch makes us report it again.
Attachment #8600549 - Flags: review?(dholbert)
(Assignee)

Updated

4 years ago
Assignee: nobody → seth
Status: NEW → ASSIGNED
Comment on attachment 8600549 [details] [diff] [review]
Report image source size again in about:memory

>diff --git a/image/src/imgLoader.cpp b/image/src/imgLoader.cpp
>-    return ReportSurfaces(aHandleReport, aData, pathPrefix, aCounter);
>+    nsresult rv = ReportSurfaces(aHandleReport, aData, pathPrefix, aCounter);
>+    NS_ENSURE_SUCCESS(rv, rv);
[...]
>   static nsresult ReportValues(nsIHandleReportCallback* aHandleReport,
>                                nsISupports* aData,
>                                const nsACString& aPathPrefix,
>                                const MemoryCounter& aCounter)
>   {
>     nsresult rv;
> 
>+    rv = ReportSourceValue(aHandleReport, aData, aPathPrefix, aCounter);
>     NS_ENSURE_SUCCESS(rv, rv);

Extreme nit: this is inconsistent in "nsresult rv" declaration style (whether it's declared ahead of time vs. declared inline with assignment).

Consider making this consistent, one way or the other.

r=me regardless
Attachment #8600549 - Flags: review?(dholbert) → review+
(Assignee)

Comment 6

4 years ago
Created attachment 8601191 [details] [diff] [review]
Report image source size again in about:memory

Thanks for the review! I made the |rv| declaration style consistent in this version of the patch.
(Assignee)

Updated

4 years ago
Attachment #8600549 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/1fa448752bd9
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-firefox40: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in before you can comment on or make changes to this bug.