Refactor OS X sandbox processing and audit sandbox files

Refactor OS X sandbox processing and audit sandbox files

Audited the sandbox files to enusre all rules are necessary and removed
any incorrect comments.

The OS X Sandbox code is refactored to get rid of all string processing
of the raw scheme code in the sandbox profiles. By using the ability to
pass parameters into sandbox profiles, the variable substitution logic
is performed by the scheme interpreter itself.

BUG=242704
Committed: https://crrev.com/76eaa8778a322803dc3ac20cda05525e615dec4d
Cr-Commit-Position: refs/heads/master@{#336610}

Committed: https://crrev.com/e46995fce5cb605f406927ecd0b2eb2018e7e3aa
Cr-Commit-Position: refs/heads/master@{#339038}

[jln (very slow on Chromium), 2015-06-16 18:18:11]

jln@chromium.org changed reviewers:
+ jln@chromium.org

[jln (very slow on Chromium), 2015-06-16 18:18:13]

Did a 30s first-pass to check that you knew about the style guide etc, here are
some trivial tiny nits.

https://codereview.chromium.org/1186233004/diff/1/content/common/sandbox_mac.h
File content/common/sandbox_mac.h (right):

https://codereview.chromium.org/1186233004/diff/1/content/common/sandbox_mac....
content/common/sandbox_mac.h:1: // Copyright (c) 2015 The Chromium Authors. All
rights reserved.
Don't change the date in existing file (for new files, the "(c)" is no longer
needed)

https://codereview.chromium.org/1186233004/diff/1/content/common/sandbox_mac....
content/common/sandbox_mac.h:38: // Disable copy constructor
Use DISALLOW_COPY_AND_ASSIGN instead.

https://codereview.chromium.org/1186233004/diff/1/content/common/sandbox_mac.mm
File content/common/sandbox_mac.mm (right):

https://codereview.chromium.org/1186233004/diff/1/content/common/sandbox_mac....
content/common/sandbox_mac.mm:1: // Copyright (c) 2015 The Chromium Authors. All
rights reserved.
No date change.

[Greg K, 2015-06-16 19:15:31]

kerrnel@chromium.org changed reviewers:
+ rsesek@chromium.org

[Greg K, 2015-06-16 19:15:31]

kerrnel@chromium.org changed required reviewers:
+ rsesek@chromium.org

[Greg K, 2015-06-16 20:12:27]

kerrnel@chromium.org changed reviewers:
- jln@chromium.org

[Robert Sesek, 2015-06-16 23:52:41]

https://codereview.chromium.org/1186233004/diff/20001/content/common/common.sb
File content/common/common.sb (right):

https://codereview.chromium.org/1186233004/diff/20001/content/common/common.s...
content/common/common.sb:42: ; Each line is marked with the System version that
needs it.
Delete this comment, since I don't think it's true anymore?

https://codereview.chromium.org/1186233004/diff/20001/content/common/sandbox_...
File content/common/sandbox_mac.h (right):

https://codereview.chromium.org/1186233004/diff/20001/content/common/sandbox_...
content/common/sandbox_mac.h:31: // ensure proper initialization and cleanup
nit: Comments should have proper punctuation. Also, you can wrap this a little
more towards the 80 column limit. Apply here and throughout the entire CL (I
didn't mark it anywhere else).

https://codereview.chromium.org/1186233004/diff/20001/content/common/sandbox_...
content/common/sandbox_mac.h:32: class CONTENT_EXPORT SandboxCompiler {
Potential future TODO: Move this to //sandbox/mac and name it
sandbox::ProfileCompiler.

https://codereview.chromium.org/1186233004/diff/20001/content/common/sandbox_...
content/common/sandbox_mac.h:34: // Explicit out-of-line constructor
nit: remove, and line 36. This should be obvious to the reader.

https://codereview.chromium.org/1186233004/diff/20001/content/common/sandbox_...
content/common/sandbox_mac.h:38: // Initialize the sandbox parameters
When does this method need to be called? Does this need to be called before
anything else?

https://codereview.chromium.org/1186233004/diff/20001/content/common/sandbox_...
content/common/sandbox_mac.h:39: bool Init();
nit: Place blank lines after each of these methods.

https://codereview.chromium.org/1186233004/diff/20001/content/common/sandbox_...
content/common/sandbox_mac.h:39: bool Init();
Potential idea: Remove Init() entirely, and wait to construct |params_| until
CompileAndApplyProfile. Store all the values (including duplicate copies of
"TRUE" and "FALSE", yes), in |strings_|. It makes the API a little cleaner at
the cost of a little more memory. Thoughts?

https://codereview.chromium.org/1186233004/diff/20001/content/common/sandbox_...
content/common/sandbox_mac.h:48: // Ensure that the C++ strings are not
destroyed while the
"Storage for the string parameters that are used in the profile."

https://codereview.chromium.org/1186233004/diff/20001/content/common/sandbox_...
content/common/sandbox_mac.h:51: void* params_;
Document these two members.

https://codereview.chromium.org/1186233004/diff/20001/content/common/sandbox_...
File content/common/sandbox_mac.mm (right):

https://codereview.chromium.org/1186233004/diff/20001/content/common/sandbox_...
content/common/sandbox_mac.mm:45: void CGSShutdownServerConnections();
nit: blank line after

https://codereview.chromium.org/1186233004/diff/20001/content/common/sandbox_...
content/common/sandbox_mac.mm:47: void sandbox_set_param(void* params, const
char* key, const char* value);
This returns int, per the disassembly I provided. You can ignore the return
value, but it should be declared properly.

https://codereview.chromium.org/1186233004/diff/20001/content/common/sandbox_...
content/common/sandbox_mac.mm:161: int
SandboxCompiler::CompileAndApplyProfile(std::string* error) {
This should probably return bool.

https://codereview.chromium.org/1186233004/diff/20001/content/common/sandbox_...
File content/common/sandbox_mac_compiler_unittest.mm (right):

https://codereview.chromium.org/1186233004/diff/20001/content/common/sandbox_...
content/common/sandbox_mac_compiler_unittest.mm:27: CHECK_EQ(compiler.Init(),
true);
CHECK_EQ(…, bool) can be jut CHECK(…)

https://codereview.chromium.org/1186233004/diff/20001/content/common/sandbox_...
content/common/sandbox_mac_compiler_unittest.mm:164: }
nit: Blank line before, and closing namespaces (except for forward-declarations)
always end in "  // namespace <namespace>" (note two spaces before EOL
comments).

https://codereview.chromium.org/1186233004/diff/20001/content/content_tests.gypi
File content/content_tests.gypi (right):

https://codereview.chromium.org/1186233004/diff/20001/content/content_tests.g...
content/content_tests.gypi:644: 'common/sandbox_mac_compiler_unittest.mm',
nit: Alphabetize.

https://codereview.chromium.org/1186233004/diff/20001/content/ppapi_plugin/pp...
File content/ppapi_plugin/ppapi.sb (left):

https://codereview.chromium.org/1186233004/diff/20001/content/ppapi_plugin/pp...
content/ppapi_plugin/ppapi.sb:16: (allow mach-lookup (global-name
"com.apple.FontObjectsServer"))  ; 10.5.6
Wow, can we really remove this?

[Greg K, 2015-06-18 19:16:29]

Patchset #3 (id:40001) has been deleted

[Greg K, 2015-06-18 20:45:18]

https://codereview.chromium.org/1186233004/diff/20001/content/common/sandbox_...
File content/common/sandbox_mac.h (right):

https://codereview.chromium.org/1186233004/diff/20001/content/common/sandbox_...
content/common/sandbox_mac.h:31: // ensure proper initialization and cleanup
On 2015/06/16 23:52:40, Robert Sesek wrote:
> nit: Comments should have proper punctuation. Also, you can wrap this a little
> more towards the 80 column limit. Apply here and throughout the entire CL (I
> didn't mark it anywhere else).

Done.

https://codereview.chromium.org/1186233004/diff/20001/content/common/sandbox_...
content/common/sandbox_mac.h:32: class CONTENT_EXPORT SandboxCompiler {
On 2015/06/16 23:52:40, Robert Sesek wrote:
> Potential future TODO: Move this to //sandbox/mac and name it
> sandbox::ProfileCompiler.

Acknowledged.

https://codereview.chromium.org/1186233004/diff/20001/content/common/sandbox_...
content/common/sandbox_mac.h:34: // Explicit out-of-line constructor
On 2015/06/16 23:52:40, Robert Sesek wrote:
> nit: remove, and line 36. This should be obvious to the reader.

Done.

https://codereview.chromium.org/1186233004/diff/20001/content/common/sandbox_...
content/common/sandbox_mac.h:38: // Initialize the sandbox parameters
On 2015/06/16 23:52:40, Robert Sesek wrote:
> When does this method need to be called? Does this need to be called before
> anything else?

Acknowledged.

https://codereview.chromium.org/1186233004/diff/20001/content/common/sandbox_...
content/common/sandbox_mac.h:39: bool Init();
On 2015/06/16 23:52:40, Robert Sesek wrote:
> nit: Place blank lines after each of these methods.

Done.

https://codereview.chromium.org/1186233004/diff/20001/content/common/sandbox_...
content/common/sandbox_mac.h:39: bool Init();
On 2015/06/16 23:52:40, Robert Sesek wrote:
> nit: Place blank lines after each of these methods.

Done.

https://codereview.chromium.org/1186233004/diff/20001/content/common/sandbox_...
content/common/sandbox_mac.h:48: // Ensure that the C++ strings are not
destroyed while the
On 2015/06/16 23:52:40, Robert Sesek wrote:
> "Storage for the string parameters that are used in the profile."

Done.

https://codereview.chromium.org/1186233004/diff/20001/content/common/sandbox_...
content/common/sandbox_mac.h:51: void* params_;
On 2015/06/16 23:52:40, Robert Sesek wrote:
> Document these two members.

Done.

https://codereview.chromium.org/1186233004/diff/20001/content/common/sandbox_...
File content/common/sandbox_mac.mm (right):

https://codereview.chromium.org/1186233004/diff/20001/content/common/sandbox_...
content/common/sandbox_mac.mm:45: void CGSShutdownServerConnections();
On 2015/06/16 23:52:40, Robert Sesek wrote:
> nit: blank line after

Done.

https://codereview.chromium.org/1186233004/diff/20001/content/common/sandbox_...
content/common/sandbox_mac.mm:47: void sandbox_set_param(void* params, const
char* key, const char* value);
On 2015/06/16 23:52:40, Robert Sesek wrote:
> This returns int, per the disassembly I provided. You can ignore the return
> value, but it should be declared properly.

Done.

https://codereview.chromium.org/1186233004/diff/20001/content/common/sandbox_...
content/common/sandbox_mac.mm:161: int
SandboxCompiler::CompileAndApplyProfile(std::string* error) {
On 2015/06/16 23:52:40, Robert Sesek wrote:
> This should probably return bool.

Done.

https://codereview.chromium.org/1186233004/diff/20001/content/common/sandbox_...
File content/common/sandbox_mac_compiler_unittest.mm (right):

https://codereview.chromium.org/1186233004/diff/20001/content/common/sandbox_...
content/common/sandbox_mac_compiler_unittest.mm:27: CHECK_EQ(compiler.Init(),
true);
On 2015/06/16 23:52:41, Robert Sesek wrote:
> CHECK_EQ(…, bool) can be jut CHECK(…)

Done: since Init() went away this did too.

https://codereview.chromium.org/1186233004/diff/20001/content/common/sandbox_...
content/common/sandbox_mac_compiler_unittest.mm:164: }
On 2015/06/16 23:52:41, Robert Sesek wrote:
> nit: Blank line before, and closing namespaces (except for
forward-declarations)
> always end in "  // namespace <namespace>" (note two spaces before EOL
> comments).

Done.

https://codereview.chromium.org/1186233004/diff/20001/content/content_tests.gypi
File content/content_tests.gypi (right):

https://codereview.chromium.org/1186233004/diff/20001/content/content_tests.g...
content/content_tests.gypi:644: 'common/sandbox_mac_compiler_unittest.mm',
On 2015/06/16 23:52:41, Robert Sesek wrote:
> nit: Alphabetize.

Done.

https://codereview.chromium.org/1186233004/diff/20001/content/ppapi_plugin/pp...
File content/ppapi_plugin/ppapi.sb (left):

https://codereview.chromium.org/1186233004/diff/20001/content/ppapi_plugin/pp...
content/ppapi_plugin/ppapi.sb:16: (allow mach-lookup (global-name
"com.apple.FontObjectsServer"))  ; 10.5.6
On 2015/06/16 23:52:41, Robert Sesek wrote:
> Wow, can we really remove this?

Yeah, from digging around on Google and Webkit2 source code, turns out we don't
want to do that.

[Robert Sesek, 2015-06-22 20:40:23]

https://codereview.chromium.org/1186233004/diff/60001/content/common/common.sb
File content/common/common.sb (right):

https://codereview.chromium.org/1186233004/diff/60001/content/common/common.s...
content/common/common.sb:20: (define perm-dir "PERMITTED_DIR")
Use the full name, permitted-dir.

https://codereview.chromium.org/1186233004/diff/60001/content/common/common.s...
content/common/common.sb:52: (if (param-defined? component-build-workaround)
This seems like it should be a param-true?

https://codereview.chromium.org/1186233004/diff/60001/content/common/sandbox_...
File content/common/sandbox_mac.h (right):

https://codereview.chromium.org/1186233004/diff/60001/content/common/sandbox_...
content/common/sandbox_mac.h:40: // Inserts a boolean into the parameters
key/value map.
Are duplicate keys allowed? What happens in that condition? If it's replacement,
then maybe this should be named Set rather than Insert?

https://codereview.chromium.org/1186233004/diff/60001/content/common/sandbox_...
content/common/sandbox_mac.h:50: 
nit: No blank line after private:

https://codereview.chromium.org/1186233004/diff/60001/content/common/sandbox_...
content/common/sandbox_mac.h:108: 
nit: No blank line after private:.

https://codereview.chromium.org/1186233004/diff/60001/content/common/sandbox_...
File content/common/sandbox_mac.mm (right):

https://codereview.chromium.org/1186233004/diff/60001/content/common/sandbox_...
content/common/sandbox_mac.mm:137: std::pair<std::string, std::string>(key,
value ? "TRUE" : "FALSE"));
Use std::make_pair(), and on line 142.

https://codereview.chromium.org/1186233004/diff/60001/content/common/sandbox_...
File content/common/sandbox_mac_compiler_unittest.mm (right):

https://codereview.chromium.org/1186233004/diff/60001/content/common/sandbox_...
content/common/sandbox_mac_compiler_unittest.mm:142: // Make sure that this
invalid profile results in an error returned
nit: punctuation

https://codereview.chromium.org/1186233004/diff/60001/content/ppapi_plugin/pp...
File content/ppapi_plugin/ppapi.sb (right):

https://codereview.chromium.org/1186233004/diff/60001/content/ppapi_plugin/pp...
content/ppapi_plugin/ppapi.sb:18: (allow file-read* (subpath (string-append
(param homedir-as-literal) "/Library/Fonts")))
Make a function user-homedir-path, since you do this in renderer.sb too?

[Greg K, 2015-06-23 19:05:15]

https://codereview.chromium.org/1186233004/diff/60001/content/common/common.sb
File content/common/common.sb (right):

https://codereview.chromium.org/1186233004/diff/60001/content/common/common.s...
content/common/common.sb:20: (define perm-dir "PERMITTED_DIR")
On 2015/06/22 20:40:22, Robert Sesek wrote:
> Use the full name, permitted-dir.

Done.

https://codereview.chromium.org/1186233004/diff/60001/content/common/common.s...
content/common/common.sb:52: (if (param-defined? component-build-workaround)
On 2015/06/22 20:40:22, Robert Sesek wrote:
> This seems like it should be a param-true?

Actually, component-build-workaround isn't always set, so, it actually checks if
it is defined. However, to avoid confusion going forward, I have turned this
into: "If its defined and its true."

https://codereview.chromium.org/1186233004/diff/60001/content/common/sandbox_...
File content/common/sandbox_mac.h (right):

https://codereview.chromium.org/1186233004/diff/60001/content/common/sandbox_...
content/common/sandbox_mac.h:40: // Inserts a boolean into the parameters
key/value map.
On 2015/06/22 20:40:23, Robert Sesek wrote:
> Are duplicate keys allowed? What happens in that condition? If it's
replacement,
> then maybe this should be named Set rather than Insert?

It will inherit the functionality of the underlying std::map and refuse to do
replacement. This has been changed and commented.

https://codereview.chromium.org/1186233004/diff/60001/content/common/sandbox_...
content/common/sandbox_mac.h:50: 
On 2015/06/22 20:40:22, Robert Sesek wrote:
> nit: No blank line after private:

Done.

https://codereview.chromium.org/1186233004/diff/60001/content/common/sandbox_...
content/common/sandbox_mac.h:108: 
On 2015/06/22 20:40:23, Robert Sesek wrote:
> nit: No blank line after private:.

Done.

https://codereview.chromium.org/1186233004/diff/60001/content/common/sandbox_...
File content/common/sandbox_mac.mm (right):

https://codereview.chromium.org/1186233004/diff/60001/content/common/sandbox_...
content/common/sandbox_mac.mm:137: std::pair<std::string, std::string>(key,
value ? "TRUE" : "FALSE"));
On 2015/06/22 20:40:23, Robert Sesek wrote:
> Use std::make_pair(), and on line 142.

Done.

https://codereview.chromium.org/1186233004/diff/60001/content/common/sandbox_...
File content/common/sandbox_mac_compiler_unittest.mm (right):

https://codereview.chromium.org/1186233004/diff/60001/content/common/sandbox_...
content/common/sandbox_mac_compiler_unittest.mm:142: // Make sure that this
invalid profile results in an error returned
On 2015/06/22 20:40:23, Robert Sesek wrote:
> nit: punctuation

Done, I also fixed other instances in this file that I missed last time.

https://codereview.chromium.org/1186233004/diff/60001/content/ppapi_plugin/pp...
File content/ppapi_plugin/ppapi.sb (right):

https://codereview.chromium.org/1186233004/diff/60001/content/ppapi_plugin/pp...
content/ppapi_plugin/ppapi.sb:18: (allow file-read* (subpath (string-append
(param homedir-as-literal) "/Library/Fonts")))
On 2015/06/22 20:40:23, Robert Sesek wrote:
> Make a function user-homedir-path, since you do this in renderer.sb too?

Done.

[Robert Sesek, 2015-06-25 20:49:27]

https://codereview.chromium.org/1186233004/diff/100001/content/browser/gpu.sb
File content/browser/gpu.sb (right):

https://codereview.chromium.org/1186233004/diff/100001/content/browser/gpu.sb...
content/browser/gpu.sb:26: 
nit: remove extra blank line

https://codereview.chromium.org/1186233004/diff/100001/content/renderer/rende...
File content/renderer/renderer.sb (left):

https://codereview.chromium.org/1186233004/diff/100001/content/renderer/rende...
content/renderer/renderer.sb:14: (allow mach-lookup (global-name
"com.apple.FontObjectsServer"))  ; 10.5.6
Don't we still need this?

[Greg K, 2015-06-25 23:17:47]

https://codereview.chromium.org/1186233004/diff/100001/content/browser/gpu.sb
File content/browser/gpu.sb (right):

https://codereview.chromium.org/1186233004/diff/100001/content/browser/gpu.sb...
content/browser/gpu.sb:26: 
On 2015/06/25 20:49:27, Robert Sesek wrote:
> nit: remove extra blank line

Done.

https://codereview.chromium.org/1186233004/diff/100001/content/renderer/rende...
File content/renderer/renderer.sb (left):

https://codereview.chromium.org/1186233004/diff/100001/content/renderer/rende...
content/renderer/renderer.sb:14: (allow mach-lookup (global-name
"com.apple.FontObjectsServer"))  ; 10.5.6
On 2015/06/25 20:49:27, Robert Sesek wrote:
> Don't we still need this?

Yes, yes we do.

[Robert Sesek, 2015-06-26 16:18:27]

LGTM

[Greg K, 2015-06-29 16:23:52]

The CQ bit was checked by kerrnel@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-06-29 16:24:19]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1186233004/120001

[commit-bot: I haz the power, 2015-06-29 18:10:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-06-29 18:10:13]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Greg K, 2015-06-29 18:12:18]

The CQ bit was checked by kerrnel@chromium.org

[commit-bot: I haz the power, 2015-06-29 18:12:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1186233004/120001

[commit-bot: I haz the power, 2015-06-29 19:13:41]

Committed patchset #6 (id:120001)

[commit-bot: I haz the power, 2015-06-29 19:14:32]

Patchset 6 (id:??) landed as
https://crrev.com/76eaa8778a322803dc3ac20cda05525e615dec4d
Cr-Commit-Position: refs/heads/master@{#336610}

[caseq, 2015-06-30 10:25:52]

A revert of this CL (patchset #6 id:120001) has been created in
https://codereview.chromium.org/1213113006/ by caseq@chromium.org.

The reason for reverting is: Speculative revert to see if this could be a
possible reason for MacOS 10.6 layout test bot failures starting since
http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.6/builds/....

[Greg K, 2015-07-10 23:38:50]

Patchset #7 (id:140001) has been deleted

[Greg K, 2015-07-14 00:48:57]

Patchset #7 (id:160001) has been deleted

[Greg K, 2015-07-14 00:49:04]

Patchset #7 (id:180001) has been deleted

[Greg K, 2015-07-14 00:49:18]

Patchset #7 (id:200001) has been deleted

[Robert Sesek, 2015-07-14 13:17:34]

https://codereview.chromium.org/1186233004/diff/220001/content/common/sandbox...
File content/common/sandbox_mac.h (right):

https://codereview.chromium.org/1186233004/diff/220001/content/common/sandbox...
content/common/sandbox_mac.h:53: // This is the internal definition of the
structure used by sandbox parameters
This can be declared in the .cc file.

https://codereview.chromium.org/1186233004/diff/220001/content/common/sandbox...
File content/common/sandbox_mac.mm (right):

https://codereview.chromium.org/1186233004/diff/220001/content/common/sandbox...
content/common/sandbox_mac.mm:165: SandboxParams* internal_params = new
SandboxParams();
malloc() this since it will be free()d rather than delete()d.

https://codereview.chromium.org/1186233004/diff/220001/content/common/sandbox...
content/common/sandbox_mac.mm:172: if (!checked_size.IsValid() ||
!checked_byte_count.IsValid())
You should perform these checks before allocating the memory, otherwise it will
leak.

https://codereview.chromium.org/1186233004/diff/220001/content/common/sandbox...
content/common/sandbox_mac.mm:175: internal_params->buf =
malloc(checked_byte_count.ValueOrDie());
calloc() here, and then it can do the overflow check.

[Greg K, 2015-07-15 02:57:33]

https://codereview.chromium.org/1186233004/diff/220001/content/common/sandbox...
File content/common/sandbox_mac.h (right):

https://codereview.chromium.org/1186233004/diff/220001/content/common/sandbox...
content/common/sandbox_mac.h:53: // This is the internal definition of the
structure used by sandbox parameters
On 2015/07/14 13:17:34, Robert Sesek wrote:
> This can be declared in the .cc file.

Done.

https://codereview.chromium.org/1186233004/diff/220001/content/common/sandbox...
File content/common/sandbox_mac.mm (right):

https://codereview.chromium.org/1186233004/diff/220001/content/common/sandbox...
content/common/sandbox_mac.mm:165: SandboxParams* internal_params = new
SandboxParams();
On 2015/07/14 13:17:34, Robert Sesek wrote:
> malloc() this since it will be free()d rather than delete()d.

Done.

https://codereview.chromium.org/1186233004/diff/220001/content/common/sandbox...
content/common/sandbox_mac.mm:172: if (!checked_size.IsValid() ||
!checked_byte_count.IsValid())
On 2015/07/14 13:17:34, Robert Sesek wrote:
> You should perform these checks before allocating the memory, otherwise it
will
> leak.

Done.

https://codereview.chromium.org/1186233004/diff/220001/content/common/sandbox...
content/common/sandbox_mac.mm:175: internal_params->buf =
malloc(checked_byte_count.ValueOrDie());
On 2015/07/14 13:17:34, Robert Sesek wrote:
> calloc() here, and then it can do the overflow check.

Done.

[Robert Sesek, 2015-07-15 15:20:18]

https://codereview.chromium.org/1186233004/diff/240001/content/common/sandbox...
File content/common/sandbox_mac.mm (right):

https://codereview.chromium.org/1186233004/diff/240001/content/common/sandbox...
content/common/sandbox_mac.mm:60: struct SandboxParams {
Move this into the content::(anonymous namespace) below.

https://codereview.chromium.org/1186233004/diff/240001/content/common/sandbox...
content/common/sandbox_mac.mm:180:
(SandboxParams*)malloc(sizeof(SandboxParams));
Google style forbids C-style casts, so use static_cast<> here.

http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Casting

[Greg K, 2015-07-15 17:57:14]

https://codereview.chromium.org/1186233004/diff/240001/content/common/sandbox...
File content/common/sandbox_mac.mm (right):

https://codereview.chromium.org/1186233004/diff/240001/content/common/sandbox...
content/common/sandbox_mac.mm:60: struct SandboxParams {
On 2015/07/15 15:20:17, Robert Sesek wrote:
> Move this into the content::(anonymous namespace) below.

Done.

https://codereview.chromium.org/1186233004/diff/240001/content/common/sandbox...
content/common/sandbox_mac.mm:180:
(SandboxParams*)malloc(sizeof(SandboxParams));
On 2015/07/15 15:20:18, Robert Sesek wrote:
> Google style forbids C-style casts, so use static_cast<> here.
> 
> http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Casting

Done.

[Robert Sesek, 2015-07-15 19:49:52]

LGTM

[Greg K, 2015-07-16 15:33:51]

The CQ bit was checked by kerrnel@chromium.org

[commit-bot: I haz the power, 2015-07-16 15:35:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1186233004/260001

[commit-bot: I haz the power, 2015-07-16 15:41:41]

Committed patchset #9 (id:260001)

[commit-bot: I haz the power, 2015-07-16 15:43:38]

Patchset 9 (id:??) landed as
https://crrev.com/e46995fce5cb605f406927ecd0b2eb2018e7e3aa
Cr-Commit-Position: refs/heads/master@{#339038}