Bug 144078 – [iOS] WebKit services should inherit environment variables for home

• Component: WebKit2
• Status: RESOLVED FIXED

[Alexey Proskuryakov]

rdar://problem/20571678

[Alexey Proskuryakov]

Created attachment 251380 [details]
proposed fix

Comment on attachment 251380 [details]
proposed fix

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

> Source/WebKit2/UIProcess/Launcher/mac/ProcessLauncherMac.mm:229
> +    if (const char* environmentHOME = getenv("HOME"))
> +        xpc_dictionary_set_string(initializationMessage.get(), "HOME", environmentHOME);
> +    if (const char* environmentCFFIXED_USER_HOME = getenv("CFFIXED_USER_HOME"))
> +        xpc_dictionary_set_string(initializationMessage.get(), "CFFIXED_USER_HOME", environmentCFFIXED_USER_HOME);
> +    if (const char* environmentTMPDIR = getenv("TMPDIR"))
> +        xpc_dictionary_set_string(initializationMessage.get(), "TMPDIR", environmentTMPDIR);

Since we share ownership of the bootstrap dictionary with Core Foundation, we should consider using keys that won’t clash with anything that framework may choose to use. I suggest WebKitEnvironmentVariableHOME etc or a WebKitEnvironment dictionary with three key-value pairs.

(In reply to comment #2)
> a WebKitEnvironment dictionary with three key-value pairs.

This would be nice because we won’t need to duplicate the knowledge of which variables we care about on both sides.

[Alexey Proskuryakov]

I don't know if it's a practical concern that someone is going to put something else into HOME.

[Alexey Proskuryakov]

The way I look into this is that it should be clear what the dictionary holds, and it doesn't hold "WebKit environment variables". I could see something like a Container dictionary with the three values, however that wouldn't address your concern.

I had two concerns: potential namespace collisions and the duplication of the list of variables. If you post a patch that addresses only the duplication, I'll consider it an improvement. I won't let the first concern stop me from reviewing such a patch.

[Alexey Proskuryakov]

Created attachment 251457 [details]
proposed fix

Comment on attachment 251457 [details]
proposed fix

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

Seems to be failing to build without the internal SDK, because xpc_copy_bootstrap is not defined.

> Source/WebCore/platform/spi/cf/CFBundleSPI.h:34
> +#include <wtf/spi/darwin/XPCSPI.h>

What are we using from this below?

[Alexey Proskuryakov]

We use xpc_object_t.

[Alexey Proskuryakov]

Created attachment 251470 [details]
proposed fix

With another iOS open source build fix.

[Alexey Proskuryakov]

Created attachment 251477 [details]
proposed fix

More SPIs for open source build...

[WebKit Commit Bot]

Attachment 251477 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/spi/darwin/XPCSPI.h:102:  The parameter name "applier" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 251477 [details]
proposed fix

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

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:16447
>  				65086DA719AC1719009AF46B /* CFLocaleSPI.h */,
> +				E10A39E11AE84E7100A29AE3 /* CFBundleSPI.h */,

These seem to be in the wrong order.

[Alexey Proskuryakov]

Committed <http://trac.webkit.org/r183209>.