[webkit-reviews] review granted: [Bug 213379] [macOS] Connections to the preference daemon are established before entering the sandbox : [Attachment 402313] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Per Arne Vollan <pvollan at apple.com>'s
request for review:
Bug 213379: [macOS] Connections to the preference daemon are established before
entering the sandbox
https://bugs.webkit.org/show_bug.cgi?id=213379

Attachment 402313: Patch

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




--- Comment #2 from Darin Adler <darin at apple.com> ---
Comment on attachment 402313
  --> https://bugs.webkit.org/attachment.cgi?id=402313
Patch

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

> Source/WebKit/NetworkProcess/mac/NetworkProcessMac.mm:96
> +    StringBuilder path;
> +    path.append([webKit2Bundle resourcePath]);
> +    path.append("/");
> +    path.append("com.apple.WebKit.NetworkProcess.sb");
> +    sandboxParameters.setOverrideSandboxProfilePath(path.toString());

This is a job for makeString, not StringBuilder.

    sandboxParameters.setOverrideSandboxProfilePath(makeString([webKit2Bundle
resourcePath], "/com.apple.WebKit.NetworkProcess.sb"));

Or use a local String variable if you like.

> Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:229
> +    // We don't need to talk to the dock.

Capitalize Dock.

> Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:233
> +    if (Class nsApplicationClass = NSClassFromString(@"NSApplication")) {
> +	   if ([nsApplicationClass
respondsToSelector:@selector(_preventDockConnections)])
> +	       [nsApplicationClass _preventDockConnections];
> +    }

I know this code is pre-existing and only being moved, but this does seem risky
— if this method is renamed, moved, or removed this code will just silently
stop having any effect.

> Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:589
> +    StringBuilder path;
> +    path.append([webKit2Bundle resourcePath]);
> +    path.append("/");
> +    path.append("com.apple.WebProcess.sb");
> +    sandboxParameters.setOverrideSandboxProfilePath(path.toString());

This is a job for makeString, not StringBuilder.

    sandboxParameters.setOverrideSandboxProfilePath(makeString([webKit2Bundle
resourcePath], "/com.apple.WebKit.WebProcess.sb"));

Or use a local String variable if you like.