Bug 156060 - Fix WEB_PROCESS_CMD_PREFIX and NETWORK_PROCESS_CMD_PREFIX after r196500
Summary: Fix WEB_PROCESS_CMD_PREFIX and NETWORK_PROCESS_CMD_PREFIX after r196500
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-03-31 05:23 PDT by Emanuele Aina
Modified: 2016-04-04 03:26 PDT (History)
6 users (show)

See Also:


Attachments
Patch (5.51 KB, patch)
2016-03-31 05:25 PDT, Emanuele Aina
no flags Details | Formatted Diff | Diff
Patch (5.23 KB, patch)
2016-04-01 05:34 PDT, Emanuele Aina
no flags Details | Formatted Diff | Diff
Patch (19.80 KB, patch)
2016-04-01 10:37 PDT, Emanuele Aina
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Emanuele Aina 2016-03-31 05:23:05 PDT
The patch on bug #154190 deleted all the platformGetLaunchOptions() callsites (except in PluginProcessProxy), thus breaking the mechanism to attach gdbserver to subprocesses using the WEB_PROCESS_CMD_PREFIX and NETWORK_PROCESS_CMD_PREFIX environment variables.
Comment 1 Emanuele Aina 2016-03-31 05:25:29 PDT
Created attachment 275274 [details]
Patch
Comment 2 Darin Adler 2016-03-31 16:02:32 PDT
Comment on attachment 275274 [details]
Patch

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

> Source/WebKit2/UIProcess/Databases/DatabaseProcessProxy.h:87
> +#if PLATFORM(GTK) || PLATFORM(EFL)
>      void platformGetLaunchOptions(ProcessLauncher::LaunchOptions&);
> +#else
> +    void platformGetLaunchOptions(ProcessLauncher::LaunchOptions&) { }
> +#endif

A much better way to do this is to write this, outside the class body:

    #if !(PLATFORM(GTK) || PLATFORM(EFL))
    inline void DatabaseProcessProxy::platformGetLaunchOptions(ProcessLauncher::LaunchOptions&)
    {
    }
    #endif
Comment 3 Emanuele Aina 2016-04-01 05:34:13 PDT
Created attachment 275395 [details]
Patch
Comment 4 Emanuele Aina 2016-04-01 08:08:41 PDT
I'm thinking about the alternate approach of checking the appropriate environment variable (accordingly to launchOptions.processType) directly in ChildProcessProxy::getLaunchOptions() and drop platformGetLaunchOptions() altogether.

At the moment we have roughly the same code checking those environment variables in multiple places, which led to some inconsistency (PLUGIN_PROCESS_CMD_PREFIX only works with the EFL port and not GTK, and there's not corresponding DATABASE_PROCESS_CMD_PREFIX).

I guess that would be more in the spirit of the original commit, right?
Comment 5 Emanuele Aina 2016-04-01 10:37:01 PDT
Created attachment 275416 [details]
Patch
Comment 6 Emanuele Aina 2016-04-01 14:33:41 PDT
The new patch follows the approach I previously described adding PLUGIN_PROCESS_CMD_PREFIX and DATABASE_PROCESS_CMD_PREFIX.
Comment 7 Darin Adler 2016-04-02 20:20:14 PDT
Comment on attachment 275416 [details]
Patch

Looks OK.
Comment 8 WebKit Commit Bot 2016-04-04 02:34:25 PDT
Comment on attachment 275416 [details]
Patch

Rejecting attachment 275416 [details] from commit-queue.

emanuele.aina@collabora.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/contributors.json.

- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/contributors.json by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.
Comment 9 WebKit Commit Bot 2016-04-04 03:26:26 PDT
Comment on attachment 275416 [details]
Patch

Clearing flags on attachment: 275416

Committed r199002: <http://trac.webkit.org/changeset/199002>
Comment 10 WebKit Commit Bot 2016-04-04 03:26:31 PDT
All reviewed patches have been landed.  Closing bug.