Don't run DMD when about:memory's "Measure" button is pressed

RESOLVED FIXED in mozilla35

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: njn, Assigned: njn)

Tracking

unspecified
mozilla35
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Assignee)

Description

4 years ago
If you click "Measure" in about:memory when DMD is running, it'll trigger a DMD
run, and the output goes to /tmp/dmd--<pid>.txt.gz. This isn't supposed to
happen.
(Assignee)

Comment 1

4 years ago
Created attachment 8482028 [details] [diff] [review]
Don't run DMD when about:memory's "Measure" button is pressed

A simple fix.
Attachment #8482028 - Flags: review?(jld)
(Assignee)

Comment 2

4 years ago
Created attachment 8482034 [details] [diff] [review]
Don't run DMD when about:memory's "Measure" button is pressed

We need a similar check in ContentParent.cpp.

With this in place, if I click "Measure" in about:memory when DMD is running, I
reliably get this message:

> IPDL protocol error: [PContentChild] Received an invalid file descriptor!

because we end up passing a null-equivalent FileDescriptor over IPDL. I'm not
sure if this is a problem, and if so, how to fix it.
Attachment #8482034 - Flags: review?(jld)
(Assignee)

Updated

4 years ago
Attachment #8482028 - Attachment is obsolete: true
Attachment #8482028 - Flags: review?(jld)
(Assignee)

Comment 3

4 years ago
froydnj, you said you had noticed this behaviour.

It sure would be nice if we had tests for this stuff.
(Assignee)

Comment 4

4 years ago
Created attachment 8482060 [details] [diff] [review]
(part 2) - Some minor DMD clean-ups

While I'm here, lets remove some dead code and do a couple of other minor
things.
Attachment #8482060 - Flags: review?(jld)
(Assignee)

Comment 5

4 years ago
Created attachment 8482100 [details] [diff] [review]
(part 3) - Remove a redundant dmd::ClearReports() call

This call is dominated by the call in GetReportsForThisProcess().
Attachment #8482100 - Flags: review?(jld)
(Assignee)

Updated

4 years ago
Blocks: 1061066
Attachment #8482100 - Flags: review?(jld) → review+
Attachment #8482060 - Flags: review?(jld) → review+
Attachment #8482034 - Flags: review?(jld) → review+
(In reply to Nicholas Nethercote [:njn] from comment #2)
> With this in place, if I click "Measure" in about:memory when DMD is running, I
> reliably get this message:
> 
> > IPDL protocol error: [PContentChild] Received an invalid file descriptor!
> 
> because we end up passing a null-equivalent FileDescriptor over IPDL. I'm not
> sure if this is a problem, and if so, how to fix it.

I'd assumed that FileDescriptor's nullability was an intentional feature from the code (that is, ipc::FileDescriptor itself, not the code generated by the IPDL compiler, which is where the warning lives), but chasing that warning message points to bug 831307, which indicates that that was not the case — it was meant to be a fatal error, but that was relaxed during the FxOS 1.0 release frenzy.  Using an IPDL union instead should be a Simple Matter Of Programming; I'll file a followup bug.


(In reply to Nicholas Nethercote [:njn] from comment #5)
> Created attachment 8482100 [details] [diff] [review]
> (part 3) - Remove a redundant dmd::ClearReports() call
> 
> This call is dominated by the call in GetReportsForThisProcess().

If only verifying that didn't require case-splitting on the emptiness of a string — but I suppose I have mostly myself to blame for that.
See Also: → bug 1064529
https://hg.mozilla.org/mozilla-central/rev/41f2237d7519
https://hg.mozilla.org/mozilla-central/rev/8163a8cf9c3c
https://hg.mozilla.org/mozilla-central/rev/af324a07dedd
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.