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

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Nicholas Nethercote [inactive]]

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.

Comment 1

[Nicholas Nethercote [inactive]]

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

A simple fix.

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: 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.

Comment 3

[Nicholas Nethercote [inactive]]

froydnj, you said you had noticed this behaviour.

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

Comment 4

[Nicholas Nethercote [inactive]]

Attachment: (part 2) - Some minor DMD clean-ups

While I'm here, lets remove some dead code and do a couple of other minor
things.

Comment 5

[Nicholas Nethercote [inactive]]

Attachment: (part 3) - Remove a redundant dmd::ClearReports() call

This call is dominated by the call in GetReportsForThisProcess().

Comment 6

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

(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.

Comment 7

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/41f2237d7519
https://hg.mozilla.org/integration/mozilla-inbound/rev/8163a8cf9c3c
https://hg.mozilla.org/integration/mozilla-inbound/rev/af324a07dedd

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/41f2237d7519
https://hg.mozilla.org/mozilla-central/rev/8163a8cf9c3c
https://hg.mozilla.org/mozilla-central/rev/af324a07dedd