Add OSSTATUS_LOG API

Add OSSTATUS_LOG API.
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=119511

[Tristan Schmelcher 2, 2012-01-27 19:56:23]

lgtm

http://codereview.chromium.org/9235084/diff/10050/chrome/service/chrome_servi...
File chrome/service/chrome_service_application_mac.mm (left):

http://codereview.chromium.org/9235084/diff/10050/chrome/service/chrome_servi...
chrome/service/chrome_service_application_mac.mm:52: LOG(ERROR) <<
GetMacOSStatusCommentString(status);
Looks like this won't be logged in the new code. Is it not useful?

[Mark Mentovai, 2012-01-27 19:58:41]

http://codereview.chromium.org/9235084/diff/10050/chrome/service/chrome_servi...
File chrome/service/chrome_service_application_mac.mm (left):

http://codereview.chromium.org/9235084/diff/10050/chrome/service/chrome_servi...
chrome/service/chrome_service_application_mac.mm:52: LOG(ERROR) <<
GetMacOSStatusCommentString(status);
Tristan Schmelcher 2 wrote:
> Looks like this won't be logged in the new code. Is it not useful?

It’s usually a sentence describing the error. It’s pretty verbose and not at all
tailored to the operation that was attempted. It’s usually just a copy of the
documentation string you’d get if you Googled the string returned by
GetMacOSStatusErrorString, so I left it out.

[Mark Mentovai, 2012-01-27 20:01:38]

stuartmorgan, scottbyer, sky, wtc, and dmaclach for OWNERS review. Rubber-stamp
is fine.

[Scott Byer, 2012-01-27 20:02:35]

lgtm

[Mark Mentovai, 2012-01-27 23:07:38]

Committed r119511 TBR for OWNERS.

[wtc, 2012-01-28 00:06:26]

Patch Set 4 LGTM.  I only reviewed src/net.

http://codereview.chromium.org/9235084/diff/10050/net/base/x509_cert_types_ma...
File net/base/x509_cert_types_mac.cc (right):

http://codereview.chromium.org/9235084/diff/10050/net/base/x509_cert_types_ma...
net/base/x509_cert_types_mac.cc:206: OSSTATUS_LOG(ERROR, err) << "SecAsn1Decode
name=" << name;
Nit: the message should say SecAsn1Decode failed.

How about
  OSSTATUS_LOG(ERROR, err) << "SecAsn1Decode failed; name=" << name;
?

Looking at this closely, I am not sure about the value of
printing the value of 'name' on failure.  'name' is a pointer
initialized to NULL, so if the function failed, 'name' is
probably still NULL.  So I think it is enough to just say
  OSSTATUS_LOG(ERROR, err) << "SecAsn1Decode failed";

http://codereview.chromium.org/9235084/diff/10050/net/base/x509_certificate_m...
File net/base/x509_certificate_mac.cc (right):

http://codereview.chromium.org/9235084/diff/10050/net/base/x509_certificate_m...
net/base/x509_certificate_mac.cc:1498: OSSTATUS_LOG(ERROR, err) <<
"SecIdentitySearch error ";

Nit: the space at the end of the string isn't needed?

http://codereview.chromium.org/9235084/diff/10050/remoting/host/user_authenti...
File remoting/host/user_authenticator_mac.cc (right):

http://codereview.chromium.org/9235084/diff/10050/remoting/host/user_authenti...
remoting/host/user_authenticator_mac.cc:73: OSSTATUS_LOG(ERROR, status) <<
"AuthorizationCreate";

Nit: should we add "failed" to the error message?
Or do you think "failed" is ambiguous here?  It could be
misinterpreted to mean authorization denied.

[Wez, 2012-01-28 00:11:52]

LGTM remoting/.

http://codereview.chromium.org/9235084/diff/10050/remoting/host/user_authenti...
File remoting/host/user_authenticator_mac.cc (right):

http://codereview.chromium.org/9235084/diff/10050/remoting/host/user_authenti...
remoting/host/user_authenticator_mac.cc:73: OSSTATUS_LOG(ERROR, status) <<
"AuthorizationCreate";
On 2012/01/28 00:06:26, wtc wrote:
> 
> Nit: should we add "failed" to the error message?
> Or do you think "failed" is ambiguous here?  It could be
> misinterpreted to mean authorization denied.

I think there's enough of a distinction between failed and denied for this to
make sense.

[stuartmorgan, 2012-01-28 08:30:42]

Password Manager LGTM