Closed
Bug 1011225
Opened 10 years ago
Closed 10 years ago
[e10s] Name mac content process
Categories
(Core :: Widget: Cocoa, defect)
Tracking
()
RESOLVED
FIXED
mozilla34
Tracking | Status | |
---|---|---|
e10s | + | --- |
People
(Reporter: BenWa, Assigned: jaas)
References
Details
Attachments
(1 file, 4 obsolete files)
6.88 KB,
patch
|
Details | Diff | Splinter Review |
No description provided.
Attachment #8423403 -
Flags: review?(mstange)
Updated•10 years ago
|
Attachment #8423403 -
Flags: review?(mstange) → review+
Reporter | ||
Comment 1•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/9ebe70d8a9b2
Blocks: core-e10s
Reporter | ||
Comment 2•10 years ago
|
||
Backed out: https://hg.mozilla.org/integration/mozilla-inbound/rev/c8a0681b6960 Apparently ./toolkit/components/aboutmemory/tests/test_memoryReporters2.xul checks the process name.
Reporter | ||
Comment 3•10 years ago
|
||
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?
Assignee: nobody → bgirard
Attachment #8423403 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8424120 -
Flags: review?(n.nethercote)
Updated•10 years ago
|
Attachment #8424120 -
Flags: review?(n.nethercote) → review+
Reporter | ||
Comment 4•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=dbf26b738663
Comment 5•10 years ago
|
||
Benoit: did you forget to land this r+'d patch from two months ago? :)
tracking-e10s:
--- → +
Flags: needinfo?(bgirard)
Reporter | ||
Comment 6•10 years ago
|
||
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.
Flags: needinfo?(bgirard)
With the current patch, the name of the process in Activity Viewer is "Nightly Plugin Process (Content Process)". This seems less than ideal.
Reporter | ||
Comment 8•10 years ago
|
||
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.
OK, I'll try to come up with a new scheme that works well for both plugins and content processes.
Assignee | ||
Comment 10•10 years ago
|
||
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)".
Attachment #8424120 -
Attachment is obsolete: true
Attachment #8478824 -
Flags: review?(n.nethercote)
Assignee | ||
Comment 11•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=6bc78e10abbc
Comment 12•10 years ago
|
||
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.
Attachment #8478824 -
Flags: review?(n.nethercote) → review+
Reporter | ||
Comment 13•10 years ago
|
||
We talked on IRC, I think this is better.
Assignee | ||
Comment 14•10 years ago
|
||
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?
Assignee | ||
Comment 15•10 years ago
|
||
Attachment #8478824 -
Attachment is obsolete: true
Attachment #8479111 -
Flags: review?(bgirard)
Reporter | ||
Comment 16•10 years ago
|
||
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.
Attachment #8479111 -
Flags: review?(bgirard) → review+
Assignee | ||
Comment 17•10 years ago
|
||
Upped character limit to 80. http://hg.mozilla.org/integration/mozilla-inbound/rev/a7966a108af1
Attachment #8479111 -
Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/a7966a108af1
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in
before you can comment on or make changes to this bug.
Description
•