1011225 - [e10s] Name mac content process

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Benoit Girard (:BenWa)]

Attachment: patch

Comment 1

[Benoit Girard (:BenWa)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9ebe70d8a9b2

Comment 2

[Benoit Girard (:BenWa)]

Backed out:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c8a0681b6960

Apparently ./toolkit/components/aboutmemory/tests/test_memoryReporters2.xul checks the process name.

Comment 3

[Benoit Girard (:BenWa)]

Attachment: patch v2

njn can you review the change to test_memoryReporters2.xul and also are there other places where about:memory will rely on this process name?

Comment 4

[Benoit Girard (:BenWa)]

https://tbpl.mozilla.org/?tree=Try&rev=dbf26b738663

Comment 5

[Chris Peterson [:cpeterson]]

Benoit: did you forget to land this r+'d patch from two months ago? :)

Comment 6

[Benoit Girard (:BenWa)]

It keeps having small failure with the about:memory code which uses these process name. Then I got moved back to b2g. If someone want to adjust how about:memory this shouldn't be very much effort to land.

Comment 7

[Josh Aas]

With the current patch, the name of the process in Activity Viewer is "Nightly Plugin Process (Content Process)". This seems less than ideal.

Comment 8

[Benoit Girard (:BenWa)]

The reason I picked this is:
- To be able to filter by 'Firefox/Nightly' in the process manager and group the processes, originally for plugins.
- To match the plugin process name.

Feel free to change it.

Comment 9

[Josh Aas]

OK, I'll try to come up with a new scheme that works well for both plugins and content processes.

Comment 10

[Josh Aas]

Attachment: patch v3

This implements a better process naming scheme and fixes another memory reporter test.

With this, using Nightly, content processes are called "Nightly Content Process" and plugins processes are called "Nightly Plugin Process (Shockwave Flash)".

Comment 11

[Josh Aas]

https://tbpl.mozilla.org/?tree=Try&rev=6bc78e10abbc

Comment 12

[Nicholas Nethercote [inactive]]

Comment on attachment 8478824 [details] [diff] [review]
patch v3

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

Seems fine. BenWa appears to have stronger opinions than me about the exact naming scheme, so you might like to get his approval too.

Comment 13

[Benoit Girard (:BenWa)]

We talked on IRC, I think this is better.

Comment 14

[Josh Aas]

I've been thinking that having the word "Process" in the name is a bit redundant, since these names show up in a list of processes, and it takes up a lot of space. I'm thinking of changing to "Nightly Web Content" and "Nightly Plugin Content (Shockwave Flash)" instead, which is also much closer to what Safari does.

Any objections?

Comment 15

[Josh Aas]

Attachment: patch v4

Comment 16

[Benoit Girard (:BenWa)]

Comment on attachment 8479111 [details] [diff] [review]
patch v4

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

::: dom/plugins/ipc/PluginModuleChild.cpp
@@ +184,5 @@
>          AddQuirk(QUIRK_FLASH_EXPOSE_COORD_TRANSLATION);
>      }
>  #else // defined(OS_MACOSX)
> +    const char* namePrefix = "Plugin Content";
> +    char nameBuffer[60];

optional nit: IMO 60 is a bit short for allowing longer APP_NAME. I don't see the harm in requesting 1024.

::: ipc/app/macbuild/Contents/Resources/English.lproj/InfoPlist.strings.in
@@ +3,5 @@
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
>  /* Localized versions of Info.plist keys */
>  
> +CFBundleName = "%APP_NAME%";

I'm not familiar with our localization setup. If we localized this in the past we wont be able to do it now.

Observation: If we can't name the process then it wont default to Plugin Process. I don't think it's happens however.

Comment 17

[Josh Aas]

Attachment: patch v5

Upped character limit to 80.

http://hg.mozilla.org/integration/mozilla-inbound/rev/a7966a108af1

Comment 18

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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