Closed Bug 1065258 Opened 5 years ago Closed 5 years ago

Refactor nsMemoryInfoDumper.cpp

Categories

(Core :: XPCOM, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla35

People

(Reporter: njn, Assigned: njn)

Details

Attachments

(8 files)

I have a stack of small patches.
I moved the code, nothing else.
Attachment #8487012 - Flags: review?(jld)
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
"mr" is short for "memory reports", but that's not obvious. Also, some field
members had "mr" prefixes which goes against the usual "mFoo" naming.
Attachment #8487013 - Flags: review?(jld)
This is necessary for one of the subsequent patches.
Attachment #8487015 - Flags: review?(jld)
This one's a little trickier.

- FinishReportCallback::Callback got its |writer| via |aData|; I changed it to
  get it via a class member, which is how TempDirMemoryFinishCallback does it.

- FinishReportingCallback::Callback and TempDirMemoryFinishCallback::Callback
  have some overlap. So I made the latter call the former, which then calls on
  to do the rest of the TempDir finish-up stuff.
Attachment #8487018 - Flags: review?(jld)
This is just for aesthetics -- it makes sense to be next to DumpMemoryInfoToTempDir().
Attachment #8487020 - Flags: review?(jld)
Because the "Memory" part isn't helpful.
Attachment #8487022 - Flags: review?(jld)
Attachment #8487012 - Flags: review?(jld) → review+
Comment on attachment 8487013 [details] [diff] [review]
(part 2) - Rename some variables

Review of attachment 8487013 [details] [diff] [review]:
-----------------------------------------------------------------

Nice.  I remember thinking the mrFoo variables were a little confusing when I first saw them.
Attachment #8487013 - Flags: review?(jld) → review+
Attachment #8487014 - Flags: review?(jld) → review+
Attachment #8487015 - Flags: review?(jld) → review+
Attachment #8487018 - Flags: review?(jld) → review+
Attachment #8487019 - Flags: review?(jld) → review+
Attachment #8487020 - Flags: review?(jld) → review+
Attachment #8487022 - Flags: review?(jld) → review+
You need to log in before you can comment on or make changes to this bug.