[webkit-reviews] review granted: [Bug 74061] WebProcess and PluginProcess should inherit environment variables provided in LC_DYLD_ENVIRONMENT of main executable binary : [Attachment 118610] Patch v3

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Mark Rowe (bdash) <mrowe at apple.com>'s
request for review:
Bug 74061: WebProcess and PluginProcess should inherit environment variables
provided in LC_DYLD_ENVIRONMENT of main executable binary
https://bugs.webkit.org/show_bug.cgi?id=74061

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=118610&action=review


> Source/WebKit2/UIProcess/Launcher/mac/DynamicLinkerEnvironmentExtractor.mm:85

> +    String name(environmentString, nameLength);
> +
> +    // LC_DYLD_ENVIRONMENT only respects DYLD_*_PATH variables.
> +    if (!name.startsWith("DYLD_") || !name.endsWith("_PATH"))
> +	   return;

Some day we should write a startsWith function that works on a CString and then
we can remove this otherwise-unneeded conversion to String.

>
Source/WebKit2/UIProcess/Launcher/mac/DynamicLinkerEnvironmentExtractor.mm:111
> +	   OwnPtr<char> environmentString = adoptPtr(new
char[environmentStringLength + 1]);

Needs to be OwnArrayPtr.

(Or you could use Vector<char, 256> and make the code a little faster since it
will almost never malloc.)

>
Source/WebKit2/UIProcess/Launcher/mac/DynamicLinkerEnvironmentExtractor.mm:186
> +    Vector<std::pair<CString, CString> >::const_iterator end =
m_extractedVariables.end();
> +    for (Vector<std::pair<CString, CString> >::const_iterator it =
m_extractedVariables.begin(); it != end; ++it) {

We normally just use indexing to iterate a vector.

>
Source/WebKit2/UIProcess/Launcher/mac/DynamicLinkerEnvironmentExtractor.mm:187
> +	   CString name = it->first;

More efficient to use const CString& here.