Introduce RegistrationServer.

handler/win/registration_server.cc

+// 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.
+
+#include "handler/win/registration_server.h"
+
+#include <windows.h>
+#include "client/registration_request_win.h"

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

Blank line before this.

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

Done.

+#include "client/registration_response_win.h"
+
+namespace crashpad {
+
+namespace {
+const int kNumThreads = 3;

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

This is only used in the constructor, it doesn’t need to be out here. You can
have a local const in the constructor.

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

Done.

+}  // namespace
+
+RegistrationServer::RegistrationServer(const base::string16& pipe_name,
+                                       scoped_ptr<Delegate> delegate)
+    : delegate_(delegate.Pass()) {

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

Explicitly initialize everything even if it’s the same as default
initialization. listener_threads_() is missing.

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

Done.

+  for (int i = 0; i < kNumThreads; ++i) {
+    DWORD open_mode = PIPE_ACCESS_DUPLEX;
+    if (i == 0)
+      open_mode |= FILE_FLAG_FIRST_PIPE_INSTANCE;
+    ScopedKernelHANDLE pipe(
+        ::CreateNamedPipe(pipe_name.c_str(), open_mode,

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

No :: in crashpad except where it’s needed to disambiguate.

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

Done.

+                          PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE | PIPE_WAIT,
+                          kNumThreads, 512, 512, 20, NULL));

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

What is the significance of 20?

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

In Crashpad, use nullptr, not NULL. Generally we don’t bin-pack like this. We
have a .clang-format that handles things fairly well.

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

Done.

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

Done.

+    if (pipe.is_valid()) {
+      listener_threads_.push_back(
+          new ListenerThread(pipe.Pass(), delegate_.get()));
+      listener_threads_.back()->Start();
+    }
+  }
+}
+
+RegistrationServer::~RegistrationServer() {
+  for (auto& thread : listener_threads_) {
+    thread->Stop();
+  }
+  for (auto& thread : listener_threads_) {
+    thread->Join();
+    delete thread;

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

Use a PointerVector<>.

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

Done.

+  }
+}
+
+RegistrationServer::ListenerThread::ListenerThread(
+    ScopedKernelHANDLE pipe,

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

This would fit on the preceding line.

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

Done.

+    Delegate* delegate)
+    : pipe_(pipe.Pass()),
+      delegate_(delegate) {
+}
+
+void RegistrationServer::ListenerThread::ThreadMain() {
+  while (true) {
+    if (!::ConnectNamedPipe(pipe_.get(), NULL)) {
+      if (::GetLastError() != ERROR_PIPE_CONNECTED)
+        break;

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

PLOG(ERROR)

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

Done.

+    }
+
+    DWORD bytes_read = 0;
+    RegistrationRequest request;
+    if (::ReadFile(pipe_.get(), &request, sizeof(request), &bytes_read, NULL)) {
+      if (bytes_read == sizeof(request)) {
+        ScopedKernelHANDLE client_process(::OpenProcess(
+            PROCESS_ALL_ACCESS, FALSE, request.client_process_id));
+        if (!client_process.is_valid()) {
+          if (::ImpersonateNamedPipeClient(pipe_.get())) {
+            client_process.reset(::OpenProcess(
+                PROCESS_ALL_ACCESS, FALSE, request.client_process_id));
+            ::RevertToSelf();
+          }

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

…otherwise? Is this something that should be logged? Should this break or
continue or return? The same applies to all of the other unexpected cases in
this method.

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

Done.

+        }
+
+        // TODO(erikwright): Verify that the client process ID is not forged.
+
+        if (client_process.is_valid()) {
+          RegistrationResponse response = {0};
+          if (delegate_->RegisterClient(client_process.Pass(),
+                                        request.crashpad_info_address,
+                                        &response.request_report_event,
+                                        &response.report_complete_event)) {
+            DWORD bytes_written = 0;
+            ::WriteFile(

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

Check everything.

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

Done.

+                pipe_.get(), &response, sizeof(response), &bytes_written, NULL);
+          }
+        }
+      }
+    }
+
+    ::DisconnectNamedPipe(pipe_.get());
+  }
+}
+
+void RegistrationServer::ListenerThread::Stop() { pipe_.reset(); }
+
+}  // namespace crashpad
+

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

Get rid of the blank line here.

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

Done.