[webkit-reviews] review denied: [Bug 185611] [GTK][WPE]: Avoid trying to getenv with an unset varname : [Attachment 340488] Address comments

[bugzilla-daemon at webkit.org]

Carlos Garcia Campos <cgarcia at igalia.com> has denied Thibault Saunier
<tsaunier at gnome.org>'s request for review:
Bug 185611: [GTK][WPE]: Avoid trying to getenv with an unset varname
https://bugs.webkit.org/show_bug.cgi?id=185611

Attachment 340488: Address comments

https://bugs.webkit.org/attachment.cgi?id=340488&action=review




--- Comment #17 from Carlos Garcia Campos <cgarcia at igalia.com> ---
Comment on attachment 340488
  --> https://bugs.webkit.org/attachment.cgi?id=340488
Address comments

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

> Source/WebKit/UIProcess/ChildProcessProxy.cpp:79
> +    default:
> +	   ASSERT_NOT_REACHED();

I don't think this is the right fix, this should never happen, we are handling
all possible values in the switch.

> Source/WebKit/UIProcess/Plugins/PluginProcessProxy.cpp:95
> -    ChildProcessProxy::getLaunchOptions(launchOptions);
>      platformGetLaunchOptions(launchOptions, m_pluginProcessAttributes);
> +    ChildProcessProxy::getLaunchOptions(launchOptions);

This doesn't look correct either. I think the problem is that most of the child
processes are calling connect(), which is virtual, in the constructor. The
right fix might be to call connect from the create() methods after the object
is constructed like WebProcessProxy does.