[Mac] Disable all sandboxd logging unless running with --enable-sandbox-logging.

[Mac] Disable all sandboxd logging unless running with --enable-sandbox-logging.
This prevents console spew.

BUG=26621
TEST=Run Chrome and don't get messages in Console.app from sandboxd about Chrome Helper. Run with --enable-sandbox-logging and get them.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=57191

[jeremy, 2010-08-22 07:14:34]

http://codereview.chromium.org/3155031/diff/3001/4001
File chrome/common/common.sb (right):

http://codereview.chromium.org/3155031/diff/3001/4001#newcode9
chrome/common/common.sb:9: ; --enable-sandbox-logging explicitly.
1. Suggest a more verbose name for the variable such as
DISABLE_SANDBOX_DENIAL_LOGGING

2.
I'd remove this sentence and instead add something like the following separate
comment after "version 1":

"""
http://crbug.com/26621
DISABLE_SANDBOX_DENIAL_LOGGING expands to syntax that turns off log message
printing on sandbox exceptions - this functionality only exists on 10.6. The
--enable-sandbox-logging flag or system versions <10.6 cause this flag to expand
to the empty string.
"""

http://codereview.chromium.org/3155031/diff/3001/4001#newcode14
chrome/common/common.sb:14: ;ENABLE_LOGGING (debug all)
Did you check that this syntax works on 10.5 & 10.6?
What kind of a difference does this make in the output?

http://codereview.chromium.org/3155031/diff/3001/4002
File chrome/common/sandbox_mac.mm (right):

http://codereview.chromium.org/3155031/diff/3001/4002#newcode321
chrome/common/sandbox_mac.mm:321: const CommandLine *command_line =
CommandLine::ForCurrentProcess();
How about flipping the logic a bit here:
if (snow_leopard_or_higher && !HasSwitch(...)) {
  // replace with (with no-log).
} else {
  // replace with empty string.
}

http://codereview.chromium.org/3155031/diff/3001/4002#newcode331
chrome/common/sandbox_mac.mm:331: // they flood the Console with useless spew.
|no-log| is 10.6+ only.
How about something like:
"Without this, the sandbox will print a message to the system log every time it
denies a request.  This floods the console with useless spew. the (with no-log)
syntax is only supported on 10.6+"

[Robert Sesek, 2010-08-22 17:45:29]

Thanks for the review.  All comments addressed.

http://codereview.chromium.org/3155031/diff/3001/4001
File chrome/common/common.sb (right):

http://codereview.chromium.org/3155031/diff/3001/4001#newcode14
chrome/common/common.sb:14: ;ENABLE_LOGGING (debug all)
On 2010/08/22 07:14:34, jeremy wrote:
> Did you check that this syntax works on 10.5 & 10.6?
> What kind of a difference does this make in the output?
> 

Per the documentation here
http://www.opensource.apple.com/source/mDNSResponder/mDNSResponder-176.3/mDNS...
(from 10.5.7), there should be a difference.  |all| logs everything, whereas
|deny| only logs denials.  In practice, I could not discern a difference.

http://codereview.chromium.org/3155031/diff/3001/4002
File chrome/common/sandbox_mac.mm (right):

http://codereview.chromium.org/3155031/diff/3001/4002#newcode321
chrome/common/sandbox_mac.mm:321: const CommandLine *command_line =
CommandLine::ForCurrentProcess();
On 2010/08/22 07:14:34, jeremy wrote:
> How about flipping the logic a bit here:
> if (snow_leopard_or_higher && !HasSwitch(...)) {
>   // replace with (with no-log).
> } else {
>   // replace with empty string.
> }

Done.

http://codereview.chromium.org/3155031/diff/3001/4002#newcode331
chrome/common/sandbox_mac.mm:331: // they flood the Console with useless spew.
|no-log| is 10.6+ only.
On 2010/08/22 07:14:34, jeremy wrote:
> How about something like:
> "Without this, the sandbox will print a message to the system log every time
it
> denies a request.  This floods the console with useless spew. the (with
no-log)
> syntax is only supported on 10.6+"

Done.

[jeremy, 2010-08-23 07:34:41]

http://codereview.chromium.org/3155031/diff/3001/4001
File chrome/common/common.sb (right):

http://codereview.chromium.org/3155031/diff/3001/4001#newcode14
chrome/common/common.sb:14: ;ENABLE_LOGGING (debug all)
That says:
"""
Use "debug all" to log all operations examined by seatbelt, whether allowed or
not.
; Use "debug deny" to log only operations that are denied by seatbelt
"""

I'd prefer not to change this since we're only interested in logging cases when
the Sandbox denies an operation.  If we find a case where this is needed we can
change it later.

http://codereview.chromium.org/3155031/diff/7002/9001#newcode10
chrome/common/common.sb:10: (version 1)
Could you go with the wording I suggested or otherwise change this to make the
following points clearer:

1. "suppress all deny messages" isn't very clear, the important point is that
without this the sandbox will log a console message any time it denies a request
causing the console log to flood.
2. If the --enable-sandbox-logging flag is enabled, or we're running on OS 10.5
this will just be replaced with an empty string.

Sorry to press this, but it's important that this be easy to understand for
someone not familiar with the sandbox configuration.

[Robert Sesek, 2010-08-24 00:25:14]

Done.  Reverted to (debug deny) and used your comment.

[jeremy, 2010-08-24 00:45:44]

LGTM