Bug 156060 – Fix WEB_PROCESS_CMD_PREFIX and NETWORK_PROCESS_CMD_PREFIX after r196500

• Component: WebKit2
• Status: RESOLVED FIXED

[Emanuele Aina]

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.

[Emanuele Aina]

Created attachment 275274 [details]
Patch

[Darin Adler]

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

[Emanuele Aina]

Created attachment 275395 [details]
Patch

[Emanuele Aina]

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?

[Emanuele Aina]

Created attachment 275416 [details]
Patch

[Emanuele Aina]

The new patch follows the approach I previously described adding PLUGIN_PROCESS_CMD_PREFIX and DATABASE_PROCESS_CMD_PREFIX.

[Darin Adler]

Comment on attachment 275416 [details]
Patch

Looks OK.

[WebKit Commit Bot]

Comment on attachment 275416 [details]
Patch

Rejecting attachment 275416 [details] from commit-queue.

[e-mail] 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.

[WebKit Commit Bot]

Comment on attachment 275416 [details]
Patch

Clearing flags on attachment: 275416

Committed r199002: <http://trac.webkit.org/changeset/199002>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.