Closed
Bug 686172
Opened 13 years ago
Closed 13 years ago
smaps trees shouldn't show up where smaps isn't available
Categories
(Toolkit :: about:memory, defect)
Tracking
()
RESOLVED
FIXED
mozilla9
People
(Reporter: justin.lebar+bug, Assigned: justin.lebar+bug)
References
Details
Attachments
(1 file)
11.31 KB,
patch
|
khuey
:
review+
|
Details | Diff | Splinter Review |
The RSS/vsize/swap breakdown trees appear on Mac and Windows, even though we have no data to fill them in with. They simply shouldn't be there. An easy way to fix this (and improve the experience on Linux at the same time) would be not to show completely empty smaps trees. On Linux, this would mean not showing the "swap usage" tree most of the time, since swap is usually empty. This might be confusing, because someone expecting Firefox to tell them how much swap is being used wouldn't even see the heading. OTOH, in normal circumstances, it means that you don't have to click the header to discover that swap is empty.
Comment 1•13 years ago
|
||
IMO, the best behaviour is: - Always show all three trees on Linux, even if empty. - Never show the trees on other platforms. (I've said before that having to click on the "swap" header to see that the tree is empty doesn't strike me as a problem.) If that's too hard, then not showing empty trees on Linux is better than showing empty trees on other platforms. But hopefully the best behaviour is possible.
Assignee | ||
Comment 2•13 years ago
|
||
I'll see how hard it is to do OS detection from aboutmemory.js.
Comment 3•13 years ago
|
||
Failing that, is it possible to communicate "present-but-empty" somehow from MapsMemoryReporter.cpp? E.g. just a single "maps/swap" reporter that has a zero value?
Assignee | ||
Comment 4•13 years ago
|
||
Attachment #560080 -
Flags: review?(khuey)
Comment on attachment 560080 [details] [diff] [review] Patch v1 Review of attachment 560080 [details] [diff] [review]: ----------------------------------------------------------------- ::: xpcom/base/MapsMemoryReporter.cpp @@ +120,5 @@ > + } > + > + PRBool mSeenResident; > + PRBool mSeenVsize; > + PRBool mSeenSwap; Use C++ bool please. @@ +256,5 @@ > MapsReporter::ParseMapping( > FILE *aFile, > nsIMemoryMultiReporterCallback *aCallback, > + nsISupports *aClosure, > + CategoriesSeen &aCategoriesSeen) I know you fixed this in the other patch, but make this a pointer here.
Attachment #560080 -
Flags: review?(khuey) → review+
Updated•13 years ago
|
Assignee: nobody → justin.lebar+bug
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
You need to log in
before you can comment on or make changes to this bug.
Description
•