1431217 - The parent process shouldn't be marked as shutting down in the profile if it's not shutting down

Status: RESOLVED FIXED

(Core :: Gecko Profiler, enhancement)

Description

[Markus Stange [:mstange]]

I had a typo in the second patch in bug 1384688: We should be passing false for aIsShuttingDown instead of true when dumping the parent process profile.

This patch is also changing a place where pass a TimeStamp but ignore its value, and makes us use a bool instead.

Comment 1

[Markus Stange [:mstange]]

Attachment: Bug 1431217 - Fix a typo that caused us to treat the parent process as shutting down when dumping the profile.

Review commit: https://reviewboard.mozilla.org/r/213722/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/213722/

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8943392 [details]
Bug 1431217 - Fix a typo that caused us to treat the parent process as shutting down when dumping the profile.

https://reviewboard.mozilla.org/r/213722/#review220298

Good catch. This is probably something we should uplift, imo.

::: tools/profiler/gecko/nsProfiler.cpp:580
(Diff revision 1)
>    TimeStamp thisProcessFirstSampleTime;
>  
>    // Start building up the JSON result and grab the profile from this process.
>    mWriter->Start();
>    if (!profiler_stream_json_for_this_process(*mWriter, aSinceTime,
> -                                             /* aIsShuttingDown */ true,
> +                                             /* aIsShuttingDown */ false,

Ooof - sorry I didn't catch that. Thanks!

Comment 3

[Markus Stange [:mstange]]

It wasn't you who missed it, that patch was reviewed by njn :)

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c7e047777011
Fix a typo that caused us to treat the parent process as shutting down when dumping the profile. r=mconley

Comment 5

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/c7e047777011