Merge master cd0e25f1ba1d into doc

client/crash_report_database_mac.mm

@@ -94,6 +94,15 @@ bool CreateOrEnsureDirectoryExists(const base::FilePath& path) {
   return EnsureDirectoryExists(path);
 }
 
+// Creates a long database xattr name from the short constant name. These names
+// have changed, and new_name determines whether the returned xattr name will be
+// the old name or its new equivalent.
+std::string XattrNameInternal(const base::StringPiece& name, bool new_name) {
+  return base::StringPrintf(new_name ? "org.chromium.crashpad.database.%s"
+                                     : "com.googlecode.crashpad.%s",
+                            name.data());
+}
+
 //! \brief A CrashReportDatabase that uses HFS+ extended attributes to store
 //!     report metadata.
 //!
@@ -173,8 +182,7 @@ class CrashReportDatabaseMac : public CrashReportDatabase {
   //!
   //! \return `true` if all the metadata was read successfully, `false`
   //!     otherwise.
-  static bool ReadReportMetadataLocked(const base::FilePath& path,
+  bool ReadReportMetadataLocked(const base::FilePath& path, Report* report);
-                                       Report* report);
 
   //! \brief Reads the metadata from all the reports in a database subdirectory.
   //!      Invalid reports are skipped.
@@ -183,18 +191,19 @@ class CrashReportDatabaseMac : public CrashReportDatabase {
   //! \param[out] reports An empty vector of reports, which will be filled.
   //!
   //! \return The operation status code.
-  static OperationStatus ReportsInDirectory(const base::FilePath& path,
+  OperationStatus ReportsInDirectory(const base::FilePath& path,
-                                            std::vector<Report>* reports);
+                                     std::vector<Report>* reports);
 
   //! \brief Creates a database xattr name from the short constant name.
   //!
   //! \param[in] name The short name of the extended attribute.
   //!
   //! \return The long name of the extended attribute.
-  static std::string XattrName(const base::StringPiece& name);
+  std::string XattrName(const base::StringPiece& name);
 
   base::FilePath base_dir_;
   Settings settings_;
+  bool xattr_new_names_;
   InitializationStateDcheck initialized_;
 
   DISALLOW_COPY_AND_ASSIGN(CrashReportDatabaseMac);
@@ -204,6 +213,7 @@ CrashReportDatabaseMac::CrashReportDatabaseMac(const base::FilePath& path)
     : CrashReportDatabase(),
       base_dir_(path),
       settings_(base_dir_.Append(kSettings)),
+      xattr_new_names_(false),
       initialized_() {
 }
 
@@ -230,10 +240,25 @@ bool CrashReportDatabaseMac::Initialize(bool may_create) {
   if (!settings_.Initialize())
     return false;
 
-  // Write an xattr as the last step, to ensure the filesystem has support for
+  // Do an xattr operation as the last step, to ensure the filesystem has
-  // them. This attribute will never be read.
+  // support for them. This xattr also serves as a marker for whether the
-  if (!WriteXattrBool(base_dir_, XattrName(kXattrDatabaseInitialized), true))
+  // database uses old or new xattr names.
-    return false;
+  bool value;
+  if (ReadXattrBool(base_dir_,
+                    XattrNameInternal(kXattrDatabaseInitialized, true),
+                    &value) == XattrStatus::kOK &&
+      value) {
+    xattr_new_names_ = true;
+  } else if (ReadXattrBool(base_dir_,
+                           XattrNameInternal(kXattrDatabaseInitialized, false),
+                           &value) == XattrStatus::kOK &&
+             value) {
+    xattr_new_names_ = false;
+  } else {
+    xattr_new_names_ = true;
+    if (!WriteXattrBool(base_dir_, XattrName(kXattrDatabaseInitialized), true))
+      return false;
+  }
 
   INITIALIZATION_STATE_SET_VALID(initialized_);
   return true;
@@ -540,7 +565,6 @@ base::ScopedFD CrashReportDatabaseMac::ObtainReportLock(
   return base::ScopedFD(fd);
 }
 
-// static
 bool CrashReportDatabaseMac::ReadReportMetadataLocked(
     const base::FilePath& path, Report* report) {
   std::string uuid_string;
@@ -583,7 +607,6 @@ bool CrashReportDatabaseMac::ReadReportMetadataLocked(
   return true;
 }
 
-// static
 CrashReportDatabase::OperationStatus CrashReportDatabaseMac::ReportsInDirectory(
     const base::FilePath& path,
     std::vector<CrashReportDatabase::Report>* reports) {
@@ -620,9 +643,8 @@ CrashReportDatabase::OperationStatus CrashReportDatabaseMac::ReportsInDirectory(
   return kNoError;
 }
 
-// static
 std::string CrashReportDatabaseMac::XattrName(const base::StringPiece& name) {
-  return base::StringPrintf("com.googlecode.crashpad.%s", name.data());
+  return XattrNameInternal(name, xattr_new_names_);
 }
 
 scoped_ptr<CrashReportDatabase> InitializeInternal(const base::FilePath& path,

client/crashpad_client_mac.cc

@@ -19,11 +19,13 @@
 #include <unistd.h>
 
 #include "base/logging.h"
+#include "base/mac/mach_logging.h"
 #include "base/posix/eintr_wrapper.h"
 #include "base/strings/stringprintf.h"
 #include "util/mach/child_port_handshake.h"
 #include "util/mach/exception_ports.h"
 #include "util/mach/mach_extensions.h"
+#include "util/misc/implicit_cast.h"
 #include "util/posix/close_multiple.h"
 
 namespace crashpad {
@@ -77,6 +79,199 @@ bool SetCrashExceptionPorts(exception_handler_t exception_handler) {
       MACHINE_THREAD_STATE);
 }
 
+//! \brief Starts a Crashpad handler.
+class HandlerStarter final {
+ public:
+  //! \brief Starts a Crashpad handler.
+  //!
+  //! All parameters are as in CrashpadClient::StartHandler().
+  //!
+  //! \return On success, a send right to the Crashpad handler that has been
+  //!     started. On failure, `MACH_PORT_NULL` with a message logged.
+  static base::mac::ScopedMachSendRight Start(
+      const base::FilePath& handler,
+      const base::FilePath& database,
+      const std::string& url,
+      const std::map<std::string, std::string>& annotations,
+      const std::vector<std::string>& arguments) {
+    base::mac::ScopedMachReceiveRight receive_right(
+        NewMachPort(MACH_PORT_RIGHT_RECEIVE));
+    if (receive_right == kMachPortNull) {
+      return base::mac::ScopedMachSendRight();
+    }
+
+    mach_port_t port;
+    mach_msg_type_name_t right_type;
+    kern_return_t kr = mach_port_extract_right(mach_task_self(),
+                                               receive_right.get(),
+                                               MACH_MSG_TYPE_MAKE_SEND,
+                                               &port,
+                                               &right_type);
+    if (kr != KERN_SUCCESS) {
+      MACH_LOG(ERROR, kr) << "mach_port_extract_right";
+      return base::mac::ScopedMachSendRight();
+    }
+    base::mac::ScopedMachSendRight send_right(port);
+    DCHECK_EQ(port, receive_right.get());
+    DCHECK_EQ(right_type,
+              implicit_cast<mach_msg_type_name_t>(MACH_MSG_TYPE_PORT_SEND));
+
+    if (!CommonStart(handler,
+                     database,
+                     url,
+                     annotations,
+                     arguments,
+                     receive_right.Pass())) {
+      return base::mac::ScopedMachSendRight();
+    }
+
+    return send_right;
+  }
+
+ private:
+  static bool CommonStart(const base::FilePath& handler,
+                          const base::FilePath& database,
+                          const std::string& url,
+                          const std::map<std::string, std::string>& annotations,
+                          const std::vector<std::string>& arguments,
+                          base::mac::ScopedMachReceiveRight receive_right) {
+    // Set up the arguments for execve() first. These aren’t needed until
+    // execve() is called, but it’s dangerous to do this in a child process
+    // after fork().
+    ChildPortHandshake child_port_handshake;
+    base::ScopedFD server_write_fd = child_port_handshake.ServerWriteFD();
+
+    // Use handler as argv[0], followed by arguments directed by this method’s
+    // parameters and a --handshake-fd argument. |arguments| are added first so
+    // that if it erroneously contains an argument such as --url, the actual
+    // |url| argument passed to this method will supersede it. In normal
+    // command-line processing, the last parameter wins in the case of a
+    // conflict.
+    std::vector<std::string> argv(1, handler.value());
+    argv.reserve(1 + arguments.size() + 2 + annotations.size() + 1);
+    for (const std::string& argument : arguments) {
+      argv.push_back(argument);
+    }
+    if (!database.value().empty()) {
+      argv.push_back(FormatArgumentString("database", database.value()));
+    }
+    if (!url.empty()) {
+      argv.push_back(FormatArgumentString("url", url));
+    }
+    for (const auto& kv : annotations) {
+      argv.push_back(
+          FormatArgumentString("annotation", kv.first + '=' + kv.second));
+    }
+    argv.push_back(FormatArgumentInt("handshake-fd", server_write_fd.get()));
+
+    const char* handler_c = handler.value().c_str();
+
+    // argv_c contains const char* pointers and is terminated by nullptr. argv
+    // is required because the pointers in argv_c need to point somewhere, and
+    // they can’t point to temporaries such as those returned by
+    // FormatArgumentString().
+    std::vector<const char*> argv_c;
+    argv_c.reserve(argv.size() + 1);
+    for (const std::string& argument : argv) {
+      argv_c.push_back(argument.c_str());
+    }
+    argv_c.push_back(nullptr);
+
+    // Double-fork(). The three processes involved are parent, child, and
+    // grandchild. The grandchild will become the handler process. The child
+    // exits immediately after spawning the grandchild, so the grandchild
+    // becomes an orphan and its parent process ID becomes 1. This relieves the
+    // parent and child of the responsibility for reaping the grandchild with
+    // waitpid() or similar. The handler process is expected to outlive the
+    // parent process, so the parent shouldn’t be concerned with reaping it.
+    // This approach means that accidental early termination of the handler
+    // process will not result in a zombie process.
+    pid_t pid = fork();
+    if (pid < 0) {
+      PLOG(ERROR) << "fork";
+      return false;
+    }
+
+    if (pid == 0) {
+      // Child process.
+
+      // Call setsid(), creating a new process group and a new session, both led
+      // by this process. The new process group has no controlling terminal.
+      // This disconnects it from signals generated by the parent process’
+      // terminal.
+      //
+      // setsid() is done in the child instead of the grandchild so that the
+      // grandchild will not be a session leader. If it were a session leader,
+      // an accidental open() of a terminal device without O_NOCTTY would make
+      // that terminal the controlling terminal.
+      //
+      // It’s not desirable for the handler to have a controlling terminal. The
+      // handler monitors clients on its own and manages its own lifetime,
+      // exiting when it loses all clients and when it deems it appropraite to
+      // do so. It may serve clients in different process groups or sessions
+      // than its original client, and receiving signals intended for its
+      // original client’s process group could be harmful in that case.
+      PCHECK(setsid() != -1) << "setsid";
+
+      pid = fork();
+      if (pid < 0) {
+        PLOG(FATAL) << "fork";
+      }
+
+      if (pid > 0) {
+        // Child process.
+
+        // _exit() instead of exit(), because fork() was called.
+        _exit(EXIT_SUCCESS);
+      }
+
+      // Grandchild process.
+
+      CloseMultipleNowOrOnExec(STDERR_FILENO + 1, server_write_fd.get());
+
+      // &argv_c[0] is a pointer to a pointer to const char data, but because of
+      // how C (not C++) works, execvp() wants a pointer to a const pointer to
+      // char data. It modifies neither the data nor the pointers, so the
+      // const_cast is safe.
+      execvp(handler_c, const_cast<char* const*>(&argv_c[0]));
+      PLOG(FATAL) << "execvp " << handler_c;
+    }
+
+    // Parent process.
+
+    // Close the write side of the pipe, so that the handler process is the only
+    // process that can write to it.
+    server_write_fd.reset();
+
+    // waitpid() for the child, so that it does not become a zombie process. The
+    // child normally exits quickly.
+    int status;
+    pid_t wait_pid = HANDLE_EINTR(waitpid(pid, &status, 0));
+    PCHECK(wait_pid != -1) << "waitpid";
+    DCHECK_EQ(wait_pid, pid);
+
+    if (WIFSIGNALED(status)) {
+      LOG(WARNING) << "intermediate process: signal " << WTERMSIG(status);
+    } else if (!WIFEXITED(status)) {
+      DLOG(WARNING) << "intermediate process: unknown termination " << status;
+    } else if (WEXITSTATUS(status) != EXIT_SUCCESS) {
+      LOG(WARNING) << "intermediate process: exit status "
+                   << WEXITSTATUS(status);
+    }
+
+    // Rendezvous with the handler running in the grandchild process.
+    if (!child_port_handshake.RunClient(receive_right.get(),
+                                        MACH_MSG_TYPE_MOVE_RECEIVE)) {
+      return false;
+    }
+
+    ignore_result(receive_right.release());
+    return true;
+  }
+
+  DISALLOW_IMPLICIT_CONSTRUCTORS(HandlerStarter);
+};
+
 }  // namespace
 
 CrashpadClient::CrashpadClient()
@@ -94,124 +289,13 @@ bool CrashpadClient::StartHandler(
     const std::vector<std::string>& arguments) {
   DCHECK(!exception_port_.is_valid());
 
-  // Set up the arguments for execve() first. These aren’t needed until execve()
+  exception_port_ = HandlerStarter::Start(
-  // is called, but it’s dangerous to do this in a child process after fork().
+      handler, database, url, annotations, arguments);
-  ChildPortHandshake child_port_handshake;
+  if (!exception_port_.is_valid()) {
-  int handshake_fd = child_port_handshake.ReadPipeFD();
-
-  // Use handler as argv[0], followed by arguments directed by this method’s
-  // parameters and a --handshake-fd argument. |arguments| are added first so
-  // that if it erroneously contains an argument such as --url, the actual |url|
-  // argument passed to this method will supersede it. In normal command-line
-  // processing, the last parameter wins in the case of a conflict.
-  std::vector<std::string> argv(1, handler.value());
-  argv.reserve(1 + arguments.size() + 2 + annotations.size() + 1);
-  for (const std::string& argument : arguments) {
-    argv.push_back(argument);
-  }
-  if (!database.value().empty()) {
-    argv.push_back(FormatArgumentString("database", database.value()));
-  }
-  if (!url.empty()) {
-    argv.push_back(FormatArgumentString("url", url));
-  }
-  for (const auto& kv : annotations) {
-    argv.push_back(
-        FormatArgumentString("annotation", kv.first + '=' + kv.second));
-  }
-  argv.push_back(FormatArgumentInt("handshake-fd", handshake_fd));
-
-  // argv_c contains const char* pointers and is terminated by nullptr. argv
-  // is required because the pointers in argv_c need to point somewhere, and
-  // they can’t point to temporaries such as those returned by
-  // FormatArgumentString().
-  std::vector<const char*> argv_c;
-  argv_c.reserve(argv.size() + 1);
-  for (const std::string& argument : argv) {
-    argv_c.push_back(argument.c_str());
-  }
-  argv_c.push_back(nullptr);
-
-  // Double-fork(). The three processes involved are parent, child, and
-  // grandchild. The grandchild will become the handler process. The child exits
-  // immediately after spawning the grandchild, so the grandchild becomes an
-  // orphan and its parent process ID becomes 1. This relieves the parent and
-  // child of the responsibility for reaping the grandchild with waitpid() or
-  // similar. The handler process is expected to outlive the parent process, so
-  // the parent shouldn’t be concerned with reaping it. This approach means that
-  // accidental early termination of the handler process will not result in a
-  // zombie process.
-  pid_t pid = fork();
-  if (pid < 0) {
-    PLOG(ERROR) << "fork";
     return false;
   }
 
-  if (pid == 0) {
+  return true;
-    // Child process.
-
-    // Call setsid(), creating a new process group and a new session, both led
-    // by this process. The new process group has no controlling terminal. This
-    // disconnects it from signals generated by the parent process’ terminal.
-    //
-    // setsid() is done in the child instead of the grandchild so that the
-    // grandchild will not be a session leader. If it were a session leader, an
-    // accidental open() of a terminal device without O_NOCTTY would make that
-    // terminal the controlling terminal.
-    //
-    // It’s not desirable for the handler to have a controlling terminal. The
-    // handler monitors clients on its own and manages its own lifetime, exiting
-    // when it loses all clients and when it deems it appropraite to do so. It
-    // may serve clients in different process groups or sessions than its
-    // original client, and receiving signals intended for its original client’s
-    // process group could be harmful in that case.
-    PCHECK(setsid() != -1) << "setsid";
-
-    pid = fork();
-    if (pid < 0) {
-      PLOG(FATAL) << "fork";
-    }
-
-    if (pid > 0) {
-      // Child process.
-
-      // _exit() instead of exit(), because fork() was called.
-      _exit(EXIT_SUCCESS);
-    }
-
-    // Grandchild process.
-
-    CloseMultipleNowOrOnExec(STDERR_FILENO + 1, handshake_fd);
-
-    // &argv_c[0] is a pointer to a pointer to const char data, but because of
-    // how C (not C++) works, execvp() wants a pointer to a const pointer to
-    // char data. It modifies neither the data nor the pointers, so the
-    // const_cast is safe.
-    execvp(handler.value().c_str(), const_cast<char* const*>(&argv_c[0]));
-    PLOG(FATAL) << "execvp " << handler.value();
-  }
-
-  // Parent process.
-
-  // waitpid() for the child, so that it does not become a zombie process. The
-  // child normally exits quickly.
-  int status;
-  pid_t wait_pid = HANDLE_EINTR(waitpid(pid, &status, 0));
-  PCHECK(wait_pid != -1) << "waitpid";
-  DCHECK_EQ(wait_pid, pid);
-
-  if (WIFSIGNALED(status)) {
-    LOG(WARNING) << "intermediate process: signal " << WTERMSIG(status);
-  } else if (!WIFEXITED(status)) {
-    DLOG(WARNING) << "intermediate process: unknown termination " << status;
-  } else if (WEXITSTATUS(status) != EXIT_SUCCESS) {
-    LOG(WARNING) << "intermediate process: exit status " << WEXITSTATUS(status);
-  }
-
-  // Rendezvous with the handler running in the grandchild process.
-  exception_port_.reset(child_port_handshake.RunServer());
-
-  return exception_port_.is_valid();
 }
 
 bool CrashpadClient::UseHandler() {

client/prune_crash_reports.cc

@@ -59,10 +59,9 @@ void PruneCrashReportDatabase(CrashReportDatabase* database,
     }
   }
 
-  // TODO(rsesek): For databases that do not use a directory structure,
+  // TODO(rsesek): For databases that do not use a directory structure, it is
-  // it is possible for the metadata sidecar to become corrupted and thus
+  // possible for the metadata sidecar to become corrupted and thus leave
-  // leave orphaned crash report files on-disk.
+  // orphaned crash report files on-disk. https://crashpad.chromium.org/bug/66
-  // https://code.google.com/p/crashpad/issues/detail?id=66
 }
 
 // static

doc/appengine/README

@@ -1,4 +1,4 @@
-This is the App Engine app that serves https://crashpad-home.appspot.com/.
+This is the App Engine app that serves https://crashpad.chromium.org/.
 
 To work on this app, obtain the App Engine SDK for Go from
 https://cloud.google.com/appengine/downloads. Unpacking it produces a

doc/developing.ad

@@ -83,7 +83,7 @@ $ *gclient sync*
 
 == Building
 
-Crashpad uses https://gyp.googlecode.com/[GYP] to generate
+Crashpad uses https://gyp.gsrc.io/[GYP] to generate
 https://martine.github.io/ninja/[Ninja] build files. The build is described by
 `.gyp` files throughout the Crashpad source code tree. The
 `build/gyp_crashpad.py` script runs GYP properly for Crashpad, and is also
@@ -106,12 +106,13 @@ need to install it separately.
 
 == Testing
 
-Crashpad uses https://googletest.googlecode.com/[Google Test] as its
+Crashpad uses https://github.com/google/googletest/[Google Test] as its
 unit-testing framework, and some tests use
-https://googlemock.googlecode.com/[Google Mock] as well. Its tests are currently
+https://github.com/google/googletest/tree/master/googlemock/[Google Mock] as
-split up into several test executables, each dedicated to testing a different
+well. Its tests are currently split up into several test executables, each
-component. This may change in the future. After a successful build, the test
+dedicated to testing a different component. This may change in the future. After
-executables will be found at `out/Debug/crashpad_*_test`.
+a successful build, the test executables will be found at
+`out/Debug/crashpad_*_test`.
 
 [subs="verbatim,quotes"]
 ----

doc/index.ad

@@ -16,16 +16,16 @@
 
 = Crashpad
 
-https://crashpad-home.appspot.com/[Crashpad] is a crash-reporting system.
+https://crashpad.chromium.org/[Crashpad] is a crash-reporting system.
 
 == Documentation
 
  * link:status.html[Project status]
  * link:developing.html[Developing Crashpad]: instructions for getting the
    source code, building, testing, and contributing to the project.
- * https://crashpad-home.appspot.com/doxygen/index.html[Crashpad interface
+ * https://crashpad.chromium.org/doxygen/index.html[Crashpad interface
    documentation]
- * https://crashpad-home.appspot.com/man/index.html[Crashpad tool man pages]
+ * https://crashpad.chromium.org/man/index.html[Crashpad tool man pages]
 
 == Source Code
 
@@ -34,8 +34,8 @@ https://chromium.googlesource.com/crashpad/crashpad.
 
 == Other Links
 
- * Bugs can be reported at the
+ * Bugs can be reported at the https://crashpad.chromium.org/bug/new[Crashpad
-   https://code.google.com/p/crashpad/issues/entry[Crashpad issue tracker].
+   issue tracker].
  * The https://build.chromium.org/p/client.crashpad[Crashpad Buildbot] performs
    automated builds and tests.
  * https://groups.google.com/a/chromium.org/group/crashpad-dev[crashpad-dev] is

doc/status.ad

@@ -28,18 +28,17 @@ https://chromium.googlesource.com/chromium/src/+/d413b2dcb54d523811d386f1ff4084f
 == In Progress
 
 Crashpad is actively being bootstrapped on
-https://code.google.com/p/crashpad/issues/detail?id=1[Windows]. All major
+https://crashpad.chromium.org/bug/1[Windows]. All major components are now
-components are now working on Windows, and integration into Chromium is expected
+working on Windows, and integration into Chromium is expected shortly.
-shortly.
 
 Initial work on a Crashpad client for
-https://code.google.com/p/crashpad/issues/detail?id=30[Android] has begun. This
+https://crashpad.chromium.org/bug/30[Android] has begun. This is currently in
-is currently in the design phase.
+the design phase.
 
 == Future
 
 There are plans to bring Crashpad clients to other operating systems in the
 future, including a more generic non-Android Linux implementation. There are
-also plans to implement a
+also plans to implement a https://crashpad.chromium.org/bug/29[crash report
-https://code.google.com/p/crashpad/issues/detail?id=29[crash report processor]
+processor] as part of Crashpad. No timeline for completing this work has been
-as part of Crashpad. No timeline for completing this work has been set yet.
+set yet.

doc/support/generate.sh

@@ -43,7 +43,7 @@ done
 
 # Move doc/index.html to index.html, adjusting relative paths to other files in
 # doc.
-base_url=https://crashpad-home.appspot.com/
+base_url=https://crashpad.chromium.org/
 ${sed_ext} -e 's%<a href="([^/]+)\.html">%<a href="doc/\1.html">%g' \
     -e 's%<a href="'"${base_url}"'">%<a href="index.html">%g' \
     -e 's%<a href="'"${base_url}"'%<a href="%g' \

doc/support/man_footer.ad

@@ -14,9 +14,9 @@
 
 == Resources
 
-Crashpad home page: https://crashpad-home.appspot.com/.
+Crashpad home page: https://crashpad.chromium.org/.
 
-Report bugs at https://code.google.com/p/crashpad/issues/entry.
+Report bugs at https://crashpad.chromium.org/bug/new.
 
 == Copyright
 

handler/mac/exception_handler_server.cc

@@ -125,7 +125,7 @@ class ExceptionHandlerServerRun : public UniversalMachExcServer::Interface,
                                    bool* destroy_complex_request) override {
     if (exception_port != exception_port_) {
       LOG(WARNING) << "exception port mismatch";
-      return MIG_BAD_ID;
+      return KERN_FAILURE;
     }
 
     return exception_interface_->CatchMachException(behavior,
@@ -172,7 +172,7 @@ class ExceptionHandlerServerRun : public UniversalMachExcServer::Interface,
       // to craft and send a no-senders notification via its exception port, and
       // cause the handler to stop processing exceptions and exit.
       LOG(WARNING) << "notify port mismatch";
-      return MIG_BAD_ID;
+      return KERN_FAILURE;
     }
 
     running_ = false;
@@ -199,11 +199,11 @@ class ExceptionHandlerServerRun : public UniversalMachExcServer::Interface,
     // called.
     if (notify != notify_port_) {
       LOG(WARNING) << "notify port mismatch";
-      return MIG_BAD_ID;
+      return KERN_FAILURE;
     }
 
     NOTREACHED();
-    return KERN_FAILURE;
+    return MIG_BAD_ID;
   }
 
   UniversalMachExcServer mach_exc_server_;
@@ -219,8 +219,9 @@ class ExceptionHandlerServerRun : public UniversalMachExcServer::Interface,
 
 }  // namespace
 
