| Summary: | [iOS] WebKit services should inherit environment variables for home | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Alexey Proskuryakov <ap> | ||||||||||
| Component: | WebKit2 | Assignee: | Alexey Proskuryakov <ap> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | andersca, benjamin, cmarcelo, commit-queue, mitz, sam | ||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||
| Hardware: | Unspecified | ||||||||||||
| OS: | Unspecified | ||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Alexey Proskuryakov
2015-04-22 16:17:42 PDT
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. I don't know if it's a practical concern that someone is going to put something else into HOME. 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. 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? We use xpc_object_t. Created attachment 251470 [details]
proposed fix
With another iOS open source build fix.
Created attachment 251477 [details]
proposed fix
More SPIs for open source build...
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. Committed <http://trac.webkit.org/r183209>. |