Bug 181187 - [WPE][GTK] Implement the assignment of ProcessIdentifiers to child processes
Summary: [WPE][GTK] Implement the assignment of ProcessIdentifiers to child processes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Michael Catanzaro
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-12-29 09:35 PST by Brady Eidson
Modified: 2018-01-02 14:45 PST (History)
5 users (show)

See Also:


Attachments
Patch (5.32 KB, patch)
2018-01-01 14:10 PST, Michael Catanzaro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brady Eidson 2017-12-29 09:35:11 PST
Non-Cocoa ports need to implement the assignment of ProcessIdentifiers to child processes in WK2

https://trac.webkit.org/changeset/226308/webkit added the notion of a "ProcessIdentifier" in child processes.
The Identifier is created by the UI process and vended out to child processes as they are launched.

I added the initialization of the identifier in the Cocoa process launching code, but added it for the Unixes was not something I could do blind.

Somebody with the expertise and build setup needs to:
- Make sure the LaunchOptions ProcessIdentifier in the UI process propagates to the child process (This will probably be done via argv?)
- Make sure the child process extracts that process identifier and sets it properly on the ChildProcessInitializationParameters
- Remove the code in ChildProcess::initialize that handles a missing process identifier as it is invalid for it to be missing.
Comment 1 Michael Catanzaro 2017-12-29 10:16:54 PST
It looks like we only have ProcessLauncherGLib and ProcessLauncherMac. I'll look into this for ProcessLauncherGLib, which covers the WPE and GTK ports. Those are the only upstream WK2 ports right now (asides the WIP Windows port) so I believe no other ports should need to be updated.
Comment 2 Michael Catanzaro 2017-12-29 10:17:12 PST
(And thanks for filing the bug. ;)
Comment 3 Carlos Alberto Lopez Perez 2017-12-29 11:12:20 PST
What about non-Glib and non-Cocoa ports like WinCairo? I guess this needs to be implemented there as well, doesn't it?
Comment 4 Michael Catanzaro 2017-12-29 12:35:35 PST
(In reply to Carlos Alberto Lopez Perez from comment #3)
> What about non-Glib and non-Cocoa ports like WinCairo? I guess this needs to
> be implemented there as well, doesn't it?

This is for WK2. Windows support is WIP, and I don't see a ProcessLauncherWin yet.
Comment 5 Michael Catanzaro 2018-01-01 14:10:14 PST
Created attachment 330300 [details]
Patch
Comment 6 Brady Eidson 2018-01-01 18:30:45 PST
Comment on attachment 330300 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=330300&action=review

LGTM

> Source/WebKit/UIProcess/Launcher/glib/ProcessLauncherGLib.cpp:97
> +    GUniquePtr<gchar> processIdentifier(g_strdup_printf("%" PRIu64, m_launchOptions.processIdentifier.toUInt64()));

Whole patch is precisely what I tried to do blind except I had no way of verifying this line. :) Thanks!
Comment 7 WebKit Commit Bot 2018-01-02 10:17:20 PST
Comment on attachment 330300 [details]
Patch

Clearing flags on attachment: 330300

Committed r226327: <https://trac.webkit.org/changeset/226327>
Comment 8 WebKit Commit Bot 2018-01-02 10:17:22 PST
All reviewed patches have been landed.  Closing bug.
Comment 9 Michael Catanzaro 2018-01-02 13:42:06 PST
I broke the plugin process.
Comment 10 Michael Catanzaro 2018-01-02 14:45:44 PST
Committed r226337: <https://trac.webkit.org/changeset/226337>