<rdar://problem/13600073>
Created attachment 200230 [details] proposed patch
Comment on attachment 200230 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=200230&action=review > Source/WebKit2/PluginProcess/mac/PluginProcessMac.mm:47 > +const CFStringRef _kLSPluginBundleIdentifierKey = CFSTR("LSPluginBundleIdentifierKey"); Why the underscore? Don’t we need “static” on this to prevent it from having external linkage? > Source/WebKit2/PluginProcess/mac/PluginProcessMac.mm:285 > +static String loadSandboxProfileForDirectory(String bundleIdentifier, NSString *sandboxProfileDirectoryPath) Why the change from const String& to String for the argument type? Please use const String&.
Comment on attachment 200230 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=200230&action=review >> Source/WebKit2/PluginProcess/mac/PluginProcessMac.mm:47 >> +const CFStringRef _kLSPluginBundleIdentifierKey = CFSTR("LSPluginBundleIdentifierKey"); > > Why the underscore? Don’t we need “static” on this to prevent it from having external linkage? I guess I was just thinking of matching other similar keys. No need to do that here indeed. In C++, constants do not have external linkage, so we do not need "static" here. >> Source/WebKit2/PluginProcess/mac/PluginProcessMac.mm:285 >> +static String loadSandboxProfileForDirectory(String bundleIdentifier, NSString *sandboxProfileDirectoryPath) > > Why the change from const String& to String for the argument type? Please use const String&. I'm modifying the argument in the function. I think that asking the caller to do the copy is a fairly common and desirable pattern, as this gives the compiler more opportunities for optimization.
Committed <http://trac.webkit.org/r149619>.
Went with const String&. While I think that a String is just fine, we should probably start using this idiom in some place where it demonstrably improves performance, not here.