1275430 - Add telemetry and logging to record content process failures to start

Status: RESOLVED FIXED

(Core :: DOM: Content Processes, defect, P2)

Description

[Haik Aftandilian [:haik]]

+++ This bug was initially created as a clone of Bug #1165945 +++

Splitting the telemetry and logging off from 1165945 so that 1165945 can be used for improving the user experience when a content process fails to launch. When I commented out the code to start a content process to test the telemetry code, Firefox hung at launch and no UI ever was displayed.

This bug will just be used for the added telemetry and logging.

Comment 1

[Haik Aftandilian [:haik]]

Attachment: Adds a telemetry probe and console logging to record subprocess launch failures

bsmedberg provided data-collection review on 1165945.

Comment 2

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8756130 [details] [diff] [review]
Adds a telemetry probe and console logging to record subprocess launch failures

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

::: ipc/glue/GeckoChildProcessHost.cpp
@@ +567,5 @@
> +    return true;
> +  } else {
> +    CHROMIUM_LOG(ERROR) << "Failed to launch " <<
> +      XRE_ChildProcessTypeToString(mProcessType) << " subprocess";
> +    Telemetry::Accumulate(Telemetry::SUBPROCESS_LAUNCH_FAILURE, mProcessType);

In other cases we've used keyed count histograms, which can be a little clearer. Here's an example:
http://searchfox.org/mozilla-central/rev/c416322cd4079ab9ef6b30203fedae11cf39fdc5/dom/ipc/ContentParent.cpp#2092

Rather than using "content", you should be able to use the result of XRE_ChildProcessTypeToString. You probably will have to use nsDependentCString as follows:
http://searchfox.org/mozilla-central/rev/c416322cd4079ab9ef6b30203fedae11cf39fdc5/ipc/glue/MessageChannel.cpp#761

Comment 3

[Haik Aftandilian [:haik]]

Attachment: Adds a telemetry probe and console logging to record subprocess launch failures

Changed the histogram type per billm's recommendation.

Should values persist across restarts? In my testing, I found they don't.

Will add about:telemetry screenshot showing the new histograms.

Comment 4

[Haik Aftandilian [:haik]]

Attachment: Histogram Screenshot

Comment 5

[Haik Aftandilian [:haik]]

(In reply to Haik Aftandilian [:haik] from comment #3)
> Created attachment 8758802 [details] [diff] [review]
> Should values persist across restarts? In my testing, I found they don't.

I wasn't looking in the right place. The values are getting saved in the archived data sets.

Comment 6

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8758802 [details] [diff] [review]
Adds a telemetry probe and console logging to record subprocess launch failures

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

Thanks.

Comment 7

[Haik Aftandilian [:haik]]

Attachment: Adds a telemetry probe and console logging to record subprocess launch failures

Added r=

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/32a8d48c26ee
Add telemetry and logging to record content process failures to start; r=billm

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/32a8d48c26ee