Introduce RegistrationServer.

handler/win/registration_server.h

Index: handler/win/registration_server.h
diff --git a/handler/win/registration_server.h b/handler/win/registration_server.h
new file mode 100644
index 0000000000000000000000000000000000000000..288b443900fe59c1d31ef476cb4d72c8932f3a3c
--- /dev/null
+++ b/handler/win/registration_server.h
@@ -0,0 +1,87 @@
+// Copyright 2015 The Crashpad Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef CRASHPAD_HANDLER_WIN_REGISTRATION_SERVER_H_
+#define CRASHPAD_HANDLER_WIN_REGISTRATION_SERVER_H_
+
+#include <vector>
+
+#include "base/basictypes.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/strings/string16.h"
+#include "test/thread.h"

[Mark Mentovai, 2015/05/05 20:20:34]

Can’t depend on test code from non-test code. Thread will need to be hoisted out
of test.

[erikwright (departed), 2015/05/20 17:10:16]

Done.

+#include "util/win/scoped_handle.h"
+
+namespace crashpad {
+
+//! \brief Implements the server side of the Crashpad client registration
+//!     protocol.

[Mark Mentovai, 2015/05/05 20:20:33]

Since this is Windows-specific, the comment should say so. The generated
documentation won’t make it very obvious that this came from handler/win.

[erikwright (departed), 2015/05/20 17:10:16]

Done.

+class RegistrationServer {
+ public:
+  //! \brief Handles registration requests.

[scottmg, 2015/05/05 21:12:27]

Please add something high level here about to whom this is intended to delegate,
and why it shouldn't just have a concrete implementation in Crashpad. (I
generally find Chromium's maze of delegate/host/impls unpleasant.)

[erikwright (departed), 2015/05/20 17:10:16]

This allows me to test the registrar without going through pipes and to verify
the pipe-based protocol without having to also test the other aspects of the
registrar.

For example, I can very trivially verify that an IPC invocation is turned into a
valid process handle with the right permissions. But if this class also
(internally or via direct invocation of a concrete class) acted as the registrar
I would not be able to intercept that process handle.

To be honest, I'm not sure what the comment would say.. "Decoupled for
testability"?

+  class Delegate {
+   public:
+    virtual ~Delegate() {}
+
+    //! \brief Responds to a request to register a client process for crash
+    //!     handling.
+    //!
+    //! \param[in] client_process The client that is making the request.
+    //! \param[in] crashpad_info_address The address of a CrashpadInfo structure
+    //!     in the client's address space.
+    //! \param[out] request_dump_event A HANDLE, valid in the client process, to

[Mark Mentovai, 2015/05/05 20:20:34]

Surround system types in backticks so that they’re set off in the documentation:
`HANDLE`. This isn’t necessary for our own types because they’re automatically
linked.

[erikwright (departed), 2015/05/20 17:10:16]

Done.

+    //!     an event that, when signaled, triggers a crash report.
+    //! \param[out] dump_complete_event A HANDLE, valid in the client process,
+    //!     to an event that will be signaled when the crash report has been
+    //!     successfully captured..

[Mark Mentovai, 2015/05/05 20:20:34]

Double period.

[erikwright (departed), 2015/05/20 17:10:16]

Done.

+    virtual bool RegisterClient(ScopedKernelHANDLE client_process,
+                                size_t crashpad_info_address,

[Mark Mentovai, 2015/05/05 20:20:34]

#include <sys/types.h>

[scottmg, 2015/05/05 21:12:27]

WinVMAddress in util/win/address_types.h would make sense instead of size_t
here.

[erikwright (departed), 2015/05/20 17:10:16]

Acknowledged.

+                                HANDLE* request_dump_event,

[Mark Mentovai, 2015/05/05 20:20:34]

#include <windows.h>

[erikwright (departed), 2015/05/20 17:10:16]

Done.

+                                HANDLE* dump_complete_event) = 0;
+  };
+
+  //! \brief Instantiates a client registration server.
+  //!
+  //! \param[in] pipe_name The pipe name to be used by the server.
+  //! \param[in] delegate The delegate to be used to handle requests.
+  RegistrationServer(const base::string16& pipe_name,
+                     scoped_ptr<Delegate> delegate);
+  ~RegistrationServer();
+
+ private:
+  class ListenerThread : public test::Thread {
+   public:
+    ListenerThread(ScopedKernelHANDLE pipe, Delegate* delegate);
+
+    void Stop();
+
+   private:
+    // test::Thread implementation.

[Mark Mentovai, 2015/05/05 20:20:34]

// test::Thread:

[erikwright (departed), 2015/05/20 17:10:16]

Done.

+    void ThreadMain() override;
+
+    ScopedKernelHANDLE pipe_;
+    Delegate* delegate_;
+
+    DISALLOW_COPY_AND_ASSIGN(ListenerThread);
+  };
+
+  scoped_ptr<Delegate> delegate_;
+  std::vector<ListenerThread*> listener_threads_;
+
+  DISALLOW_COPY_AND_ASSIGN(RegistrationServer);
+};
+
+}  // namespace crashpad
+
+#endif  // CRASHPAD_HANDLER_WIN_REGISTRATION_SERVER_H_