-ExceptionHandlerServer::ExceptionHandlerServer()
+ExceptionHandlerServer::ExceptionHandlerServer(
-    : receive_port_(NewMachPort(MACH_PORT_RIGHT_RECEIVE)) {
+    base::mac::ScopedMachReceiveRight receive_port)
+    : receive_port_(receive_port.Pass()) {
   CHECK(receive_port_.is_valid());
 }
 

handler/mac/exception_handler_server.h

@@ -28,19 +28,24 @@ namespace crashpad {
 //!     process.
 class ExceptionHandlerServer {
  public:
-  ExceptionHandlerServer();
+  //! \brief Constructs an ExceptionHandlerServer object.
+  //!
+  //! \param[in] receive_port The port that exception messages and no-senders
+  //!     notifications will be received on.
+  explicit ExceptionHandlerServer(
+      base::mac::ScopedMachReceiveRight receive_port);
   ~ExceptionHandlerServer();
 
   //! \brief Runs the exception-handling server.
   //!
   //! \param[in] exception_interface An object to send exception messages to.
   //!
-  //! This method monitors receive_port() for exception messages and no-senders
+  //! This method monitors the receive port for exception messages and
-  //! notifications. It continues running until it has no more clients,
+  //! no-senders notifications. It continues running until it has no more
-  //! indicated by the receipt of a no-senders notification. It is important to
+  //! clients, indicated by the receipt of a no-senders notification. It is
-  //! assure that a send right has been transferred to a client (or queued by
+  //! important to assure that a send right exists in a client (or has been
-  //! `mach_msg()` to be sent to a client) prior to calling this method, or it
+  //! queued by `mach_msg()` to be sent to a client) prior to calling this
-  //! will detect that it is sender-less and return immediately.
+  //! method, or it will detect that it is sender-less and return immediately.
   //!
   //! All exception messages will be passed to \a exception_interface.
   //!
@@ -48,17 +53,10 @@ class ExceptionHandlerServer {
   //!
   //! If an unexpected condition that prevents this method from functioning is
   //! encountered, it will log a message and terminate execution. Receipt of an
-  //! invalid message on receive_port() will cause a message to be logged, but
+  //! invalid message on the receive port will cause a message to be logged, but
   //! this method will continue running normally.
   void Run(UniversalMachExcServer::Interface* exception_interface);
 
-  //! \brief Returns the receive right that will be monitored for exception
-  //!     messages.
-  //!
-  //! The caller does not take ownership of this port. The caller must not use
-  //! this port for any purpose other than to make send rights for clients.
-  mach_port_t receive_port() const { return receive_port_.get(); }
-
  private:
   base::mac::ScopedMachReceiveRight receive_port_;
 

handler/main.cc

@@ -19,6 +19,7 @@
 #include <string>
 
 #include "base/files/file_path.h"
+#include "base/files/scoped_file.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "build/build_config.h"
@@ -33,6 +34,7 @@
 
 #if defined(OS_MACOSX)
 #include <libgen.h>
+#include "base/mac/scoped_mach_port.h"
 #include "handler/mac/crash_report_exception_handler.h"
 #include "handler/mac/exception_handler_server.h"
 #include "util/mach/child_port_handshake.h"
@@ -210,18 +212,23 @@ int HandlerMain(int argc, char* argv[]) {
   }
 
 #if defined(OS_MACOSX)
+  CloseStdinAndStdout();
+
   if (options.reset_own_crash_exception_port_to_system_default) {
     CrashpadClient::UseSystemDefaultHandler();
   }
-#endif
 
-  ExceptionHandlerServer exception_handler_server;
+  base::mac::ScopedMachReceiveRight receive_right(
+      ChildPortHandshake::RunServerForFD(
+          base::ScopedFD(options.handshake_fd),
+          ChildPortHandshake::PortRightType::kReceiveRight));
+  if (!receive_right.is_valid()) {
+    return EXIT_FAILURE;
+  }
 
-#if defined(OS_MACOSX)
+  ExceptionHandlerServer exception_handler_server(receive_right.Pass());
-  CloseStdinAndStdout();
+#elif defined(OS_WIN)
-  ChildPortHandshake::RunClient(options.handshake_fd,
+  ExceptionHandlerServer exception_handler_server(options.pipe_name);
-                                exception_handler_server.receive_port(),
-                                MACH_MSG_TYPE_MAKE_SEND);
 #endif  // OS_MACOSX
 
   scoped_ptr<CrashReportDatabase> database(CrashReportDatabase::Initialize(
@@ -237,11 +244,7 @@ int HandlerMain(int argc, char* argv[]) {
   CrashReportExceptionHandler exception_handler(
       database.get(), &upload_thread, &options.annotations);
 
-#if defined(OS_MACOSX)
   exception_handler_server.Run(&exception_handler);
-#elif defined(OS_WIN)
-  exception_handler_server.Run(&exception_handler, options.pipe_name);
-#endif  // OS_MACOSX
 
   upload_thread.Stop();
 

minidump/minidump_module_writer.cc

@@ -106,15 +106,7 @@ void MinidumpModuleCodeViewRecordPDB70Writer::InitializeFromSnapshot(
     const ModuleSnapshot* module_snapshot) {
   DCHECK_EQ(state(), kStateMutable);
 
-  std::string name = module_snapshot->Name();
+  SetPDBName(module_snapshot->DebugFileName());
-  std::string leaf_name;
-  size_t last_slash = name.find_last_of('/');
-  if (last_slash != std::string::npos) {
-    leaf_name = name.substr(last_slash + 1);
-  } else {
-    leaf_name = name;
-  }
-  SetPDBName(leaf_name);
 
   UUID uuid;
   uint32_t age;

minidump/minidump_module_writer_test.cc

@@ -614,6 +614,7 @@ void InitializeTestModuleSnapshotFromMinidumpModule(
     TestModuleSnapshot* module_snapshot,
     const MINIDUMP_MODULE& minidump_module,
     const std::string& name,
+    const std::string& pdb_name,
     const crashpad::UUID& uuid,
     uint32_t age) {
   module_snapshot->SetName(name);
@@ -647,12 +648,14 @@ void InitializeTestModuleSnapshotFromMinidumpModule(
   module_snapshot->SetModuleType(module_type);
 
   module_snapshot->SetUUIDAndAge(uuid, age);
+  module_snapshot->SetDebugFileName(pdb_name);
 }
 
 TEST(MinidumpModuleWriter, InitializeFromSnapshot) {
   MINIDUMP_MODULE expect_modules[3] = {};
   const char* module_paths[arraysize(expect_modules)] = {};
   const char* module_names[arraysize(expect_modules)] = {};
+  const char* module_pdbs[arraysize(expect_modules)] = {};
   UUID uuids[arraysize(expect_modules)] = {};
   uint32_t ages[arraysize(expect_modules)] = {};
 
@@ -666,6 +669,7 @@ TEST(MinidumpModuleWriter, InitializeFromSnapshot) {
   expect_modules[0].VersionInfo.dwFileType = VFT_APP;
   module_paths[0] = "/usr/bin/true";
   module_names[0] = "true";
+  module_pdbs[0] = "true";
   const uint8_t kUUIDBytes0[16] =
       {0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77,
        0x88, 0x99, 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff};
@@ -682,6 +686,7 @@ TEST(MinidumpModuleWriter, InitializeFromSnapshot) {
   expect_modules[1].VersionInfo.dwFileType = VFT_DLL;
   module_paths[1] = "/usr/lib/libSystem.B.dylib";
   module_names[1] = "libSystem.B.dylib";
+  module_pdbs[1] = "libSystem.B.dylib.pdb";
   const uint8_t kUUIDBytes1[16] =
       {0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
        0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f};
@@ -698,6 +703,7 @@ TEST(MinidumpModuleWriter, InitializeFromSnapshot) {
   expect_modules[2].VersionInfo.dwFileType = VFT_UNKNOWN;
   module_paths[2] = "/usr/lib/dyld";
   module_names[2] = "dyld";
+  module_pdbs[2] = "/usr/lib/dyld.pdb";
   const uint8_t kUUIDBytes2[16] =
       {0xff, 0xfe, 0xfd, 0xfc, 0xfb, 0xfa, 0xf9, 0xf8,
        0xf7, 0xf6, 0xf5, 0xf4, 0xf3, 0xf2, 0xf1, 0xf0};
@@ -712,6 +718,7 @@ TEST(MinidumpModuleWriter, InitializeFromSnapshot) {
     InitializeTestModuleSnapshotFromMinidumpModule(module_snapshot,
                                                    expect_modules[index],
                                                    module_paths[index],
+                                                   module_pdbs[index],
                                                    uuids[index],
                                                    ages[index]);
     module_snapshots.push_back(module_snapshot);
@@ -738,7 +745,7 @@ TEST(MinidumpModuleWriter, InitializeFromSnapshot) {
                                          &module_list->Modules[index],
                                          string_file.string(),
                                          module_paths[index],
-                                         module_names[index],
+                                         module_pdbs[index],
                                          &uuids[index],
                                          0,
                                          ages[index],

package.h

@@ -15,7 +15,7 @@
 #ifndef CRASHPAD_PACKAGE_H_
 #define CRASHPAD_PACKAGE_H_
 
-#define PACKAGE_BUGREPORT "https://code.google.com/p/crashpad/issues/entry"
+#define PACKAGE_BUGREPORT "https://crashpad.chromium.org/bug/new"
 #define PACKAGE_COPYRIGHT \
   "Copyright " PACKAGE_COPYRIGHT_YEAR " " PACKAGE_COPYRIGHT_OWNER
 #define PACKAGE_COPYRIGHT_OWNER "The Crashpad Authors"
@@ -24,6 +24,6 @@
 #define PACKAGE_STRING PACKAGE_NAME " " PACKAGE_VERSION
 #define PACKAGE_TARNAME "crashpad"
 #define PACKAGE_VERSION "0.7.0"
-#define PACKAGE_URL "https://crashpad-home.appspot.com/"
+#define PACKAGE_URL "https://crashpad.chromium.org/"
 
 #endif  // CRASHPAD_PACKAGE_H_

snapshot/mac/module_snapshot_mac.cc

@@ -17,6 +17,7 @@
 #include <mach/mach.h>
 #include <mach-o/loader.h>
 
+#include "base/files/file_path.h"
 #include "base/strings/stringprintf.h"
 #include "snapshot/mac/mach_o_image_annotations_reader.h"
 #include "snapshot/mac/mach_o_image_reader.h"
@@ -156,6 +157,11 @@ void ModuleSnapshotMac::UUIDAndAge(crashpad::UUID* uuid, uint32_t* age) const {
   *age = 0;
 }
 
+std::string ModuleSnapshotMac::DebugFileName() const {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+  return base::FilePath(Name()).BaseName().value();
+}
+
 std::vector<std::string> ModuleSnapshotMac::AnnotationsVector() const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
   MachOImageAnnotationsReader annotations_reader(

snapshot/mac/module_snapshot_mac.h

@@ -76,6 +76,7 @@ class ModuleSnapshotMac final : public ModuleSnapshot {
                      uint16_t* version_3) const override;
   ModuleType GetModuleType() const override;
   void UUIDAndAge(crashpad::UUID* uuid, uint32_t* age) const override;
+  std::string DebugFileName() const override;
   std::vector<std::string> AnnotationsVector() const override;
   std::map<std::string, std::string> AnnotationsSimpleMap() const override;
 

snapshot/minidump/module_snapshot_minidump.cc

@@ -58,25 +58,25 @@ bool ModuleSnapshotMinidump::Initialize(
 
 std::string ModuleSnapshotMinidump::Name() const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-  NOTREACHED();  // https://code.google.com/p/crashpad/issues/detail?id=10
+  NOTREACHED();  // https://crashpad.chromium.org/bug/10
   return std::string();
 }
 
 uint64_t ModuleSnapshotMinidump::Address() const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-  NOTREACHED();  // https://code.google.com/p/crashpad/issues/detail?id=10
+  NOTREACHED();  // https://crashpad.chromium.org/bug/10
   return 0;
 }
 
 uint64_t ModuleSnapshotMinidump::Size() const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-  NOTREACHED();  // https://code.google.com/p/crashpad/issues/detail?id=10
+  NOTREACHED();  // https://crashpad.chromium.org/bug/10
   return 0;
 }
 
 time_t ModuleSnapshotMinidump::Timestamp() const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-  NOTREACHED();  // https://code.google.com/p/crashpad/issues/detail?id=10
+  NOTREACHED();  // https://crashpad.chromium.org/bug/10
   return 0;
 }
 
@@ -85,7 +85,7 @@ void ModuleSnapshotMinidump::FileVersion(uint16_t* version_0,
                                          uint16_t* version_2,
                                          uint16_t* version_3) const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-  NOTREACHED();  // https://code.google.com/p/crashpad/issues/detail?id=10
+  NOTREACHED();  // https://crashpad.chromium.org/bug/10
   *version_0 = 0;
   *version_1 = 0;
   *version_2 = 0;
@@ -97,7 +97,7 @@ void ModuleSnapshotMinidump::SourceVersion(uint16_t* version_0,
                                            uint16_t* version_2,
                                            uint16_t* version_3) const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-  NOTREACHED();  // https://code.google.com/p/crashpad/issues/detail?id=10
+  NOTREACHED();  // https://crashpad.chromium.org/bug/10
   *version_0 = 0;
   *version_1 = 0;
   *version_2 = 0;
@@ -106,18 +106,24 @@ void ModuleSnapshotMinidump::SourceVersion(uint16_t* version_0,
 
 ModuleSnapshot::ModuleType ModuleSnapshotMinidump::GetModuleType() const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-  NOTREACHED();  // https://code.google.com/p/crashpad/issues/detail?id=10
+  NOTREACHED();  // https://crashpad.chromium.org/bug/10
   return kModuleTypeUnknown;
 }
 
 void ModuleSnapshotMinidump::UUIDAndAge(crashpad::UUID* uuid,
                                         uint32_t* age) const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-  NOTREACHED();  // https://code.google.com/p/crashpad/issues/detail?id=10
+  NOTREACHED();  // https://crashpad.chromium.org/bug/10
   *uuid = crashpad::UUID();
   *age = 0;
 }
 
+std::string ModuleSnapshotMinidump::DebugFileName() const {
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
+  NOTREACHED();  // https://crashpad.chromium.org/bug/10
+  return std::string();
+}
+
 std::vector<std::string> ModuleSnapshotMinidump::AnnotationsVector() const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
   return annotations_vector_;

snapshot/minidump/module_snapshot_minidump.h

@@ -73,6 +73,7 @@ class ModuleSnapshotMinidump final : public ModuleSnapshot {
                      uint16_t* version_3) const override;
   ModuleType GetModuleType() const override;
   void UUIDAndAge(crashpad::UUID* uuid, uint32_t* age) const override;
+  std::string DebugFileName() const override;
   std::vector<std::string> AnnotationsVector() const override;
   std::map<std::string, std::string> AnnotationsSimpleMap() const override;
 

snapshot/minidump/process_snapshot_minidump.cc

@@ -94,26 +94,26 @@ bool ProcessSnapshotMinidump::Initialize(FileReaderInterface* file_reader) {
 
 pid_t ProcessSnapshotMinidump::ProcessID() const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-  NOTREACHED();  // https://code.google.com/p/crashpad/issues/detail?id=10
+  NOTREACHED();  // https://crashpad.chromium.org/bug/10
   return 0;
 }
 
 pid_t ProcessSnapshotMinidump::ParentProcessID() const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-  NOTREACHED();  // https://code.google.com/p/crashpad/issues/detail?id=10
+  NOTREACHED();  // https://crashpad.chromium.org/bug/10
   return 0;
 }
 
 void ProcessSnapshotMinidump::SnapshotTime(timeval* snapshot_time) const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-  NOTREACHED();  // https://code.google.com/p/crashpad/issues/detail?id=10
+  NOTREACHED();  // https://crashpad.chromium.org/bug/10
   snapshot_time->tv_sec = 0;
   snapshot_time->tv_usec = 0;
 }
 
 void ProcessSnapshotMinidump::ProcessStartTime(timeval* start_time) const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-  NOTREACHED();  // https://code.google.com/p/crashpad/issues/detail?id=10
+  NOTREACHED();  // https://crashpad.chromium.org/bug/10
   start_time->tv_sec = 0;
   start_time->tv_usec = 0;
 }
@@ -121,7 +121,7 @@ void ProcessSnapshotMinidump::ProcessStartTime(timeval* start_time) const {
 void ProcessSnapshotMinidump::ProcessCPUTimes(timeval* user_time,
                                               timeval* system_time) const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-  NOTREACHED();  // https://code.google.com/p/crashpad/issues/detail?id=10
+  NOTREACHED();  // https://crashpad.chromium.org/bug/10
   user_time->tv_sec = 0;
   user_time->tv_usec = 0;
   system_time->tv_sec = 0;
@@ -145,20 +145,20 @@ ProcessSnapshotMinidump::AnnotationsSimpleMap() const {
   // annotations_simple_map_ to be lazily constructed: InitializeCrashpadInfo()
   // could be called here, and from other locations that require it, rather than
   // calling it from Initialize().
-  // https://code.google.com/p/crashpad/issues/detail?id=9
+  // https://crashpad.chromium.org/bug/9
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
   return annotations_simple_map_;
 }
 
 const SystemSnapshot* ProcessSnapshotMinidump::System() const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-  NOTREACHED();  // https://code.google.com/p/crashpad/issues/detail?id=10
+  NOTREACHED();  // https://crashpad.chromium.org/bug/10
   return nullptr;
 }
 
 std::vector<const ThreadSnapshot*> ProcessSnapshotMinidump::Threads() const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-  NOTREACHED();  // https://code.google.com/p/crashpad/issues/detail?id=10
+  NOTREACHED();  // https://crashpad.chromium.org/bug/10
   return std::vector<const ThreadSnapshot*>();
 }
 
@@ -173,27 +173,27 @@ std::vector<const ModuleSnapshot*> ProcessSnapshotMinidump::Modules() const {
 
 const ExceptionSnapshot* ProcessSnapshotMinidump::Exception() const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-  NOTREACHED();  // https://code.google.com/p/crashpad/issues/detail?id=10
+  NOTREACHED();  // https://crashpad.chromium.org/bug/10
   return nullptr;
 }
 
 std::vector<const MemoryMapRegionSnapshot*> ProcessSnapshotMinidump::MemoryMap()
     const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-  NOTREACHED();  // https://code.google.com/p/crashpad/issues/detail?id=10
+  NOTREACHED();  // https://crashpad.chromium.org/bug/10
   return std::vector<const MemoryMapRegionSnapshot*>();
 }
 
 std::vector<HandleSnapshot> ProcessSnapshotMinidump::Handles() const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-  NOTREACHED();  // https://code.google.com/p/crashpad/issues/detail?id=10
+  NOTREACHED();  // https://crashpad.chromium.org/bug/10
   return std::vector<HandleSnapshot>();
 }
 
 std::vector<const MemorySnapshot*> ProcessSnapshotMinidump::ExtraMemory()
     const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-  NOTREACHED();  // https://code.google.com/p/crashpad/issues/detail?id=10
+  NOTREACHED();  // https://crashpad.chromium.org/bug/10
   return std::vector<const MemorySnapshot*>();
 }
 

snapshot/module_snapshot.h

@@ -118,8 +118,20 @@ class ModuleSnapshot {
   //! \a age is the number of times the UUID has been reused. This occurs on
   //! Windows with incremental linking. On other platforms \a age will always be
   //! `0`.
+  //!
+  //! \sa DebugFileName()
   virtual void UUIDAndAge(crashpad::UUID* uuid, uint32_t* age) const = 0;
 
+  //! \brief Returns the module’s debug file info name.
+  //!
+  //! On Windows, this references the PDB file, which contains symbol
+  //! information held separately from the module itself. On other platforms,
+  //! this is normally just be the basename of the module, because the debug
+  //! info file’s name is not relevant even in split-debug scenarios.
+  //!
+  //! \sa UUIDAndAge()
+  virtual std::string DebugFileName() const = 0;
+
   //! \brief Returns string annotations recorded in the module.
   //!
   //! This method retrieves annotations recorded in a module. These annotations

snapshot/test/test_module_snapshot.cc

@@ -27,6 +27,7 @@ TestModuleSnapshot::TestModuleSnapshot()
       module_type_(kModuleTypeUnknown),
       age_(0),
       uuid_(),
+      debug_file_name_(),
       annotations_vector_(),
       annotations_simple_map_() {
 }
@@ -79,6 +80,10 @@ void TestModuleSnapshot::UUIDAndAge(crashpad::UUID* uuid, uint32_t* age) const {
   *age = age_;
 }
 
+std::string TestModuleSnapshot::DebugFileName() const {
+  return debug_file_name_;
+}
+
 std::vector<std::string> TestModuleSnapshot::AnnotationsVector() const {
   return annotations_vector_;
 }

snapshot/test/test_module_snapshot.h

@@ -64,6 +64,9 @@ class TestModuleSnapshot final : public ModuleSnapshot {
     uuid_ = uuid;
     age_ = age;
   }
+  void SetDebugFileName(const std::string& debug_file_name) {
+    debug_file_name_ = debug_file_name;
+  }
   void SetAnnotationsVector(
       const std::vector<std::string>& annotations_vector) {
     annotations_vector_ = annotations_vector;
@@ -89,6 +92,7 @@ class TestModuleSnapshot final : public ModuleSnapshot {
                      uint16_t* version_3) const override;
   ModuleType GetModuleType() const override;
   void UUIDAndAge(crashpad::UUID* uuid, uint32_t* age) const override;
+  std::string DebugFileName() const override;
   std::vector<std::string> AnnotationsVector() const override;
   std::map<std::string, std::string> AnnotationsSimpleMap() const override;
 
@@ -102,6 +106,7 @@ class TestModuleSnapshot final : public ModuleSnapshot {
   ModuleType module_type_;
   uint32_t age_;
   crashpad::UUID uuid_;
+  std::string debug_file_name_;
   std::vector<std::string> annotations_vector_;
   std::map<std::string, std::string> annotations_simple_map_;
 

snapshot/win/exception_snapshot_win.cc

@@ -153,7 +153,7 @@ bool ExceptionSnapshotWin::InitializeFromExceptionPointers(
   for (DWORD i = 0; i < first_record.NumberParameters; ++i)
     codes_.push_back(first_record.ExceptionInformation[i]);
   if (first_record.ExceptionRecord) {
-    // https://code.google.com/p/crashpad/issues/detail?id=43
+    // https://crashpad.chromium.org/bug/43
     LOG(WARNING) << "dropping chained ExceptionRecord";
   }
 

snapshot/win/exception_snapshot_win_test.cc

@@ -41,18 +41,16 @@ class RunServerThread : public Thread {
  public:
   // Instantiates a thread which will invoke server->Run(delegate, pipe_name);
   RunServerThread(ExceptionHandlerServer* server,
-                  ExceptionHandlerServer::Delegate* delegate,
+                  ExceptionHandlerServer::Delegate* delegate)
-                  const std::string& pipe_name)
+      : server_(server), delegate_(delegate) {}
-      : server_(server), delegate_(delegate), pipe_name_(pipe_name) {}
   ~RunServerThread() override {}
 
  private:
   // Thread:
-  void ThreadMain() override { server_->Run(delegate_, pipe_name_); }
+  void ThreadMain() override { server_->Run(delegate_); }
 
   ExceptionHandlerServer* server_;
   ExceptionHandlerServer::Delegate* delegate_;
-  std::string pipe_name_;
 
   DISALLOW_COPY_AND_ASSIGN(RunServerThread);
 };
@@ -130,9 +128,8 @@ void TestCrashingChild(const base::string16& directory_modification) {
   ScopedKernelHANDLE completed(CreateEvent(nullptr, false, false, nullptr));
   CrashingDelegate delegate(server_ready.get(), completed.get());
 
-  ExceptionHandlerServer exception_handler_server;
+  ExceptionHandlerServer exception_handler_server(pipe_name);
-  RunServerThread server_thread(
+  RunServerThread server_thread(&exception_handler_server, &delegate);
-      &exception_handler_server, &delegate, pipe_name);
   server_thread.Start();
   ScopedStopServerAndJoinThread scoped_stop_server_and_join_thread(
       &exception_handler_server, &server_thread);
@@ -233,9 +230,8 @@ void TestDumpWithoutCrashingChild(
   ScopedKernelHANDLE completed(CreateEvent(nullptr, false, false, nullptr));
   SimulateDelegate delegate(server_ready.get(), completed.get());
 
-  ExceptionHandlerServer exception_handler_server;
+  ExceptionHandlerServer exception_handler_server(pipe_name);
-  RunServerThread server_thread(
+  RunServerThread server_thread(&exception_handler_server, &delegate);
-      &exception_handler_server, &delegate, pipe_name);
   server_thread.Start();
   ScopedStopServerAndJoinThread scoped_stop_server_and_join_thread(
       &exception_handler_server, &server_thread);

snapshot/win/module_snapshot_win.cc

@@ -27,8 +27,12 @@ namespace internal {
 ModuleSnapshotWin::ModuleSnapshotWin()
     : ModuleSnapshot(),
       name_(),
-      timestamp_(0),
+      pdb_name_(),
+      uuid_(),
+      pe_image_reader_(),
       process_reader_(nullptr),
+      timestamp_(0),
+      age_(0),
       initialized_() {
 }
 
@@ -51,6 +55,13 @@ bool ModuleSnapshotWin::Initialize(
     return false;
   }
 
+  DWORD age_dword;
+  if (pe_image_reader_->DebugDirectoryInformation(
+          &uuid_, &age_dword, &pdb_name_)) {
+    static_assert(sizeof(DWORD) == sizeof(uint32_t), "unexpected age size");
+    age_ = age_dword;
+  }
+
   INITIALIZATION_STATE_SET_VALID(initialized_);
   return true;
 }
@@ -137,23 +148,20 @@ ModuleSnapshot::ModuleType ModuleSnapshotWin::GetModuleType() const {
 
 void ModuleSnapshotWin::UUIDAndAge(crashpad::UUID* uuid, uint32_t* age) const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-  // TODO(scottmg): Consider passing pdbname through to snapshot.
+  *uuid = uuid_;
-  std::string pdbname;
+  *age = age_;
-  DWORD age_dword;
+}
-  if (!pe_image_reader_->DebugDirectoryInformation(
+
-          uuid, &age_dword, &pdbname)) {
+std::string ModuleSnapshotWin::DebugFileName() const {
-    *uuid = crashpad::UUID();
+  INITIALIZATION_STATE_DCHECK_VALID(initialized_);
-    *age = 0;
+  return pdb_name_;
-  }
-  static_assert(sizeof(DWORD) == sizeof(uint32_t), "unexpected age size");
-  *age = age_dword;
 }
 
 std::vector<std::string> ModuleSnapshotWin::AnnotationsVector() const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
   // These correspond to system-logged things on Mac. We don't currently track
   // any of these on Windows, but could in the future.
-  // See https://code.google.com/p/crashpad/issues/detail?id=38.
+  // See https://crashpad.chromium.org/bug/38.
   return std::vector<std::string>();
 }
 

snapshot/win/module_snapshot_win.h

@@ -81,6 +81,7 @@ class ModuleSnapshotWin final : public ModuleSnapshot {
                      uint16_t* version_3) const override;
   ModuleType GetModuleType() const override;
   void UUIDAndAge(crashpad::UUID* uuid, uint32_t* age) const override;
+  std::string DebugFileName() const override;
   std::vector<std::string> AnnotationsVector() const override;
   std::map<std::string, std::string> AnnotationsSimpleMap() const override;
 
@@ -89,9 +90,12 @@ class ModuleSnapshotWin final : public ModuleSnapshot {
   void GetCrashpadOptionsInternal(CrashpadInfoClientOptions* options);
 
   std::wstring name_;
-  time_t timestamp_;
+  std::string pdb_name_;
+  UUID uuid_;
   scoped_ptr<PEImageReader> pe_image_reader_;
   ProcessReaderWin* process_reader_;  // weak
+  time_t timestamp_;
+  uint32_t age_;
   InitializationStateDcheck initialized_;
 
   DISALLOW_COPY_AND_ASSIGN(ModuleSnapshotWin);

snapshot/win/pe_image_reader.cc

@@ -191,7 +191,7 @@ bool PEImageReader::ReadDebugDirectoryInformation(UUID* uuid,
     if (*reinterpret_cast<DWORD*>(data.get()) !=
         CodeViewRecordPDB70::kSignature) {
       // TODO(scottmg): Consider supporting other record types, see
-      // https://code.google.com/p/crashpad/issues/detail?id=47.
+      // https://crashpad.chromium.org/bug/47.
       LOG(WARNING) << "encountered non-7.0 CodeView debug record";
       continue;
     }

snapshot/win/process_snapshot_win.cc

@@ -353,7 +353,7 @@ void ProcessSnapshotWin::AddMemorySnapshot(
   // useful for the LDR module lists which are a set of doubly-linked lists, all
   // pointing to the same module name strings.
   // TODO(scottmg): A more general version of this, handling overlapping,
-  // contained, etc. https://code.google.com/p/crashpad/issues/detail?id=61.
+  // contained, etc. https://crashpad.chromium.org/bug/61.
   for (const auto& memory_snapshot : *into) {
     if (memory_snapshot->Address() == address &&
         memory_snapshot->Size() == size) {

snapshot/win/thread_snapshot_win.cc

@@ -112,8 +112,7 @@ uint64_t ThreadSnapshotWin::ThreadSpecificDataAddress() const {
 std::vector<const MemorySnapshot*> ThreadSnapshotWin::ExtraMemory() const {
   INITIALIZATION_STATE_DCHECK_VALID(initialized_);
   // TODO(scottmg): Ensure this region is readable, and make sure we don't
-  // discard the entire dump if it isn't.
+  // discard the entire dump if it isn't. https://crashpad.chromium.org/bug/59
-  // https://code.google.com/p/crashpad/issues/detail?id=59
   return std::vector<const MemorySnapshot*>(1, &teb_);
 }
 

test/mac/mach_multiprocess.cc

@@ -92,7 +92,7 @@ void MachMultiprocess::PreFork() {
   // Set up the parent port and register it with the bootstrap server before
   // forking, so that it’s guaranteed to be there when the child attempts to
   // look it up.
-  info_->service_name = "com.googlecode.crashpad.test.mach_multiprocess.";
+  info_->service_name = "org.chromium.crashpad.test.mach_multiprocess.";
   for (int index = 0; index < 16; ++index) {
     info_->service_name.append(1, base::RandInt('A', 'Z'));
   }

test/scoped_temp_dir_posix.cc

@@ -33,7 +33,7 @@ void ScopedTempDir::Rename() {
 
 // static
 base::FilePath ScopedTempDir::CreateTemporaryDirectory() {
-  char dir_template[] = "/tmp/com.googlecode.crashpad.test.XXXXXX";
+  char dir_template[] = "/tmp/org.chromium.crashpad.test.XXXXXX";
   PCHECK(mkdtemp(dir_template)) << "mkdtemp " << dir_template;
   return base::FilePath(dir_template);
 }

third_party/gmock/README.crashpad

@@ -1,6 +1,6 @@
 Name: Google C++ Mocking Framework (googlemock)
 Short Name: gmock
-URL: https://googlemock.googlecode.com/
+URL: https://github.com/google/googletest/tree/master/googlemock/
 Revision: See DEPS
 License: BSD 3-clause
 License File: gmock/LICENSE

third_party/gtest/README.crashpad

@@ -1,6 +1,6 @@
 Name: Google C++ Testing Framework (googletest)
 Short Name: gtest
-URL: https://googletest.googlecode.com/
+URL: https://github.com/google/googletest/
 Revision: See DEPS
 License: BSD 3-clause
 License File: gtest/LICENSE

third_party/gyp/README.crashpad

@@ -1,6 +1,6 @@
 Name: GYP (Generate Your Projects)
 Short Name: gyp
-URL: https://gyp.googlecode.com/
+URL: https://gyp.gsrc.io/
 Revision: See DEPS
 License: BSD 3-clause
 License File: gyp/LICENSE

util/mac/service_management_test.mm

@@ -117,9 +117,9 @@ TEST(ServiceManagement, SubmitRemoveJob) {
         base::StringPrintf("sleep 10; echo %s", cookie.c_str());
     NSString* shell_script_ns = base::SysUTF8ToNSString(shell_script);
 
-    const char kJobLabel[] = "com.googlecode.crashpad.test.service_management";
+    const char kJobLabel[] = "org.chromium.crashpad.test.service_management";
     NSDictionary* job_dictionary_ns = @{
-      @LAUNCH_JOBKEY_LABEL : @"com.googlecode.crashpad.test.service_management",
+      @LAUNCH_JOBKEY_LABEL : @"org.chromium.crashpad.test.service_management",
       @LAUNCH_JOBKEY_RUNATLOAD : @YES,
       @LAUNCH_JOBKEY_PROGRAMARGUMENTS :
           @[ @"/bin/sh", @"-c", shell_script_ns, ],

util/mach/child_port.defs

@@ -16,10 +16,10 @@
 #include <mach/std_types.defs>
 
 // child_port provides an interface for port rights to be transferred between
-// tasks. Its expected usage is for child processes to be able to pass port
+// tasks. Its expected usage is for processes to be able to pass port rights
-// rights to their parent processes. A child may wish to give its parent a copy
+// across IPC boundaries. A child process may wish to give its parent a copy of
-// of a send right to its own task port, or a child may hold a receive right for
+// of a send right to its own task port, or a parent process may wish to give a
-// a server and wish to furnish its parent with a send right to that server.
+// receive right to a child process that implements a server.
 //
 // This Mach subsystem defines the lowest-level interface for these rights to
 // be transferred. Most users will not user this interface directly, but will
@@ -48,9 +48,7 @@ import "util/mach/child_port_types.h";
 //     secret allowing the server to verify that it has received a request from
 //     the intended client. |server| will reject requests with an invalid
 //     |token|.
-// port[in]: A port right to transfer to the server. In expected usage, this may
+// port[in]: A port right to transfer to the server.
-//     be a send or send-once right, and the |server| will reject a receive
-//     right. It is permissible to specify make-send for a receive right.
 //
 // Return value: As this is a “simpleroutine”, the server does not respond to
 //     the client request, and the client does not block waiting for a response

util/mach/child_port_handshake.cc

@@ -32,60 +32,60 @@
 #include "base/strings/stringprintf.h"
 #include "util/file/file_io.h"
 #include "util/mach/child_port.h"
+#include "util/mach/child_port_server.h"
 #include "util/mach/mach_extensions.h"
 #include "util/mach/mach_message.h"
 #include "util/mach/mach_message_server.h"
 #include "util/misc/implicit_cast.h"
 
 namespace crashpad {
+namespace {
 
-ChildPortHandshake::ChildPortHandshake()
+class ChildPortHandshakeServer final : public ChildPortServer::Interface {
+ public:
+  ChildPortHandshakeServer();
+  ~ChildPortHandshakeServer();
+
+  mach_port_t RunServer(base::ScopedFD server_write_fd,
+                        ChildPortHandshake::PortRightType port_right_type);
+
+ private:
+  // ChildPortServer::Interface:
+  kern_return_t HandleChildPortCheckIn(child_port_server_t server,
+                                       child_port_token_t token,
+                                       mach_port_t port,
+                                       mach_msg_type_name_t right_type,
+                                       const mach_msg_trailer_t* trailer,
+                                       bool* destroy_request) override;
+
+  child_port_token_t token_;
+  mach_port_t port_;
+  mach_msg_type_name_t right_type_;
+  bool checked_in_;
+
+  DISALLOW_COPY_AND_ASSIGN(ChildPortHandshakeServer);
+};
+
+ChildPortHandshakeServer::ChildPortHandshakeServer()
     : token_(0),
-      pipe_read_(),
+      port_(MACH_PORT_NULL),
-      pipe_write_(),
+      right_type_(MACH_MSG_TYPE_PORT_NONE),
-      child_port_(MACH_PORT_NULL),
       checked_in_(false) {
-  // Use socketpair() instead of pipe(). There is no way to suppress SIGPIPE on
-  // pipes in Mac OS X 10.6, because the F_SETNOSIGPIPE fcntl() command was not
-  // introduced until 10.7.
-  int pipe_fds[2];
-  PCHECK(socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, pipe_fds) == 0)
-      << "socketpair";
-
-  pipe_read_.reset(pipe_fds[0]);
-  pipe_write_.reset(pipe_fds[1]);
-
-  // Simulate pipe() semantics by shutting down the “wrong” sides of the socket.
-  PCHECK(shutdown(pipe_write_.get(), SHUT_RD) == 0) << "shutdown";
-  PCHECK(shutdown(pipe_read_.get(), SHUT_WR) == 0) << "shutdown";
-
-  // SIGPIPE is undesirable when writing to this pipe. Allow broken-pipe writes
-  // to fail with EPIPE instead.
-  const int value = 1;
-  PCHECK(setsockopt(
-      pipe_write_.get(), SOL_SOCKET, SO_NOSIGPIPE, &value, sizeof(value)) == 0)
-      << "setsockopt";
 }
 
-ChildPortHandshake::~ChildPortHandshake() {
+ChildPortHandshakeServer::~ChildPortHandshakeServer() {
 }
 
-int ChildPortHandshake::ReadPipeFD() const {
+mach_port_t ChildPortHandshakeServer::RunServer(
-  DCHECK_NE(pipe_read_.get(), -1);
+    base::ScopedFD server_write_fd,
-  return pipe_read_.get();
+    ChildPortHandshake::PortRightType port_right_type) {
-}
+  DCHECK_EQ(port_, kMachPortNull);
-
+  DCHECK(!checked_in_);
-mach_port_t ChildPortHandshake::RunServer() {
+  DCHECK(server_write_fd.is_valid());
-  DCHECK_NE(pipe_read_.get(), -1);
-  pipe_read_.reset();
-
-  // Transfer ownership of the write pipe into this method’s scope.
-  base::ScopedFD pipe_write_owner = pipe_write_.Pass();
 
   // Initialize the token and share it with the client via the pipe.
   token_ = base::RandUint64();
-  int pipe_write = pipe_write_owner.get();
+  if (!LoggingWriteFile(server_write_fd.get(), &token_, sizeof(token_))) {
-  if (!LoggingWriteFile(pipe_write, &token_, sizeof(token_))) {
     LOG(WARNING) << "no client check-in";
     return MACH_PORT_NULL;
   }
@@ -97,7 +97,7 @@ mach_port_t ChildPortHandshake::RunServer() {
   errno = pthread_threadid_np(pthread_self(), &thread_id);
   PCHECK(errno == 0) << "pthread_threadid_np";
   std::string service_name = base::StringPrintf(
-      "com.googlecode.crashpad.child_port_handshake.%d.%llu.%016llx",
+      "org.chromium.crashpad.child_port_handshake.%d.%llu.%016llx",
       getpid(),
       thread_id,
       base::RandUint64());
@@ -109,14 +109,15 @@ mach_port_t ChildPortHandshake::RunServer() {
 
   // Share the service name with the client via the pipe.
   uint32_t service_name_length = service_name.size();
-  if (!LoggingWriteFile(
+  if (!LoggingWriteFile(server_write_fd.get(),
-          pipe_write, &service_name_length, sizeof(service_name_length))) {
+                        &service_name_length,
+                        sizeof(service_name_length))) {
     LOG(WARNING) << "no client check-in";
     return MACH_PORT_NULL;
   }
 
   if (!LoggingWriteFile(
-          pipe_write, service_name.c_str(), service_name_length)) {
+          server_write_fd.get(), service_name.c_str(), service_name_length)) {
     LOG(WARNING) << "no client check-in";
     return MACH_PORT_NULL;
   }
@@ -147,7 +148,7 @@ mach_port_t ChildPortHandshake::RunServer() {
          0,
          nullptr);
   EV_SET(&changelist[1],
-         pipe_write,
+         server_write_fd.get(),
          EVFILT_WRITE,
          EV_ADD | EV_CLEAR,
          0,
@@ -162,7 +163,7 @@ mach_port_t ChildPortHandshake::RunServer() {
   bool blocking = true;
   DCHECK(!checked_in_);
   while (!checked_in_) {
-    DCHECK_EQ(child_port_, kMachPortNull);
+    DCHECK_EQ(port_, kMachPortNull);
 
     // Get a kevent from the kqueue. Block while waiting for an event unless the
     // write pipe has arrived at EOF, in which case the kevent() should be
@@ -230,7 +231,7 @@ mach_port_t ChildPortHandshake::RunServer() {
         // pipe. Ignore that case. Multiple notifications for that situation
         // will not be generated because edge triggering (EV_CLEAR) is used
         // above.
-        DCHECK_EQ(implicit_cast<int>(event.ident), pipe_write);
+        DCHECK_EQ(implicit_cast<int>(event.ident), server_write_fd.get());
         if (event.flags & EV_EOF) {
           // There are no readers attached to the write pipe. The client has
           // closed its side of the pipe. There can be one last shot at
@@ -246,19 +247,48 @@ mach_port_t ChildPortHandshake::RunServer() {
     }
   }
 
-  mach_port_t child_port = MACH_PORT_NULL;
+  if (port_ == MACH_PORT_NULL) {
-  std::swap(child_port_, child_port);
+    return MACH_PORT_NULL;
-  return child_port;
+  }
+
+  bool mismatch = false;
+  switch (port_right_type) {
+    case ChildPortHandshake::PortRightType::kReceiveRight:
+      if (right_type_ != MACH_MSG_TYPE_PORT_RECEIVE) {
+        LOG(ERROR) << "expected receive right, observed " << right_type_;
+        mismatch = true;
+      }
+      break;
+    case ChildPortHandshake::PortRightType::kSendRight:
+      if (right_type_ != MACH_MSG_TYPE_PORT_SEND &&
+          right_type_ != MACH_MSG_TYPE_PORT_SEND_ONCE) {
+        LOG(ERROR) << "expected send or send-once right, observed "
+                   << right_type_;
+        mismatch = true;
+      }
+      break;
+  }
+
+  if (mismatch) {
+    MachMessageDestroyReceivedPort(port_, right_type_);
+    port_ = MACH_PORT_NULL;
+    return MACH_PORT_NULL;
+  }
+
+  mach_port_t port = MACH_PORT_NULL;
+  std::swap(port_, port);
+  return port;
 }
 
-kern_return_t ChildPortHandshake::HandleChildPortCheckIn(
+kern_return_t ChildPortHandshakeServer::HandleChildPortCheckIn(
     child_port_server_t server,
     const child_port_token_t token,
     mach_port_t port,
     mach_msg_type_name_t right_type,
     const mach_msg_trailer_t* trailer,
     bool* destroy_request) {
-  DCHECK_EQ(child_port_, kMachPortNull);
+  DCHECK_EQ(port_, kMachPortNull);
+  DCHECK(!checked_in_);
 
   if (token != token_) {
     // If the token’s not correct, someone’s attempting to spoof the legitimate
@@ -268,19 +298,18 @@ kern_return_t ChildPortHandshake::HandleChildPortCheckIn(
   } else {
     checked_in_ = true;
 
-    if (right_type == MACH_MSG_TYPE_PORT_RECEIVE) {
+    if (right_type != MACH_MSG_TYPE_PORT_RECEIVE &&
-      // The message needs to carry a send right or a send-once right. This
+        right_type != MACH_MSG_TYPE_PORT_SEND &&
-      // isn’t a strict requirement of the protocol, but users of this class
+        right_type != MACH_MSG_TYPE_PORT_SEND_ONCE) {
-      // expect a send right or a send-once right, both of which can be managed
+      // The message needs to carry a receive, send, or send-once right.
-      // by base::mac::ScopedMachSendRight. It is invalid to store a receive
+      LOG(ERROR) << "invalid right type " << right_type;
-      // right in that scoper.
-      LOG(WARNING) << "ignoring MACH_MSG_TYPE_PORT_RECEIVE";
       *destroy_request = true;
     } else {
-      // Communicate the child port back to the RunServer().
+      // Communicate the child port and right type back to the RunServer().
       // *destroy_request is left at false, because RunServer() needs the right
       // to remain intact. It gives ownership of the right to its caller.
-      child_port_ = port;
+      port_ = port;
+      right_type_ = right_type;
     }
   }
 
@@ -288,40 +317,112 @@ kern_return_t ChildPortHandshake::HandleChildPortCheckIn(
   return MIG_NO_REPLY;
 }
 
-// static
+}  // namespace
-void ChildPortHandshake::RunClient(int pipe_read,
+
-                                   mach_port_t port,
+ChildPortHandshake::ChildPortHandshake()
+    : client_read_fd_(),
+      server_write_fd_() {
+  // Use socketpair() instead of pipe(). There is no way to suppress SIGPIPE on
+  // pipes in Mac OS X 10.6, because the F_SETNOSIGPIPE fcntl() command was not
+  // introduced until 10.7.
+  int pipe_fds[2];
+  PCHECK(socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, pipe_fds) == 0)
+      << "socketpair";
+
+  client_read_fd_.reset(pipe_fds[0]);
+  server_write_fd_.reset(pipe_fds[1]);
+
+  // Simulate pipe() semantics by shutting down the “wrong” sides of the socket.
+  PCHECK(shutdown(server_write_fd_.get(), SHUT_RD) == 0) << "shutdown SHUT_RD";
+  PCHECK(shutdown(client_read_fd_.get(), SHUT_WR) == 0) << "shutdown SHUT_WR";
+
+  // SIGPIPE is undesirable when writing to this pipe. Allow broken-pipe writes
+  // to fail with EPIPE instead.
+  const int value = 1;
+  PCHECK(setsockopt(server_write_fd_.get(),
+                    SOL_SOCKET,
+                    SO_NOSIGPIPE,
+                    &value,
+                    sizeof(value)) == 0) << "setsockopt";
+}
+
+ChildPortHandshake::~ChildPortHandshake() {
+}
+
+base::ScopedFD ChildPortHandshake::ClientReadFD() {
+  DCHECK(client_read_fd_.is_valid());
+  return client_read_fd_.Pass();
+}
+
+base::ScopedFD ChildPortHandshake::ServerWriteFD() {
+  DCHECK(server_write_fd_.is_valid());
+  return server_write_fd_.Pass();
+}
+
+mach_port_t ChildPortHandshake::RunServer(PortRightType port_right_type) {
+  client_read_fd_.reset();
+  return RunServerForFD(server_write_fd_.Pass(), port_right_type);
+}
+
+bool ChildPortHandshake::RunClient(mach_port_t port,
                                    mach_msg_type_name_t right_type) {
-  base::ScopedFD pipe_read_owner(pipe_read);
+  server_write_fd_.reset();
+  return RunClientForFD(client_read_fd_.Pass(), port, right_type);
+}
+
+// static
+mach_port_t ChildPortHandshake::RunServerForFD(base::ScopedFD server_write_fd,
+                                               PortRightType port_right_type) {
+  ChildPortHandshakeServer server;
+  return server.RunServer(server_write_fd.Pass(), port_right_type);
+}
+
+// static
+bool ChildPortHandshake::RunClientForFD(base::ScopedFD client_read_fd,
+                                        mach_port_t port,
+                                        mach_msg_type_name_t right_type) {
+  DCHECK(client_read_fd.is_valid());
 
   // Read the token and the service name from the read side of the pipe.
   child_port_token_t token;
   std::string service_name;
-  RunClientInternal_ReadPipe(pipe_read, &token, &service_name);
+  if (!RunClientInternal_ReadPipe(
+          client_read_fd.get(), &token, &service_name)) {
+    return false;
+  }
 
   // Look up the server and check in with it by providing the token and port.
-  RunClientInternal_SendCheckIn(service_name, token, port, right_type);
+  return RunClientInternal_SendCheckIn(service_name, token, port, right_type);
 }
 
 // static
-void ChildPortHandshake::RunClientInternal_ReadPipe(int pipe_read,
+bool ChildPortHandshake::RunClientInternal_ReadPipe(int client_read_fd,
                                                     child_port_token_t* token,
                                                     std::string* service_name) {
   // Read the token from the pipe.
-  CheckedReadFile(pipe_read, token, sizeof(*token));
+  if (!LoggingReadFile(client_read_fd, token, sizeof(*token))) {
+    return false;
+  }
 
   // Read the service name from the pipe.
   uint32_t service_name_length;
-  CheckedReadFile(pipe_read, &service_name_length, sizeof(service_name_length));
+  if (!LoggingReadFile(
+      client_read_fd, &service_name_length, sizeof(service_name_length))) {
+    return false;
+  }
 
   service_name->resize(service_name_length);
-  if (!service_name->empty()) {
+  if (!service_name->empty() &&
-    CheckedReadFile(pipe_read, &(*service_name)[0], service_name_length);
+      !LoggingReadFile(
+          client_read_fd, &(*service_name)[0], service_name_length)) {
+    return false;
   }
+
+  return true;
 }
 
 // static
-void ChildPortHandshake::RunClientInternal_SendCheckIn(
+bool ChildPortHandshake::RunClientInternal_SendCheckIn(
     const std::string& service_name,
     child_port_token_t token,
     mach_port_t port,
@@ -329,12 +430,19 @@ void ChildPortHandshake::RunClientInternal_SendCheckIn(
   // Get a send right to the server by looking up the service with the bootstrap
   // server by name.
   base::mac::ScopedMachSendRight server_port(BootstrapLookUp(service_name));
-  CHECK(server_port.is_valid());
+  if (server_port == kMachPortNull) {
+    return false;
+  }
 
   // Check in with the server.
-  kern_return_t kr =
+  kern_return_t kr = child_port_check_in(
-      child_port_check_in(server_port.get(), token, port, right_type);
+      server_port.get(), token, port, right_type);
-  MACH_CHECK(kr == KERN_SUCCESS, kr) << "child_port_check_in";
+  if (kr != KERN_SUCCESS) {
+    MACH_LOG(ERROR, kr) << "child_port_check_in";
+    return false;
+  }
+
+  return true;
 }
 
 }  // namespace crashpad

util/mach/child_port_handshake.h

@@ -21,7 +21,7 @@
 
 #include "base/basictypes.h"
 #include "base/files/scoped_file.h"
-#include "util/mach/child_port_server.h"
+#include "util/mach/child_port_types.h"
 
 namespace crashpad {
 
@@ -31,80 +31,177 @@ class ChildPortHandshakeTest;
 }  // namespace
 }  // namespace test
 
-//! \brief Implements a handshake protocol that allows a parent process to
+//! \brief Implements a handshake protocol that allows processes to exchange
-//!     obtain a Mach port right from a child process.
+//!     port rights.
 //!
 //! Ordinarily, there is no way for parent and child processes to exchange port
 //! rights, outside of the rights that children inherit from their parents.
 //! These include task-special ports and exception ports, but all of these have
 //! system-defined uses, and cannot reliably be replaced: in a multi-threaded
-//! parent, it is impossible to temporarily change one an inheritable port while
+//! parent, it is impossible to temporarily change an inheritable port while
 //! maintaining a guarantee that another thread will not attempt to use it, and
 //! in children, it difficult to guarantee that nothing will attempt to use an
 //! inheritable port before it can be replaced with the correct one. This latter
 //! concern is becoming increasingly more pronounced as system libraries perform
-//! more operations that rely on an inheritable port in module initializers.
+//! more operations that rely on an inherited port in module initializers.
 //!
-//! The protocol implemented by this class involves a server that runs in the
+//! The protocol implemented by this class involves a server that runs in one
-//! parent process. The server is published with the bootstrap server, which the
+//! process. The server is published with the bootstrap server, which the other
-//! child has access to because the bootstrap port is one of the inherited
+//! process has access to because the bootstrap port is one of the inherited
-//! task-special ports. The parent and child also share a pipe, which the parent
+//! task-special ports. The two processes also share a pipe, which the server
-//! can write to and the child can read from. After launching a child process,
+//! can write to and the client can read from. The server will write a random
-//! the parent will write a random token to this pipe, along with the name under
+//! token to this pipe, along with the name under which its service has been
-//! which its server has been registered with the bootstrap server. The child
+//! registered with the bootstrap server. The client can then obtain a send
-//! can then obtain a send right to this server with `bootstrap_look_up()`, and
+//! right to this service with `bootstrap_look_up()`, and send a check-in
-//! send a check-in message containing the token value and the port right of its
+//! message containing the token value and the port right of its choice by
-//! choice by calling `child_port_check_in()`.
+//! calling `child_port_check_in()`.
 //!
-//! The inclusion of the token authenticates the child to its parent. This is
+//! The inclusion of the token authenticates the client to the server. This is
 //! necessary because the service is published with the bootstrap server, which
-//! opens up access to it to more than the child process. Because the token is
+//! opens up access to it to more than the intended client. Because the token is
-//! passed to the child by a shared pipe, it constitutes a shared secret not
+//! passed to the client by a shared pipe, it constitutes a shared secret not
 //! known by other processes that may have incidental access to the server. The
 //! ChildPortHandshake server considers its randomly-generated token valid until
 //! a client checks in with it. This mechanism is used instead of examining the
 //! request message’s audit trailer to verify the sender’s process ID because in
-//! some process architectures, it may be impossible to verify the child’s
+//! some process architectures, it may be impossible to verify the client’s
-//! process ID. This may happen when the child disassociates from the parent
+//! process ID.
-//! with a double fork(), and the actual client is the parent’s grandchild. In
-//! this case, the child would not check in, but the grandchild, in possession
-//! of the token, would check in.
 //!
 //! The shared pipe serves another purpose: the server monitors it for an
 //! end-of-file (no readers) condition. Once detected, it will stop its blocking
-//! wait for a client to check in. This mechanism was chosen over monitoring a
+//! wait for a client to check in. This mechanism was also chosen for its
-//! child process directly for exit to account for the possibility that the
+//! ability to function properly in diverse process architectures.
-//! child might disassociate with a double fork().
 //!
-//! This class can be used to allow a child process to provide its parent with
+//! This class can be used to allow a child process to provide its parent with a
-//! a send right to its task port, in cases where it is desirable for the parent
+//! send right to its task port, in cases where it is desirable for the parent
-//! to have such access. It can also be used to allow a child process to
+//! to have such access. It can also be used to allow a parent process to
-//! establish its own server and provide its parent with a send right to that
+//! transfer a receive right to a child process that implements the server for
-//! server, for cases where a service is provided and it is undesirable or
+//! that right, or for a child process to establish its own server and provide
-//! impossible to provide it via the bootstrap or launchd interfaces.
+//! its parent with a send right to that server, for cases where a service is
-class ChildPortHandshake : public ChildPortServer::Interface {
+//! provided and it is undesirable or impossible to provide it via the bootstrap
+//! or launchd interfaces.
+//!
+//! Example parent process, running a client that sends a receive right to its
+//! child:
+//! \code
+//!   ChildPortHandshake child_port_handshake;
+//!   base::ScopedFD server_write_fd = child_port_handshake.ServerWriteFD();
+//!   std::string server_write_fd_string =
+//!       base::StringPrintf("%d", server_write_fd.get());
+//!
+//!   pid_t pid = fork();
+//!   if (pid == 0) {
+//!     // Child
+//!
+//!     // Close all file descriptors above STDERR_FILENO except for
+//!     // server_write_fd. Let the child know what file descriptor to use for
+//!     // server_write_fd by passing it as argv[1]. Example code for the child
+//!     // process is below.
+//!     CloseMultipleNowOrOnExec(STDERR_FILENO + 1, server_write_fd.get());
+//!     execlp("./child", "child", server_write_fd_string.c_str(), nullptr);
+//!   }
+//!
+//!   // Parent
+//!
+//!   // Close the child’s end of the pipe.
+//!   server_write_fd.reset();
+//!
+//!   // Make a new Mach receive right.
+//!   base::mac::ScopedMachReceiveRight
+//!       receive_right(NewMachPort(MACH_PORT_RIGHT_RECEIVE));
+//!
+//!   // Make a send right corresponding to the receive right.
+//!   mach_port_t send_right;
+//!   mach_msg_type_name_t send_right_type;
+//!   mach_port_extract_right(mach_task_self(),
+//!                           receive_right.get(),
+//!                           MACH_MSG_TYPE_MAKE_SEND,
+//!                           &send_right,
+//!                           &send_right_type);
+//!   base::mac::ScopedMachSendRight send_right_owner(send_right);
+//!
+//!   // Send the receive right to the child process, retaining the send right
+//!   // for use in the parent process.
+//!   if (child_port_handshake.RunClient(receive_right.get(),
+//!                                      MACH_MSG_TYPE_MOVE_RECEIVE)) {
+//!     ignore_result(receive_right.release());
+//!   }
+//! \endcode
+//!
+//! Example child process, running a server that receives a receive right from
+//! its parent:
+//! \code
+//!   int main(int argc, char* argv[]) {
+//!     // The parent passed server_write_fd in argv[1].
+//!     base::ScopedFD server_write_fd(atoi(argv[1]));
+//!
+//!     // Obtain a receive right from the parent process.
+//!     base::mac::ScopedMachReceiveRight receive_right(
+//!         ChildPortHandshake::RunServerForFD(
+//!             server_write_fd.Pass(),
+//!             ChildPortHandshake::PortRightType::kReceiveRight));
+//!   }
+//! \endcode
+class ChildPortHandshake {
  public:
-  //! \brief Initializes the server.
+  //! \brief Controls whether a receive or send right is expected to be
-  //!
+  //!     obtained from the client by the server’s call to RunServer().
-  //! This creates the pipe so that the “read” side can be obtained by calling
+  enum class PortRightType {
-  //! ReadPipeFD().
+    //! \brief The server expects to receive a receive right.
-  ChildPortHandshake();
+    kReceiveRight = 0,
 
+    //! \brief The server expects to receive a send or send-once right.
+    kSendRight,
+  };
+
+  ChildPortHandshake();
   ~ChildPortHandshake();
 
   //! \brief Obtains the “read” side of the pipe, to be used by the client.
   //!
-  //! Callers must obtain this file descriptor and arrange for the caller to
+  //! This file descriptor must be passed to RunClientForFD().
-  //! have access to it before calling RunServer().
   //!
   //! \return The file descriptor that the client should read from.
-  int ReadPipeFD() const;
+  base::ScopedFD ClientReadFD();
+
+  //! \brief Obtains the “write” side of the pipe, to be used by the server.
+  //!
+  //! This file descriptor must be passed to RunServerForFD().
+  //!
+  //! \return The file descriptor that the server should write to.
+  base::ScopedFD ServerWriteFD();
 
   //! \brief Runs the server.
   //!
-  //! This method performs these tasks:
+  //! This method closes the “read” side of the pipe in-process, so that the
-  //!  - Closes the “read” side of the pipe in-process, so that the client
+  //! client process holds the only file descriptor that can read from the pipe.
-  //!    process holds the only file descriptor that can read from the pipe.
+  //! It then calls RunServerForFD() using the “write” side of the pipe. If
+  //! ClientReadFD() has already been called in the server process, the caller
+  //! must ensure that the file descriptor returned by ClientReadFD() is closed
+  //! prior to calling this method.
+  mach_port_t RunServer(PortRightType port_right_type);
+
+  //! \brief Runs the client.
+  //!
+  //! This method closes the “write” side of the pipe in-process, so that the
+  //! server process holds the only file descriptor that can write to the pipe.
+  //! It then calls RunClientForFD() using the “read” side of the pipe. If
+  //! ServerWriteFD() has already been called in the client process, the caller
+  //! must ensure that the file descriptor returned by ServerWriteFD() is closed
+  //! prior to calling this method.
+  //!
+  //! \return `true` on success, `false` on failure with a message logged.
+  bool RunClient(mach_port_t port, mach_msg_type_name_t right_type);
+
+  //! \brief Runs the server.
+  //!
+  //! If a ChildPortHandshake object is available, don’t call this static
+  //! function. Instead, call RunServer(), which wraps this function. When using
+  //! this function, the caller is responsible for ensuring that the client
+  //! “read” side of the pipe is closed in the server process prior to calling
+  //! this function.
+  //!
+  //! This function performs these tasks:
   //!  - Creates a random token and sends it via the pipe.
   //!  - Checks its service in with the bootstrap server, and sends the name
   //!    of its bootstrap service mapping via the pipe.
@@ -114,33 +211,35 @@ class ChildPortHandshake : public ChildPortServer::Interface {
   //!    interpret and validate it, and if the message is valid, returns the
   //!    port right extracted from the message. If the message is not valid,
   //!    this method will continue waiting for a valid message. Valid messages
-  //!    are properly formatted and have the correct token. If a valid message
+  //!    are properly formatted and have the correct token. The right carried in
-  //!    carries a send or send-once right, it will be returned. If a valid
+  //!    a valid message will be returned. If a message is not valid, this
-  //!    message contains a receive right, it will be destroyed and
-  //!    `MACH_PORT_NULL` will be returned. If a message is not valid, this
   //!    method will continue waiting for pipe EOF or a valid message.
   //!  - When notified of pipe EOF, returns `MACH_PORT_NULL`.
   //!  - Regardless of return value, destroys the server’s receive right and
   //!    closes the pipe.
   //!
-  //! \return On success, the send or send-once right to the port provided by
+  //! \param[in] port_right_type The port right type expected to be received
-  //!     the client. The caller takes ownership of this right. On failure,
+  //!     from the client. If the port right received from the client does not
-  //!     `MACH_PORT_NULL`, indicating that the client did not check in properly
+  //!     match the expected type, the received port right will be destroyed,
-  //!     before terminating, where termination is detected by noticing that the
+  //!     and `MACH_PORT_NULL` will be returned.
-  //!     read side of the shared pipe has closed. On failure, a message
+  //!
-  //!     indiciating the nature of the failure will be logged.
+  //! \return On success, the port right provided by the client. The caller
-  mach_port_t RunServer();
+  //!     takes ownership of this right. On failure, `MACH_PORT_NULL`,
-
+  //!     indicating that the client did not check in properly before
-  // ChildPortServer::Interface:
+  //!     terminating, where termination is detected by detecting that the read
-  kern_return_t HandleChildPortCheckIn(child_port_server_t server,
+  //!     side of the shared pipe has closed. On failure, a message indicating
-                                       child_port_token_t token,
+  //!     the nature of the failure will be logged.
-                                       mach_port_t port,
+  static mach_port_t RunServerForFD(base::ScopedFD server_write_fd,
-                                       mach_msg_type_name_t right_type,
+                                    PortRightType port_right_type);
-                                       const mach_msg_trailer_t* trailer,
-                                       bool* destroy_request) override;
 
   //! \brief Runs the client.
   //!
+  //! If a ChildPortHandshake object is available, don’t call this static
+  //! function. Instead, call RunClient(), which wraps this function. When using
+  //! this function, the caller is responsible for ensuring that the server
+  //! “write” side of the pipe is closed in the client process prior to calling
+  //! this function.
+  //!
   //! This function performs these tasks:
   //!  - Reads the token from the pipe.
   //!  - Reads the bootstrap service name from the pipe.
@@ -155,32 +254,42 @@ class ChildPortHandshake : public ChildPortServer::Interface {
   //! check-in to occur without blocking to wait for a reply.
   //!
   //! \param[in] pipe_read The “read” side of the pipe shared with the server
-  //!     process.
+  //!     process. This function takes ownership of this file descriptor, and
-  //! \param[in] port The port that will be passed to the server by
+  //!     will close it prior to returning.
+  //! \param[in] port The port right that will be passed to the server by
   //!     `child_port_check_in()`.
-  //! \param[in] right_type The right type to furnish the parent with. If \a
+  //! \param[in] right_type The right type to furnish the server with. If \a
   //!     port is a send right, this can be `MACH_MSG_TYPE_COPY_SEND` or
   //!     `MACH_MSG_TYPE_MOVE_SEND`. If \a port is a send-once right, this can
-  //!      be `MACH_MSG_TYPE_MOVE_SEND_ONCE`. If \a port is a receive right,
+  //!     be `MACH_MSG_TYPE_MOVE_SEND_ONCE`. If \a port is a receive right, this
-  //!      this can be `MACH_MSG_TYPE_MAKE_SEND`. `MACH_MSG_TYPE_MOVE_RECEIVE`
+  //!     can be `MACH_MSG_TYPE_MAKE_SEND`, `MACH_MSG_TYPE_MAKE_SEND_ONCE`, or
-  //!      is supported by the client interface but will be silently rejected by
+  //!     `MACH_MSG_TYPE_MOVE_RECEIVE`.
-  //!      server run by RunServer(), which expects to receive only send or
+  //!
-  //!      send-once rights.
+  //! \return `true` on success, `false` on failure with a message logged. On
-  static void RunClient(int pipe_read,
+  //!     failure, the port right corresponding to a \a right_type of
-                        mach_port_t port,
+  //!     `MACH_MSG_TYPE_MOVE_*` is not consumed, and the caller must dispose of
-                        mach_msg_type_name_t right_type);
+  //!     the right if necessary.
+  static bool RunClientForFD(base::ScopedFD client_read_fd,
+                             mach_port_t port,
+                             mach_msg_type_name_t right_type);
 
  private:
   //! \brief Runs the read-from-pipe portion of the client’s side of the
   //!     handshake. This is an implementation detail of RunClient and is only
   //!     exposed for testing purposes.
   //!
+  //! When using this function and RunClientInternal_SendCheckIn(), the caller
+  //! is responsible for closing \a pipe_read at an appropriate time, normally
+  //! after calling RunClientInternal_SendCheckIn().
+  //!
   //! \param[in] pipe_read The “read” side of the pipe shared with the server
   //!     process.
   //! \param[out] token The token value read from \a pipe_read.
   //! \param[out] service_name The service name as registered with the bootstrap
   //!     server, read from \a pipe_read.
-  static void RunClientInternal_ReadPipe(int pipe_read,
+  //!
+  //! \return `true` on success, `false` on failure with a message logged.
+  static bool RunClientInternal_ReadPipe(int pipe_read,
                                          child_port_token_t* token,
                                          std::string* service_name);
 
@@ -188,34 +297,25 @@ class ChildPortHandshake : public ChildPortServer::Interface {
   //!     This is an implementation detail of RunClient and is only exposed for
   //!     testing purposes.
   //!
+  //! When using this RunClientInternal_ReadPipe() and this function, the caller
+  //! is responsible for closing the “read” side of the pipe at an appropriate
+  //! time, normally after calling this function.
+  //!
   //! \param[in] service_name The service name as registered with the bootstrap
   //!     server, to be looked up with `bootstrap_look_up()`.
   //! \param[in] token The token value to provide during check-in.
   //! \param[in] port The port that will be passed to the server by
   //!     `child_port_check_in()`.
-  //! \param[in] right_type The right type to furnish the parent with.
+  //! \param[in] right_type The right type to furnish the server with.
-  static void RunClientInternal_SendCheckIn(const std::string& service_name,
+  //!
+  //! \return `true` on success, `false` on failure with a message logged.
+  static bool RunClientInternal_SendCheckIn(const std::string& service_name,
                                             child_port_token_t token,
                                             mach_port_t port,
                                             mach_msg_type_name_t right_type);
 
-  // Communicates the token from RunServer(), where it’s generated, to
+  base::ScopedFD client_read_fd_;
-  // HandleChildPortCheckIn(), where it’s validated.
+  base::ScopedFD server_write_fd_;
-  child_port_token_t token_;
-
-  base::ScopedFD pipe_read_;
-  base::ScopedFD pipe_write_;
-
-  // Communicates the port received from the client from
-  // HandleChildPortCheckIn(), where it’s received, to RunServer(), where it’s
-  // returned. This is strongly-owned, but ownership is transferred to
-  // RunServer()’s caller.
-  mach_port_t child_port_;
-
-  // Communicates that a check-in with a valid token was received by
-  // HandleChildPortCheckIn(), and that the value of child_port_ should be
-  // returned to RunServer()’s caller.
-  bool checked_in_;
 
   friend class test::ChildPortHandshakeTest;
 

util/mach/child_port_handshake_test.cc

@@ -26,161 +26,355 @@ namespace {
 
 class ChildPortHandshakeTest : public Multiprocess {
  public:
-  enum TestType {
+  enum class ClientProcess {
-    kTestTypeChildChecksIn = 0,
+    // The child runs the client and the parent runs the server.
-    kTestTypeChildDoesNotCheckIn_ReadsPipe,
+    kChildClient = 0,
-    kTestTypeChildDoesNotCheckIn,
+
-    kTestTypeTokenIncorrect,
+    // The parent runs the client and the child runs the server.
-    kTestTypeTokenIncorrectThenCorrect,
+    kParentClient,
   };
 
-  explicit ChildPortHandshakeTest(TestType test_type)
+  enum class TestType {
-      : Multiprocess(), child_port_handshake_(), test_type_(test_type) {}
+    // The client checks in with the server, transferring a receive right.
-  ~ChildPortHandshakeTest() {}
+    kClientChecksIn_ReceiveRight = 0,
+
+    // In this test, the client checks in with the server normally. It sends a
+    // copy of its bootstrap port to the server, because both parent and child
+    // should have the same bootstrap port, allowing for verification.
+    kClientChecksIn_SendRight,
+
+    // The client checks in with the server, transferring a send-once right.
+    kClientChecksIn_SendOnceRight,
+
+    // In this test, the client reads from its pipe, and subsequently exits
+    // without checking in. This tests that the server properly detects that it
+    // has lost its client after sending instructions to it via the pipe, while
+    // waiting for a check-in message.
+    kClientDoesNotCheckIn,
+
+    // In this test, the client exits without checking in. This tests that the
+    // server properly detects that it has lost a client. Whether or not the
+    // client closes the pipe before the server writes to it is a race, and the
+    // server needs to be able to detect client loss in both cases, so the
+    // ClientDoesNotCheckIn_ReadsPipe and NoClient tests also exist to test
+    // these individual cases more deterministically.
+    kClientDoesNotCheckIn_ReadsPipe,
+
+    // In this test, the client checks in with the server with an incorrect
+    // token value and a copy of its own task port. The server should reject the
+    // message because of the invalid token, and return MACH_PORT_NULL to its
+    // caller.
+    kTokenIncorrect,
+
+    // In this test, the client checks in with the server with an incorrect
+    // token value and a copy of its own task port, and subsequently, the
+    // correct token value and a copy of its bootstrap port. The server should
+    // reject the first because of the invalid token, but it should continue
+    // waiting for a message with a valid token as long as the pipe remains
+    // open. It should wind wind up returning the bootstrap port, allowing for
+    // verification.
+    kTokenIncorrectThenCorrect,
+
+    // The server dies. The failure should be reported in the client. This test
+    // type is only compatible with ClientProcess::kParentClient.
+    kServerDies,
+  };
+
+  ChildPortHandshakeTest(ClientProcess client_process, TestType test_type)
+      : Multiprocess(),
+        child_port_handshake_(),
+        client_process_(client_process),
+        test_type_(test_type) {
+  }
+
+  ~ChildPortHandshakeTest() {
+  }
 
  private:
+  void RunServer() {
+    if (test_type_ == TestType::kServerDies) {
+      return;
+    }
+
+    base::mac::ScopedMachReceiveRight receive_right;
+    base::mac::ScopedMachSendRight send_right;
+    if (test_type_ == TestType::kClientChecksIn_ReceiveRight) {
+      receive_right.reset(child_port_handshake_.RunServer(
+          ChildPortHandshake::PortRightType::kReceiveRight));
+    } else {
+      send_right.reset(child_port_handshake_.RunServer(
+          ChildPortHandshake::PortRightType::kSendRight));
+    }
+
+    switch (test_type_) {
+      case TestType::kClientChecksIn_ReceiveRight:
+        EXPECT_TRUE(receive_right.is_valid());
+        break;
+
+      case TestType::kClientChecksIn_SendRight:
+      case TestType::kTokenIncorrectThenCorrect:
+        EXPECT_EQ(bootstrap_port, send_right);
+        break;
+
+      case TestType::kClientChecksIn_SendOnceRight:
+        EXPECT_TRUE(send_right.is_valid());
+        EXPECT_NE(bootstrap_port, send_right);
+        break;
+
+      case TestType::kClientDoesNotCheckIn:
+      case TestType::kClientDoesNotCheckIn_ReadsPipe:
+      case TestType::kTokenIncorrect:
+        EXPECT_FALSE(send_right.is_valid());
+        break;
+
+      case TestType::kServerDies:
+        // This was special-cased as an early return above.
+        FAIL();
+        break;
+    }
+  }
+
+  void RunClient() {
+    switch (test_type_) {
+      case TestType::kClientChecksIn_SendRight: {
+        ASSERT_TRUE(child_port_handshake_.RunClient(bootstrap_port,
+                                                    MACH_MSG_TYPE_COPY_SEND));
+        break;
+      }
+
+      case TestType::kClientChecksIn_ReceiveRight: {
+        mach_port_t receive_right = NewMachPort(MACH_PORT_RIGHT_RECEIVE);
+        ASSERT_TRUE(child_port_handshake_.RunClient(
+              receive_right, MACH_MSG_TYPE_MOVE_RECEIVE));
+        break;
+      }
+
+      case TestType::kClientChecksIn_SendOnceRight: {
+        base::mac::ScopedMachReceiveRight receive_right(
+            NewMachPort(MACH_PORT_RIGHT_RECEIVE));
+        ASSERT_TRUE(child_port_handshake_.RunClient(
+            receive_right.get(), MACH_MSG_TYPE_MAKE_SEND_ONCE));
+        break;
+      }
+
+      case TestType::kClientDoesNotCheckIn: {
+        child_port_handshake_.ServerWriteFD().reset();
+        child_port_handshake_.ClientReadFD().reset();
+        break;
+      }
+
+      case TestType::kClientDoesNotCheckIn_ReadsPipe: {
+        // Don’t run the standard client routine. Instead, drain the pipe, which
+        // will get the parent to the point that it begins waiting for a
+        // check-in message. Then, exit. The pipe is drained using the same
+        // implementation that the real client would use.
+        child_port_handshake_.ServerWriteFD().reset();
+        base::ScopedFD client_read_fd = child_port_handshake_.ClientReadFD();
+        child_port_token_t token;
+        std::string service_name;
+        ASSERT_TRUE(ChildPortHandshake::RunClientInternal_ReadPipe(
+            client_read_fd.get(), &token, &service_name));
+        break;
+      }
+
+      case TestType::kTokenIncorrect: {
+        // Don’t run the standard client routine. Instead, read the token and
+        // service name, mutate the token, and then check in with the bad token.
+        // The parent should reject the message.
+        child_port_handshake_.ServerWriteFD().reset();
+        base::ScopedFD client_read_fd = child_port_handshake_.ClientReadFD();
+        child_port_token_t token;
+        std::string service_name;
+        ASSERT_TRUE(ChildPortHandshake::RunClientInternal_ReadPipe(
+            client_read_fd.get(), &token, &service_name));
+        child_port_token_t bad_token = ~token;
+        ASSERT_TRUE(ChildPortHandshake::RunClientInternal_SendCheckIn(
+            service_name,
+            bad_token,
+            mach_task_self(),
+            MACH_MSG_TYPE_COPY_SEND));
+        break;
+      }
+
+      case TestType::kTokenIncorrectThenCorrect: {
+        // Don’t run the standard client routine. Instead, read the token and
+        // service name. Mutate the token, and check in with the bad token,
+        // expecting the parent to reject the message. Then, check in with the
+        // correct token, expecting the parent to accept it.
+        child_port_handshake_.ServerWriteFD().reset();
+        base::ScopedFD client_read_fd = child_port_handshake_.ClientReadFD();
+        child_port_token_t token;
+        std::string service_name;
+        ASSERT_TRUE(ChildPortHandshake::RunClientInternal_ReadPipe(
+            client_read_fd.release(), &token, &service_name));
+        child_port_token_t bad_token = ~token;
+        ASSERT_TRUE(ChildPortHandshake::RunClientInternal_SendCheckIn(
+            service_name,
+            bad_token,
+            mach_task_self(),
+            MACH_MSG_TYPE_COPY_SEND));
+        ASSERT_TRUE(ChildPortHandshake::RunClientInternal_SendCheckIn(
+            service_name, token, bootstrap_port, MACH_MSG_TYPE_COPY_SEND));
+        break;
+      }
+
+      case TestType::kServerDies: {
+        ASSERT_EQ(ClientProcess::kParentClient, client_process_);
+        ASSERT_FALSE(child_port_handshake_.RunClient(bootstrap_port,
+                                                     MACH_MSG_TYPE_COPY_SEND));
+        break;
+      }
+    }
+  }
+
   // Multiprocess:
 
   void MultiprocessParent() override {
-    base::mac::ScopedMachSendRight child_port(
+    switch (client_process_) {
-        child_port_handshake_.RunServer());
+      case ClientProcess::kChildClient:
-    switch (test_type_) {
+        RunServer();
-      case kTestTypeChildChecksIn:
-      case kTestTypeTokenIncorrectThenCorrect:
-        EXPECT_EQ(bootstrap_port, child_port);
         break;
-
+      case ClientProcess::kParentClient:
-      case kTestTypeChildDoesNotCheckIn_ReadsPipe:
+        RunClient();
-      case kTestTypeChildDoesNotCheckIn:
-      case kTestTypeTokenIncorrect:
-        EXPECT_EQ(kMachPortNull, child_port);
         break;
     }
   }
 
   void MultiprocessChild() override {
-    int read_pipe = child_port_handshake_.ReadPipeFD();
+    switch (client_process_) {
-    switch (test_type_) {
+      case ClientProcess::kChildClient:
-      case kTestTypeChildChecksIn:
+        RunClient();
-        ChildPortHandshake::RunClient(
-            read_pipe, bootstrap_port, MACH_MSG_TYPE_COPY_SEND);
         break;
-
+      case ClientProcess::kParentClient:
-      case kTestTypeChildDoesNotCheckIn_ReadsPipe: {
+        RunServer();
-        // Don’t run the standard client routine. Instead, drain the pipe, which
-        // will get the parent to the point that it begins waiting for a
-        // check-in message. Then, exit. The pipe is drained using the same
-        // implementation that the real client would use.
-        child_port_token_t token;
-        std::string service_name;
-        ChildPortHandshake::RunClientInternal_ReadPipe(
-            read_pipe, &token, &service_name);
         break;
-      }
-
-      case kTestTypeChildDoesNotCheckIn:
-        break;
-
-      case kTestTypeTokenIncorrect: {
-        // Don’t run the standard client routine. Instead, read the token and
-        // service name, mutate the token, and then check in with the bad token.
-        // The parent should reject the message.
-        child_port_token_t token;
-        std::string service_name;
-        ChildPortHandshake::RunClientInternal_ReadPipe(
-            read_pipe, &token, &service_name);
-        child_port_token_t bad_token = ~token;
-        ChildPortHandshake::RunClientInternal_SendCheckIn(
-            service_name, bad_token, mach_task_self(), MACH_MSG_TYPE_COPY_SEND);
-        break;
-      }
-
-      case kTestTypeTokenIncorrectThenCorrect: {
-        // Don’t run the standard client routine. Instead, read the token and
-        // service name. Mutate the token, and check in with the bad token,
-        // expecting the parent to reject the message. Then, check in with the
-        // correct token, expecting the parent to accept it.
-        child_port_token_t token;
-        std::string service_name;
-        ChildPortHandshake::RunClientInternal_ReadPipe(
-            read_pipe, &token, &service_name);
-        child_port_token_t bad_token = ~token;
-        ChildPortHandshake::RunClientInternal_SendCheckIn(
-            service_name, bad_token, mach_task_self(), MACH_MSG_TYPE_COPY_SEND);
-        ChildPortHandshake::RunClientInternal_SendCheckIn(
-            service_name, token, bootstrap_port, MACH_MSG_TYPE_COPY_SEND);
-        break;
-      }
     }
   }
 
  private:
   ChildPortHandshake child_port_handshake_;
+  ClientProcess client_process_;
   TestType test_type_;
 
   DISALLOW_COPY_AND_ASSIGN(ChildPortHandshakeTest);
 };
 
-TEST(ChildPortHandshake, ChildChecksIn) {
+TEST(ChildPortHandshake, ChildClientChecksIn_ReceiveRight) {
-  // In this test, the client checks in with the server normally. It sends a
-  // copy of its bootstrap port to the server, because both parent and child
-  // should have the same bootstrap port, allowing for verification.
-  ChildPortHandshakeTest test(ChildPortHandshakeTest::kTestTypeChildChecksIn);
-  test.Run();
-}
-
-TEST(ChildPortHandshake, ChildDoesNotCheckIn) {
-  // In this test, the client exits without checking in. This tests that the
-  // server properly detects that it has lost a client. Whether or not the
-  // client closes the pipe before the server writes to it is a race, and the
-  // server needs to be able to detect client loss in both cases, so the
-  // ChildDoesNotCheckIn_ReadsPipe and NoChild tests also exist to test these
-  // individual cases more deterministically.
   ChildPortHandshakeTest test(
-      ChildPortHandshakeTest::kTestTypeChildDoesNotCheckIn);
+      ChildPortHandshakeTest::ClientProcess::kChildClient,
+      ChildPortHandshakeTest::TestType::kClientChecksIn_ReceiveRight);
   test.Run();
 }
 
-TEST(ChildPortHandshake, ChildDoesNotCheckIn_ReadsPipe) {
+TEST(ChildPortHandshake, ChildClientChecksIn_SendRight) {
-  // In this test, the client reads from its pipe, and subsequently exits
-  // without checking in. This tests that the server properly detects that it
-  // has lost its client after sending instructions to it via the pipe, while
-  // waiting for a check-in message.
   ChildPortHandshakeTest test(
-      ChildPortHandshakeTest::kTestTypeChildDoesNotCheckIn_ReadsPipe);
+      ChildPortHandshakeTest::ClientProcess::kChildClient,
+      ChildPortHandshakeTest::TestType::kClientChecksIn_SendRight);
   test.Run();
 }
 
-TEST(ChildPortHandshake, TokenIncorrect) {
+TEST(ChildPortHandshake, ChildClientChecksIn_SendOnceRight) {
-  // In this test, the client checks in with the server with an incorrect token
-  // value and a copy of its own task port. The server should reject the message
-  // because of the invalid token, and return MACH_PORT_NULL to its caller.
-  ChildPortHandshakeTest test(ChildPortHandshakeTest::kTestTypeTokenIncorrect);
-  test.Run();
-}
-
-TEST(ChildPortHandshake, TokenIncorrectThenCorrect) {
-  // In this test, the client checks in with the server with an incorrect token
-  // value and a copy of its own task port, and subsequently, the correct token
-  // value and a copy of its bootstrap port. The server should reject the first
-  // because of the invalid token, but it should continue waiting for a message
-  // with a valid token as long as the pipe remains open. It should wind wind up
-  // returning the bootstrap port, allowing for verification.
   ChildPortHandshakeTest test(
-      ChildPortHandshakeTest::kTestTypeTokenIncorrectThenCorrect);
+      ChildPortHandshakeTest::ClientProcess::kChildClient,
+      ChildPortHandshakeTest::TestType::kClientChecksIn_SendOnceRight);
   test.Run();
 }
 
-TEST(ChildPortHandshake, NoChild) {
+TEST(ChildPortHandshake, ChildClientDoesNotCheckIn) {
-  // In this test, the client never checks in with the parent because the child
+  ChildPortHandshakeTest test(
-  // never even runs. This tests that the server properly detects that it has
+      ChildPortHandshakeTest::ClientProcess::kChildClient,
-  // no client at all, and does not terminate execution with an error such as
+      ChildPortHandshakeTest::TestType::kClientDoesNotCheckIn);
+  test.Run();
+}
+
+TEST(ChildPortHandshake, ChildClientDoesNotCheckIn_ReadsPipe) {
+  ChildPortHandshakeTest test(
+      ChildPortHandshakeTest::ClientProcess::kChildClient,
+      ChildPortHandshakeTest::TestType::kClientDoesNotCheckIn_ReadsPipe);
+  test.Run();
+}
+
+TEST(ChildPortHandshake, ChildClientTokenIncorrect) {
+  ChildPortHandshakeTest test(
+      ChildPortHandshakeTest::ClientProcess::kChildClient,
+      ChildPortHandshakeTest::TestType::kTokenIncorrect);
+  test.Run();
+}
+
+TEST(ChildPortHandshake, ChildClientTokenIncorrectThenCorrect) {
+  ChildPortHandshakeTest test(
+      ChildPortHandshakeTest::ClientProcess::kChildClient,
+      ChildPortHandshakeTest::TestType::kTokenIncorrectThenCorrect);
+  test.Run();
+}
+
+TEST(ChildPortHandshake, ParentClientChecksIn_ReceiveRight) {
+  ChildPortHandshakeTest test(
+      ChildPortHandshakeTest::ClientProcess::kParentClient,
+      ChildPortHandshakeTest::TestType::kClientChecksIn_ReceiveRight);
+  test.Run();
+}
+
+TEST(ChildPortHandshake, ParentClientChecksIn_SendRight) {
+  ChildPortHandshakeTest test(
+      ChildPortHandshakeTest::ClientProcess::kParentClient,
+      ChildPortHandshakeTest::TestType::kClientChecksIn_SendRight);
+  test.Run();
+}
+
+TEST(ChildPortHandshake, ParentClientChecksIn_SendOnceRight) {
+  ChildPortHandshakeTest test(
+      ChildPortHandshakeTest::ClientProcess::kParentClient,
+      ChildPortHandshakeTest::TestType::kClientChecksIn_SendOnceRight);
+  test.Run();
+}
+
+TEST(ChildPortHandshake, ParentClientDoesNotCheckIn) {
+  ChildPortHandshakeTest test(
+      ChildPortHandshakeTest::ClientProcess::kParentClient,
+      ChildPortHandshakeTest::TestType::kClientDoesNotCheckIn);
+  test.Run();
+}
+
+TEST(ChildPortHandshake, ParentClientDoesNotCheckIn_ReadsPipe) {
+  ChildPortHandshakeTest test(
+      ChildPortHandshakeTest::ClientProcess::kParentClient,
+      ChildPortHandshakeTest::TestType::kClientDoesNotCheckIn_ReadsPipe);
+  test.Run();
+}
+
+TEST(ChildPortHandshake, ParentClientTokenIncorrect) {
+  ChildPortHandshakeTest test(
+      ChildPortHandshakeTest::ClientProcess::kParentClient,
+      ChildPortHandshakeTest::TestType::kTokenIncorrect);
+  test.Run();
+}
+
+TEST(ChildPortHandshake, ParentClientTokenIncorrectThenCorrect) {
+  ChildPortHandshakeTest test(
+      ChildPortHandshakeTest::ClientProcess::kParentClient,
+      ChildPortHandshakeTest::TestType::kTokenIncorrectThenCorrect);
+  test.Run();
+}
+
+TEST(ChildPortHandshake, ParentClientServerDies) {
+  ChildPortHandshakeTest test(
+      ChildPortHandshakeTest::ClientProcess::kParentClient,
+      ChildPortHandshakeTest::TestType::kServerDies);
+  test.Run();
+}
+
+TEST(ChildPortHandshake, NoClient) {
+  // In this test, the client never checks in with the server because it never
+  // even runs. This tests that the server properly detects that it has no
+  // client at all, and does not terminate execution with an error such as
   // “broken pipe” when attempting to send instructions to the client. This test
-  // is similar to ChildDoesNotCheckIn, but because there’s no child at all, the
+  // is similar to kClientDoesNotCheckIn, but because there’s no client at all,
-  // server is guaranteed to see that its pipe partner is gone.
+  // the server is guaranteed to see that its pipe partner is gone.
   ChildPortHandshake child_port_handshake;
-  base::mac::ScopedMachSendRight child_port(child_port_handshake.RunServer());
+  base::mac::ScopedMachSendRight child_port(child_port_handshake.RunServer(
-  EXPECT_EQ(kMachPortNull, child_port);
+      ChildPortHandshake::PortRightType::kSendRight));
+  EXPECT_FALSE(child_port.is_valid());
 }
 
 }  // namespace

util/mach/mach_extensions_test.cc

@@ -138,7 +138,7 @@ TEST(MachExtensions, BootstrapCheckInAndLookUp) {
       report_crash(BootstrapLookUp("com.apple.ReportCrash"));
   EXPECT_NE(report_crash, kMachPortNull);
 
-  std::string service_name = "com.googlecode.crashpad.test.bootstrap_check_in.";
+  std::string service_name = "org.chromium.crashpad.test.bootstrap_check_in.";
   for (int index = 0; index < 16; ++index) {
     service_name.append(1, base::RandInt('A', 'Z'));
   }

util/mach/mach_message.cc

@@ -20,6 +20,7 @@
 #include <limits>
 
 #include "base/logging.h"
+#include "base/mac/mach_logging.h"
 #include "util/misc/clock.h"
 #include "util/misc/implicit_cast.h"
 
@@ -249,4 +250,37 @@ pid_t AuditPIDFromMachMessageTrailer(const mach_msg_trailer_t* trailer) {
   return audit_pid;
 }
 
+bool MachMessageDestroyReceivedPort(mach_port_t port,
+                                    mach_msg_type_name_t port_right_type) {
+  // This implements a subset of 10.10.5
+  // xnu-2782.40.9/libsyscall/mach/mach_msg.c mach_msg_destroy_port() that deals
+  // only with port rights that can be received in Mach messages.
+  switch (port_right_type) {
+    case MACH_MSG_TYPE_PORT_RECEIVE: {
+      kern_return_t kr = mach_port_mod_refs(
+          mach_task_self(), port, MACH_PORT_RIGHT_RECEIVE, -1);
+      if (kr != KERN_SUCCESS) {
+        MACH_LOG(ERROR, kr) << "mach_port_mod_refs";
+        return false;
+      }
+      return true;
+    }
+
+    case MACH_MSG_TYPE_PORT_SEND:
+    case MACH_MSG_TYPE_PORT_SEND_ONCE: {
+      kern_return_t kr = mach_port_deallocate(mach_task_self(), port);
+      if (kr != KERN_SUCCESS) {
+        MACH_LOG(ERROR, kr) << "mach_port_deallocate";
+        return false;
+      }
+      return true;
+    }
+
+    default: {
+      LOG(ERROR) << "unexpected port right type " << port_right_type;
+      return false;
+    }
+  }
+}
+
 }  // namespace crashpad

util/mach/mach_message.h

@@ -174,6 +174,21 @@ const mach_msg_trailer_t* MachMessageTrailerFromHeader(
 //!     audit information.
 pid_t AuditPIDFromMachMessageTrailer(const mach_msg_trailer_t* trailer);
 
+//! \brief Destroys or deallocates a Mach port received in a Mach message.
+//!
+//! This function disposes of port rights received in a Mach message. Receive
+//! rights will be destroyed with `mach_port_mod_refs()`. Send and send-once
+//! rights will be deallocated with `mach_port_deallocate()`.
+//!
+//! \param[in] port The port to destroy or deallocate.
+//! \param[in] port_right_type The right type held for \a port:
+//!     `MACH_MSG_TYPE_PORT_RECEIVE`, `MACH_MSG_TYPE_PORT_SEND`, or
+//!     `MACH_MSG_TYPE_PORT_SEND_ONCE`.
+//!
+//! \return `true` on success, or `false` on failure with a message logged.
+bool MachMessageDestroyReceivedPort(mach_port_t port,
+                                    mach_msg_type_name_t port_right_type);
+
 }  // namespace crashpad
 
 #endif  // CRASHPAD_UTIL_MACH_MACH_MESSAGE_H_

util/mach/mach_message_server.h

@@ -157,8 +157,8 @@ class MachMessageServer {
   //!     #kPersistent, the timeout applies to the overall duration of this
   //!     function, not to any individual `mach_msg()` call.
   //!
-  //! \return On success, `KERN_SUCCESS` (when \a persistent is #kOneShot) or
+  //! \return On success, `MACH_MSG_SUCCESS` (when \a persistent is #kOneShot)
-  //!     `MACH_RCV_TIMED_OUT` (when \a persistent is #kOneShot and \a
+  //!     or `MACH_RCV_TIMED_OUT` (when \a persistent is #kOneShot and \a
   //!     timeout_ms is not #kMachMessageTimeoutWaitIndefinitely). This function
   //!     has no successful return value when \a persistent is #kPersistent and
   //!     \a timeout_ms is #kMachMessageTimeoutWaitIndefinitely. On failure,

util/mach/mach_message_server_test.cc

@@ -216,8 +216,8 @@ class TestMachMessageServer : public MachMessageServer::Interface,
         MACH_MSGH_BITS(MACH_MSG_TYPE_MOVE_SEND, MACH_MSG_TYPE_MOVE_SEND) |
         (options_.client_send_complex ? MACH_MSGH_BITS_COMPLEX : 0);
     EXPECT_EQ(expect_msgh_bits, request->header.msgh_bits);
-    EXPECT_EQ(options_.client_send_large ? sizeof(LargeRequestMessage) :
+    EXPECT_EQ(options_.client_send_large ? sizeof(LargeRequestMessage)
-                                           sizeof(RequestMessage),
+                                         : sizeof(RequestMessage),
               request->header.msgh_size);
     if (options_.client_reply_port_type == Options::kReplyPortNormal) {
       EXPECT_EQ(RemotePort(), request->header.msgh_remote_port);
@@ -277,8 +277,8 @@ class TestMachMessageServer : public MachMessageServer::Interface,
 
   std::set<mach_msg_id_t> MachMessageServerRequestIDs() override {
     const mach_msg_id_t request_ids[] = {kRequestMessageID};
-    return std::set<mach_msg_id_t>(
+    return std::set<mach_msg_id_t>(&request_ids[0],
-        &request_ids[0], &request_ids[arraysize(request_ids)]);
+                                   &request_ids[arraysize(request_ids)]);
   }
 
   mach_msg_size_t MachMessageServerRequestSize() override {
@@ -368,8 +368,8 @@ class TestMachMessageServer : public MachMessageServer::Interface,
         EXPECT_EQ(MACH_PORT_TYPE_SEND, type);
 
         // Destroy the resources here.
-        kr = mach_port_deallocate(
+        kr = mach_port_deallocate(mach_task_self(),
-            mach_task_self(), parent_complex_message_port_);
+                                  parent_complex_message_port_);
         EXPECT_EQ(KERN_SUCCESS, kr)
             << MachErrorMessage(kr, "mach_port_deallocate");
       }
@@ -378,8 +378,8 @@ class TestMachMessageServer : public MachMessageServer::Interface,
       // this task so soon. It’s possible that something else in this task could
       // have reused the name, but it’s unlikely for that to have happened in
       // this test environment.
-      kr = mach_port_type(
+      kr =
-          mach_task_self(), parent_complex_message_port_, &type);
+          mach_port_type(mach_task_self(), parent_complex_message_port_, &type);
       EXPECT_EQ(KERN_INVALID_NAME, kr)
           << MachErrorMessage(kr, "mach_port_type");
     }

util/mach/mach_message_test.cc

@@ -16,6 +16,7 @@
 
 #include <unistd.h>
 
+#include "base/basictypes.h"
 #include "base/mac/scoped_mach_port.h"
 #include "gtest/gtest.h"
 #include "test/mac/mach_errors.h"
@@ -148,6 +149,55 @@ TEST(MachMessage, AuditPIDFromMachMessageTrailer) {
   EXPECT_EQ(getpid(), AuditPIDFromMachMessageTrailer(&receive.trailer));
 }
 
+TEST(MachMessage, MachMessageDestroyReceivedPort) {
+  mach_port_t port = NewMachPort(MACH_PORT_RIGHT_RECEIVE);
+  ASSERT_NE(kMachPortNull, port);
+  EXPECT_TRUE(MachMessageDestroyReceivedPort(port, MACH_MSG_TYPE_PORT_RECEIVE));
+
+  base::mac::ScopedMachReceiveRight receive(
+      NewMachPort(MACH_PORT_RIGHT_RECEIVE));
+  mach_msg_type_name_t right_type;
+  kern_return_t kr = mach_port_extract_right(mach_task_self(),
+                                             receive.get(),
+                                             MACH_MSG_TYPE_MAKE_SEND,
+                                             &port,
+                                             &right_type);
+  ASSERT_EQ(KERN_SUCCESS, kr)
+      << MachErrorMessage(kr, "mach_port_extract_right");
+  ASSERT_EQ(receive, port);
+  ASSERT_EQ(implicit_cast<mach_msg_type_name_t>(MACH_MSG_TYPE_PORT_SEND),
+            right_type);
+  EXPECT_TRUE(MachMessageDestroyReceivedPort(port, MACH_MSG_TYPE_PORT_SEND));
+
+  kr = mach_port_extract_right(mach_task_self(),
+                               receive.get(),
+                               MACH_MSG_TYPE_MAKE_SEND_ONCE,
+                               &port,
+                               &right_type);
+  ASSERT_EQ(KERN_SUCCESS, kr)
+      << MachErrorMessage(kr, "mach_port_extract_right");
+  ASSERT_NE(kMachPortNull, port);
+  EXPECT_NE(receive, port);
+  ASSERT_EQ(implicit_cast<mach_msg_type_name_t>(MACH_MSG_TYPE_PORT_SEND_ONCE),
+            right_type);
+  EXPECT_TRUE(
+      MachMessageDestroyReceivedPort(port, MACH_MSG_TYPE_PORT_SEND_ONCE));
+
+  kr = mach_port_extract_right(mach_task_self(),
+                               receive.get(),
+                               MACH_MSG_TYPE_MAKE_SEND,
+                               &port,
+                               &right_type);
+  ASSERT_EQ(KERN_SUCCESS, kr)
+      << MachErrorMessage(kr, "mach_port_extract_right");
+  ASSERT_EQ(receive, port);
+  ASSERT_EQ(implicit_cast<mach_msg_type_name_t>(MACH_MSG_TYPE_PORT_SEND),
+            right_type);
+  EXPECT_TRUE(MachMessageDestroyReceivedPort(port, MACH_MSG_TYPE_PORT_RECEIVE));
+  ignore_result(receive.release());
+  EXPECT_TRUE(MachMessageDestroyReceivedPort(port, MACH_MSG_TYPE_PORT_SEND));
+}
+
 }  // namespace
 }  // namespace test
 }  // namespace crashpad

util/win/capture_context.asm

@@ -351,8 +351,7 @@ $FXSave:
   ; Free the stack space used for the temporary fxsave area.
   lea esp, [ebp-8]
 
-  ; TODO(mark): AVX/xsave support.
+  ; TODO(mark): AVX/xsave support. https://crashpad.chromium.org/bug/58
-  ; https://code.google.com/p/crashpad/issues/detail?id=58
 
 $FXSaveDone:
   ; fnsave reinitializes the FPU with an implicit finit operation, so use frstor
@@ -491,8 +490,7 @@ CAPTURECONTEXT_SYMBOL proc frame
   ; declared as 16-byte-aligned, which is correct for this operation.
   fxsave [rcx.CONTEXT].c_FltSave
 
-  ; TODO(mark): AVX/xsave support.
+  ; TODO(mark): AVX/xsave support. https://crashpad.chromium.org/bug/58
-  ; https://code.google.com/p/crashpad/issues/detail?id=58
 
   ; The register parameter home address fields aren’t used, so zero them out.
   mov [rcx.CONTEXT].c_P1Home, 0

util/win/exception_handler_server.cc

@@ -234,8 +234,9 @@ class ClientData {
 ExceptionHandlerServer::Delegate::~Delegate() {
 }
 
-ExceptionHandlerServer::ExceptionHandlerServer()
+ExceptionHandlerServer::ExceptionHandlerServer(const std::string& pipe_name)
-    : port_(CreateIoCompletionPort(INVALID_HANDLE_VALUE, nullptr, 0, 1)),
+    : pipe_name_(pipe_name),
+      port_(CreateIoCompletionPort(INVALID_HANDLE_VALUE, nullptr, 0, 1)),
       clients_lock_(),
       clients_() {
 }
@@ -243,13 +244,12 @@ ExceptionHandlerServer::ExceptionHandlerServer()
 ExceptionHandlerServer::~ExceptionHandlerServer() {
 }
 
-void ExceptionHandlerServer::Run(Delegate* delegate,
+void ExceptionHandlerServer::Run(Delegate* delegate) {
-                                 const std::string& pipe_name) {
   uint64_t shutdown_token = base::RandUint64();
   // We create two pipe instances, so that there's one listening while the
   // PipeServiceProc is processing a registration.
   ScopedKernelHANDLE thread_handles[2];
-  base::string16 pipe_name_16(base::UTF8ToUTF16(pipe_name));
+  base::string16 pipe_name_16(base::UTF8ToUTF16(pipe_name_));
   for (int i = 0; i < arraysize(thread_handles); ++i) {
     HANDLE pipe =
         CreateNamedPipe(pipe_name_16.c_str(),
@@ -311,7 +311,7 @@ void ExceptionHandlerServer::Run(Delegate* delegate,
     message.type = ClientToServerMessage::kShutdown;
     message.shutdown.token = shutdown_token;
     ServerToClientMessage response;
-    SendToCrashHandlerServer(base::UTF8ToUTF16(pipe_name),
+    SendToCrashHandlerServer(pipe_name_16,
                              reinterpret_cast<ClientToServerMessage&>(message),
                              &response);
   }

util/win/exception_handler_server.h

@@ -61,16 +61,18 @@ class ExceptionHandlerServer {
   };
 
   //! \brief Constructs the exception handling server.
-  ExceptionHandlerServer();
+  //!
+  //! \param[in] pipe_name The name of the pipe to listen on. Must be of the
+  //!     form "\\.\pipe\<some_name>".
+  explicit ExceptionHandlerServer(const std::string& pipe_name);
+
   ~ExceptionHandlerServer();
 
   //! \brief Runs the exception-handling server.
   //!
   //! \param[in] delegate The interface to which the exceptions are delegated
   //!     when they are caught in Run(). Ownership is not transferred.
-  //! \param[in] pipe_name The name of the pipe to listen on. Must be of the
+  void Run(Delegate* delegate);
-  //!     form "\\.\pipe\<some_name>".
-  void Run(Delegate* delegate, const std::string& pipe_name);
 
   //! \brief Stops the exception-handling server. Returns immediately. The
   //!     object must not be destroyed until Run() returns.
@@ -84,6 +86,7 @@ class ExceptionHandlerServer {
   static void __stdcall OnNonCrashDumpEvent(void* ctx, BOOLEAN);
   static void __stdcall OnProcessEnd(void* ctx, BOOLEAN);
 
+  std::string pipe_name_;
   ScopedKernelHANDLE port_;
 
   base::Lock clients_lock_;

util/win/exception_handler_server_test.cc

@@ -36,20 +36,18 @@ namespace {
 // Runs the ExceptionHandlerServer on a background thread.
 class RunServerThread : public Thread {
  public:
-  // Instantiates a thread which will invoke server->Run(delegate, pipe_name).
+  // Instantiates a thread which will invoke server->Run(delegate).
   RunServerThread(ExceptionHandlerServer* server,
-                  ExceptionHandlerServer::Delegate* delegate,
+                  ExceptionHandlerServer::Delegate* delegate)
-                  const std::string& pipe_name)
+      : server_(server), delegate_(delegate) {}
-      : server_(server), delegate_(delegate), pipe_name_(pipe_name) {}
   ~RunServerThread() override {}
 
  private:
   // Thread:
-  void ThreadMain() override { server_->Run(delegate_, pipe_name_); }
+  void ThreadMain() override { server_->Run(delegate_); }
 
   ExceptionHandlerServer* server_;
   ExceptionHandlerServer::Delegate* delegate_;
-  std::string pipe_name_;
 
   DISALLOW_COPY_AND_ASSIGN(RunServerThread);
 };
@@ -85,8 +83,8 @@ class ExceptionHandlerServerTest : public testing::Test {
                    base::StringPrintf("%08x", GetCurrentProcessId())),
         server_ready_(CreateEvent(nullptr, false, false, nullptr)),
         delegate_(server_ready_.get()),
-        server_(),
+        server_(pipe_name_),
-        server_thread_(&server_, &delegate_, pipe_name_) {}
+        server_thread_(&server_, &delegate_) {}
 
   TestDelegate& delegate() { return delegate_; }
   ExceptionHandlerServer& server() { return server_; }

util/win/process_info.h

@@ -180,7 +180,7 @@ class ProcessInfo {
   std::vector<MEMORY_BASIC_INFORMATION64> memory_info_;
 
   // Handles() is logically const, but updates this member on first retrieval.
-  // See https://code.google.com/p/crashpad/issues/detail?id=9.
+  // See https://crashpad.chromium.org/bug/9.
   mutable std::vector<Handle> handles_;
 
   bool is_64_bit_;