Bug 144078 - [iOS] WebKit services should inherit environment variables for home
Summary: [iOS] WebKit services should inherit environment variables for home
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-04-22 16:17 PDT by Alexey Proskuryakov
Modified: 2015-04-23 14:16 PDT (History)
6 users (show)

See Also:


Attachments
proposed fix (10.66 KB, patch)
2015-04-22 16:21 PDT, Alexey Proskuryakov
no flags Details | Formatted Diff | Diff
proposed fix (10.77 KB, patch)
2015-04-23 11:14 PDT, Alexey Proskuryakov
no flags Details | Formatted Diff | Diff
proposed fix (11.96 KB, patch)
2015-04-23 12:34 PDT, Alexey Proskuryakov
no flags Details | Formatted Diff | Diff
proposed fix (12.72 KB, patch)
2015-04-23 13:23 PDT, Alexey Proskuryakov
mitz: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2015-04-22 16:17:42 PDT
rdar://problem/20571678
Comment 1 Alexey Proskuryakov 2015-04-22 16:21:42 PDT
Created attachment 251380 [details]
proposed fix
Comment 2 mitz 2015-04-22 17:59:06 PDT
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.
Comment 3 mitz 2015-04-22 18:00:34 PDT
(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.
Comment 4 Alexey Proskuryakov 2015-04-22 18:06:45 PDT
I don't know if it's a practical concern that someone is going to put something else into HOME.
Comment 5 Alexey Proskuryakov 2015-04-22 19:01:34 PDT
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.
Comment 6 mitz 2015-04-22 19:08:10 PDT
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.
Comment 7 Alexey Proskuryakov 2015-04-23 11:14:50 PDT
Created attachment 251457 [details]
proposed fix
Comment 8 mitz 2015-04-23 12:09:04 PDT
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?
Comment 9 Alexey Proskuryakov 2015-04-23 12:30:39 PDT
We use xpc_object_t.
Comment 10 Alexey Proskuryakov 2015-04-23 12:34:12 PDT
Created attachment 251470 [details]
proposed fix

With another iOS open source build fix.
Comment 11 Alexey Proskuryakov 2015-04-23 13:23:39 PDT
Created attachment 251477 [details]
proposed fix

More SPIs for open source build...
Comment 12 WebKit Commit Bot 2015-04-23 13:26:22 PDT
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 13 mitz 2015-04-23 13:38:31 PDT
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.
Comment 14 Alexey Proskuryakov 2015-04-23 14:16:20 PDT
Committed <http://trac.webkit.org/r183209>.