[webkit-reviews] review denied: [Bug 123201] [WK2] [GTK] Allow running the web process with an arbitrary prefix command : [Attachment 214946] Patch

[bugzilla-daemon at webkit.org]

Gustavo Noronha (kov) <gns at gnome.org> has denied Alberto Garcia
<berto at igalia.com>'s request for review:
Bug 123201: [WK2] [GTK] Allow running the web process with an arbitrary prefix
command
https://bugs.webkit.org/show_bug.cgi?id=123201

Attachment 214946: Patch
https://bugs.webkit.org/attachment.cgi?id=214946&action=review

------- Additional Comments from Gustavo Noronha (kov) <gns at gnome.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=214946&action=review


Think the idea is a great one, would prefer the impl to be simplified a bit.

> Source/WebKit2/UIProcess/Launcher/gtk/ProcessLauncherGtk.cpp:104
> +    if (!m_launchOptions.processCmdPrefix.isEmpty()) {
> +	   Vector<String> splitted;
> +	   m_launchOptions.processCmdPrefix.split(' ', splitted);
> +	   for (auto it = splitted.begin(); it != splitted.end(); it++)
> +	       prefixArgs.append(it->utf8());

Can we do away with this splitted intermediate vector? Are you doing this to
make sure the CString is kept alive? I think it would be better to use g_strdup
if that is the case - not a lot of overhead, way simpler code.

> Source/WebKit2/UIProcess/gtk/WebProcessProxyGtk.cpp:29
>  
> +#include <glib.h>

This should not have an empty line.

> Source/WebKit2/UIProcess/gtk/WebProcessProxyGtk.cpp:37
> +    if (webProcessCmdPrefix && *webProcessCmdPrefix)

Why do you do this check on the dereferenced pointer?