Add pipe implementation for OOP memory logging

chrome/profiling/memlog_receiver_pipe_server_win.cc

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/profiling/memlog_receiver_pipe_server_win.h"
+
+#include "base/bind.h"
+#include "base/memory/ptr_util.h"
+#include "base/message_loop/message_loop.h"
+#include "base/strings/utf_string_conversions.h"
+#include "chrome/common/profiling/memlog_stream.h"
+
+namespace profiling {
+
+namespace {
+
+// Use a large buffer for our pipe. We don't want the sender to block
+// if at all possible since it will slow the app down quite a bit. Windows
+// seems to max out a 64K per read so we use that (larger would be better if it
+// worked).

[awong, 2017/06/15 21:32:47]

Cite source for this information?

[brettw, 2017/06/15 22:12:22]

Done.

+const int kReadBufferSize = 1024 * 64;
+
+}  // namespace
+
+MemlogReceiverPipeServer::MemlogReceiverPipeServer(
+    base::TaskRunner* io_runner,
+    const std::string& pipe_id,
+    NewConnectionCallback on_new_conn)
+    : io_runner_(io_runner),
+      pipe_id_(base::ASCIIToUTF16(pipe_id)),
+      on_new_connection_(on_new_conn) {}
+
+MemlogReceiverPipeServer::~MemlogReceiverPipeServer() {
+  // TODO(brettw) we should ensure this destructor is not called when there are
+  // pending I/O operations as per RegisterIOHandler documentation.
+}
+
+void MemlogReceiverPipeServer::Start() {
+  // It's important that the first call to CreatePipeInstance be here (which
+  // is called on the main thread) instead of on the background thread.
+  // Otherwise, the creation of the first pipe instance on the background
+  // thread and the launching of the child process will race. But the child
+  // process expects the pipe to already have been created.

[awong, 2017/06/15 21:32:47]

This is a great comment...but I don't see how this is the right place to put
this comment?  Should this be API documentation in the header explaining when
Start() be called?

[brettw, 2017/06/15 22:12:22]

Thanks for noticing, the comment is wrong. It explains why it can't be done on a
background thread right before it posts to the background thread. I wrote that
when I thought this race was causing a problem.

I'll put a TODO in since this is suspicious, but I think this will be moot when
I change the process topology as promised (to avoid launching the browser from
the logging process).

+  io_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&MemlogReceiverPipeServer::ScheduleNewConnection, this, true));
+}
+
+base::string16 MemlogReceiverPipeServer::GetPipeName() const {
+  base::string16 pipe_name(kWindowsPipePrefix);
+  pipe_name.append(pipe_id_);
+  return pipe_name;
+}
+
+HANDLE MemlogReceiverPipeServer::CreatePipeInstance(bool first_instance) const {
+  base::string16 pipe_name = GetPipeName();
+
+  DWORD open_mode = PIPE_ACCESS_DUPLEX | FILE_FLAG_OVERLAPPED;
+  if (first_instance)
+    open_mode |= FILE_FLAG_FIRST_PIPE_INSTANCE;
+
+  return ::CreateNamedPipe(
+      pipe_name.c_str(), open_mode, PIPE_TYPE_BYTE | PIPE_READMODE_BYTE,
+      PIPE_UNLIMITED_INSTANCES, kReadBufferSize, kReadBufferSize, 5000, NULL);

[awong, 2017/06/15 21:32:47]

kBufferSize seems more appropriate if you're using the value for both
read/write.

[brettw, 2017/06/15 22:12:21]

Done.

+}
+
+void MemlogReceiverPipeServer::ScheduleNewConnection(bool first_instance) {
+  DCHECK(!current_);
+  // Need Unretained to avoid creating a reference cycle.
+  current_ = base::MakeUnique<MemlogReceiverPipe::CompletionThunk>(
+      CreatePipeInstance(first_instance),
+      base::BindRepeating(&MemlogReceiverPipeServer::OnIOCompleted,
+                          base::Unretained(this)));
+  ::ConnectNamedPipe(current_->handle(), current_->overlapped());
+}
+
+void MemlogReceiverPipeServer::OnIOCompleted(size_t bytes_transfered,
+                                             DWORD error) {
+  scoped_refptr<MemlogReceiverPipe> pipe(
+      new MemlogReceiverPipe(std::move(current_)));
+  ScheduleNewConnection(false);
+
+  if (!on_new_connection_.is_null())
+    on_new_connection_.Run(pipe);
+  pipe->StartReadingOnIOThread();
+}
+
+}  // namespace profiling