Add NotifyServer and its test

Add NotifyServer and its test.

TEST=util_test NotifyServerTest.*
R=rsesek@chromium.org

Committed: https://chromium.googlesource.com/crashpad/crashpad/+/d78b003ef1fb6efc6dfd7deb5ec9371ddff01a1b

[Mark Mentovai, 2014-12-12 21:02:53]

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

[Mark Mentovai, 2014-12-13 02:51:33]

Patchset #2 (id:20001) has been deleted

[Robert Sesek, 2014-12-15 21:27:17]

Nice test :)

https://codereview.chromium.org/804633002/diff/40001/util/mach/notify_server.h
File util/mach/notify_server.h (right):

https://codereview.chromium.org/804633002/diff/40001/util/mach/notify_server....
util/mach/notify_server.h:38: //! \brief An interface that the request message
that is a part of the
This sentence is awkward.

https://codereview.chromium.org/804633002/diff/40001/util/mach/notify_server....
util/mach/notify_server.h:75: mach_port_t rights,
I'd document all the parameters here. Do I need to destroy this right?

https://codereview.chromium.org/804633002/diff/40001/util/mach/notify_server....
util/mach/notify_server.h:161: 
nit: I don't think you normally put blank lines between overrides.

https://codereview.chromium.org/804633002/diff/40001/util/mach/notify_server_...
File util/mach/notify_server_test.cc (right):

https://codereview.chromium.org/804633002/diff/40001/util/mach/notify_server_...
util/mach/notify_server_test.cc:44: mach_port_t NewReceiveRight() {
Not to do here, but I've long thought about having this as a static method on
ScopedMachReceieveRight…

https://codereview.chromium.org/804633002/diff/40001/util/mach/notify_server_...
util/mach/notify_server_test.cc:99: kern_return_t kr =
mach_port_extract_right(mach_task_self(),
Why is this extract instead of insert?

https://codereview.chromium.org/804633002/diff/40001/util/mach/notify_server_...
util/mach/notify_server_test.cc:160: // gmock’s default is to ignore calls to
mocked methods that don’t have any
I believe you can create a "strict matcher" in gmock that does this.

https://codereview.chromium.org/804633002/diff/40001/util/mach/notify_server_...
util/mach/notify_server_test.cc:462: // user reference count of 1, but the
dead-name notification increments the
It may be good to test the actual ref-count numbers, though it may be hard with
the test structure.

[Mark Mentovai, 2014-12-15 23:04:57]

All feedback addressed.

I though it was important to have the test actually trigger all of the
notifications through the kernel, so that there’s at least some reasonable
example code of how this is all supposed to work.

https://codereview.chromium.org/804633002/diff/40001/util/mach/notify_server.h
File util/mach/notify_server.h (right):

https://codereview.chromium.org/804633002/diff/40001/util/mach/notify_server....
util/mach/notify_server.h:161: 
Robert Sesek wrote:
> nit: I don't think you normally put blank lines between overrides.

Me personally, or style generally? Because I had been putting blanks between
these particular overrides, so I took them out of the other classes to match.

https://codereview.chromium.org/804633002/diff/40001/util/mach/notify_server_...
File util/mach/notify_server_test.cc (right):

https://codereview.chromium.org/804633002/diff/40001/util/mach/notify_server_...
util/mach/notify_server_test.cc:44: mach_port_t NewReceiveRight() {
Robert Sesek wrote:
> Not to do here, but I've long thought about having this as a static method on
> ScopedMachReceieveRight…

That’s a really good idea. We do this enough elsewhere…

https://codereview.chromium.org/804633002/diff/40001/util/mach/notify_server_...
util/mach/notify_server_test.cc:99: kern_return_t kr =
mach_port_extract_right(mach_task_self(),
Robert Sesek wrote:
> Why is this extract instead of insert?

Mach.

Send-once rights always have their own names. The only way to get a new
send-once right is to allocate a new name for it. You could make up a name and
hope it’s not already in use, or you can use mach_port_extract_right() to have
the kernel allocate the name for you.

This is just how send-once rights work. Is it worth a comment?

https://codereview.chromium.org/804633002/diff/40001/util/mach/notify_server_...
util/mach/notify_server_test.cc:462: // user reference count of 1, but the
dead-name notification increments the
Robert Sesek wrote:
> It may be good to test the actual ref-count numbers, though it may be hard
with
> the test structure.

I can augment the matcher to do this.

[Robert Sesek, 2014-12-16 18:49:32]

LGTM

https://codereview.chromium.org/804633002/diff/40001/util/mach/notify_server.h
File util/mach/notify_server.h (right):

https://codereview.chromium.org/804633002/diff/40001/util/mach/notify_server....
util/mach/notify_server.h:161: 
On 2014/12/15 23:04:56, Mark Mentovai wrote:
> Robert Sesek wrote:
> > nit: I don't think you normally put blank lines between overrides.
> 
> Me personally, or style generally? Because I had been putting blanks between
> these particular overrides, so I took them out of the other classes to match.

I meant in the project/according to style.

[Mark Mentovai, 2014-12-16 19:10:22]

Committed patchset #4 (id:80001) manually as
d78b003ef1fb6efc6dfd7deb5ec9371ddff01a1b (presubmit successful).