1465924 - Correctly label the WebExtension process

Status: RESOLVED FIXED

(Core :: Gecko Profiler, enhancement, P1)

Description

[:Harald Kirschner :digitarald]

about:memory correctly labels it was "WebExtension" (vs "Web Content").

perf.html shows the extension process as "content", which we should improve.

Comment 1

[Markus Stange [:mstange]]

Attachment: Bug 1465924 - Add a new optional property called "processName" to profile.meta which contains "Main Process" in the parent process and a content process name like "WebExtensions" for special content processes, see ContentChild::RecvRemoteType.

This field is in addition to the two existing process type fields we already have:
 - profile.meta.processType contains the integer value of the GeckoProcessType enum for the process.
 - profile.threads[i].processType contains the string for that GeckoProcessType.

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

Comment 2

[Markus Stange [:mstange]]

Greg, this patch is untested and (as you can tell by the commit message) the process name stuff is rather messy. I haven't checked all the combinations of values and I'm not sure which ones we really need.

Here are the two lists of values:

https://searchfox.org/mozilla-central/rev/83a923ef7a3b95a516f240a6810c20664b1e0ac9/xpcom/build/nsXULAppAPI.h#365-380
https://searchfox.org/mozilla-central/rev/83a923ef7a3b95a516f240a6810c20664b1e0ac9/dom/ipc/ContentChild.cpp#2727-2735

I hope you have ideas and opinions on how we should put this information into the profile.

Comment 3

[Greg Tatum [:gregtatum]]

Comment on attachment 8982318 [details]
Bug 1465924 - Add a new optional property called "processName" to profile.meta which contains "Main Process" in the parent process and a content process name like "WebExtensions" for special content processes, see ContentChild::RecvRemoteType.

https://reviewboard.mozilla.org/r/248264/#review254532

::: tools/profiler/core/platform.cpp:1691
(Diff revision 1)
>    aWriter.IntProperty("processType", XRE_GetProcessType());
>  
> +  nsAutoCString processName;
> +  if (XRE_IsParentProcess()) {
> +    // We're the main process.
> +    processName.AssignLiteral("Main Process");

Should this be "Parent Process"? I feel like "main" may be a bit overloaded, especially since I have confused the main thread terminology in the past.

Comment 4

[Greg Tatum [:gregtatum]]

Comment on attachment 8982318 [details]
Bug 1465924 - Add a new optional property called "processName" to profile.meta which contains "Main Process" in the parent process and a content process name like "WebExtensions" for special content processes, see ContentChild::RecvRemoteType.

https://reviewboard.mozilla.org/r/248264/#review254532

Thanks, this will be handy to have, especially with the upcoming timeline work.

Comment 5

[Greg Tatum [:gregtatum]]

I mistakenly didn't read your comment. I'd like to look at this again when I have a bit of time to look through the code.

Comment 6

[Greg Tatum [:gregtatum]]

Comment on attachment 8982318 [details]
Bug 1465924 - Add a new optional property called "processName" to profile.meta which contains "Main Process" in the parent process and a content process name like "WebExtensions" for special content processes, see ContentChild::RecvRemoteType.

https://reviewboard.mozilla.org/r/248264/#review254544

I missed that this was WIP.

Comment 7

[Greg Tatum [:gregtatum]]

I had a needinfo on myself for this from awhile back, but I was ignoring it. I think my opinion here would be to capture all the variations of names we can get so that I can see them in context, and see if they make sense.

I think the approach should probably be to query the name sources we know, and then have a systematic way to present that to the user where it makes the most sense. As far as implementation details for this, I figure it will look similar to the code that is already here.

Comment 8

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Florian suggested I take this one. Markus, just let me know if you want it back! ;-)

I've updated the patch to output "processName" in all threads. (Will attach soon.)

Greg, I think ContentChild::GetProcessName() is in fact the best name source, and it's consistent with what about::memory does.
Now I'm guessing we'll need to update the data formats first, right?

Comment 9

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1465924 - Add profile.threads[i].processName which contains "Main Process", or the content process's name like "WebExtensions" - r?gregtatum

This field is in addition to the existing process type fields we already have:
- profile.threads[i].processType contains the string for the GeckoProcessType.
- profile.threads[i].name contains the ThreadInfo name.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8355628364af
Add profile.threads[i].processName which contains "Main Process", or the content process's name like "WebExtensions" - r=gregtatum

Comment 11

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/8355628364af