Mac: Other approach for IPCing child task_ts.

Also move mach_ipc_mac to base, where it's now used.

Based on http://www.foldr.org/~michaelw/log/2009/03/13/ , but uses a named connection instead. Do the IPC right after fork-time, so that the sandbox is not yet in effect.

See the codereview comments for a benchmark that proves that this shouldn't be expensive, and for pros and cons for using a named connection vs temporarily switching out the bootstrap port.

Works for worker processes too and seems more reliable in general.

Measured perf impact in http://src.chromium.org/viewvc/chrome?view=rev&revision=35888 , it's negligible.

BUG=13156
TEST=(requires that one enables the task manager in browser.cc)
1.) Open one tab that plays a youtube video
2.) Open a second and visit http://www.whatwg.org/demos/workers/primes/page.html
3.) Install e.g. the gmail checker extension
4.) Open the task manager
It should report metrics for
* one browser process
* two renderer processes
* one plugin process
* one worker process
* one extension process
Check that %cpu etc more or less match what Activity Monitor displays if you filter for "Chromium".
Also choose "Open all bookmarks" on the bookmarks bar with the task manager open and check that metrics for all tabs appear immediately.

Review URL: http://codereview.chromium.org/549002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@35977 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 273 insertions, 14 deletions

diff --git a/base/base.gypi b/base/base.gypi
index fc7c811..f7584c4 100644
--- a/base/base.gypi
+++ b/base/base.gypi
@@ -103,6 +103,8 @@
           'logging.h',
           'mac_util.h',
           'mac_util.mm',
+          'mach_ipc_mac.h',
+          'mach_ipc_mac.mm',
           'memory_debug.cc',
           'memory_debug.h',
           'message_loop.cc',
diff --git a/chrome/common/mach_ipc_mac.h b/base/mach_ipc_mac.h
index 42b9c65..7e9839c 100644
--- a/chrome/common/mach_ipc_mac.h
+++ b/base/mach_ipc_mac.h
@@ -258,7 +258,7 @@ class MachReceiveMessage : public MachMessage {
 //==============================================================================
 class MachSendMessage : public MachMessage {
  public:
-  MachSendMessage(int32_t message_id);
+  explicit MachSendMessage(int32_t message_id);
   MachSendMessage(void *storage, size_t storage_length, int32_t message_id);
 
  private:
@@ -272,11 +272,11 @@ class MachSendMessage : public MachMessage {
 class ReceivePort {
  public:
   // Creates a new mach port for receiving messages and registers a name for it
-  ReceivePort(const char *receive_port_name);
+  explicit ReceivePort(const char *receive_port_name);
 
   // Given an already existing mach port, use it.  We take ownership of the
   // port and deallocate it in our destructor.
-  ReceivePort(mach_port_t receive_port);
+  explicit ReceivePort(mach_port_t receive_port);
 
   // Create a new mach port for receiving messages
   ReceivePort();
@@ -302,11 +302,12 @@ class ReceivePort {
 class MachPortSender {
  public:
   // get a port with send rights corresponding to a named registered service
-  MachPortSender(const char *receive_port_name);
+  explicit MachPortSender(const char *receive_port_name);
 
 
-  // Given an already existing mach port, use it.
-  MachPortSender(mach_port_t send_port);
+  // Given an already existing mach port, use it. Does not take ownership of
+  // |send_port|.
+  explicit MachPortSender(mach_port_t send_port);
 
   kern_return_t SendMessage(MachSendMessage &message,
                             mach_msg_timeout_t timeout);
diff --git a/chrome/common/mach_ipc_mac.mm b/base/mach_ipc_mac.mm
index 7693d9a..fc78bcc 100644
--- a/chrome/common/mach_ipc_mac.mm
+++ b/base/mach_ipc_mac.mm
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/common/mach_ipc_mac.h"
+#include "base/mach_ipc_mac.h"
 
 #import <Foundation/Foundation.h>
 
diff --git a/base/process_util.h b/base/process_util.h
index cec37bd..eb21188 100644
--- a/base/process_util.h
+++ b/base/process_util.h
@@ -17,6 +17,7 @@
 // kinfo_proc is defined in <sys/sysctl.h>, but this forward declaration
 // is sufficient for the vector<kinfo_proc> below.
 struct kinfo_proc;
+#include <mach/mach.h>
 #elif defined(OS_POSIX)
 #include <dirent.h>
 #include <limits.h>
@@ -155,13 +156,25 @@ bool LaunchApp(const std::vector<std::string>& argv,
                const file_handle_mapping_vector& fds_to_remap,
                bool wait, ProcessHandle* process_handle);
 
-// Similar to above, but also (un)set environment variables in child process
+// Similar to the above, but also (un)set environment variables in child process
 // through |environ|.
 typedef std::vector<std::pair<std::string, std::string> > environment_vector;
 bool LaunchApp(const std::vector<std::string>& argv,
                const environment_vector& environ,
                const file_handle_mapping_vector& fds_to_remap,
                bool wait, ProcessHandle* process_handle);
+
+#if defined(OS_MACOSX)
+// Similar to the above, but also returns the new process's task_t if
+// |task_handle| is not NULL. If |task_handle| is not NULL, the caller is
+// responsible for calling |mach_port_deallocate()| on the returned handle.
+bool LaunchAppAndGetTask(const std::vector<std::string>& argv,
+                         const environment_vector& environ,
+                         const file_handle_mapping_vector& fds_to_remap,
+                         bool wait,
+                         task_t* task_handle,
+                         ProcessHandle* process_handle);
+#endif  // defined(OS_MACOSX)
 #endif  // defined(OS_POSIX)
 
 // Executes the application specified by cl. This function delegates to one
diff --git a/base/process_util_posix.cc b/base/process_util_posix.cc
index 21626732..a771fa2 100644
--- a/base/process_util_posix.cc
+++ b/base/process_util_posix.cc
@@ -21,11 +21,16 @@
 #include "base/logging.h"
 #include "base/platform_thread.h"
 #include "base/process_util.h"
+#include "base/rand_util.h"
 #include "base/scoped_ptr.h"
 #include "base/sys_info.h"
 #include "base/time.h"
 #include "base/waitable_event.h"
 
+#if defined(OS_MACOSX)
+#include "base/mach_ipc_mac.h"
+#endif
+
 const int kMicrosecondsPerSecond = 1000000;
 
 namespace base {
@@ -280,11 +285,148 @@ void SetAllFDsToCloseOnExec() {
   }
 }
 
+#if defined(OS_MACOSX)
+static std::string MachErrorCode(kern_return_t err) {
+  return StringPrintf("0x%x %s", err, mach_error_string(err));
+}
+
+// Forks the current process and returns the child's |task_t| in the parent
+// process.
+static pid_t fork_and_get_task(task_t* child_task) {
+  const int kTimeoutMs = 100;
+  kern_return_t err;
+
+  // Put a random number into the channel name, so that a compromised renderer
+  // can't pretend being the child that's forked off.
+  std::string mach_connection_name = StringPrintf(
+      "com.google.Chrome.samplingfork.%p.%d",
+      child_task, base::RandInt(0, std::numeric_limits<int>::max()));
+  ReceivePort parent_recv_port(mach_connection_name.c_str());
+
+  // Error handling philosophy: If Mach IPC fails, don't touch |child_task| but
+  // return a valid pid. If IPC fails in the child, the parent will have to wait
+  // until kTimeoutMs is over. This is not optimal, but I've never seen it
+  // happen, and stuff should still mostly work.
+  pid_t pid = fork();
+  switch (pid) {
+    case -1:
+      return pid;
+    case 0: {  // child
+      ReceivePort child_recv_port;
+
+      MachSendMessage child_message(/* id= */0);
+      if (!child_message.AddDescriptor(mach_task_self())) {
+        LOG(ERROR) << "child AddDescriptor(mach_task_self()) failed.";
+        return pid;
+      }
+      mach_port_t raw_child_recv_port = child_recv_port.GetPort();
+      if (!child_message.AddDescriptor(raw_child_recv_port)) {
+        LOG(ERROR) << "child AddDescriptor(" << raw_child_recv_port
+                   << ") failed.";
+        return pid;
+      }
+
+      MachPortSender child_sender(mach_connection_name.c_str());
+      err = child_sender.SendMessage(child_message, kTimeoutMs);
+      if (err != KERN_SUCCESS) {
+        LOG(ERROR) << "child SendMessage() failed: " << MachErrorCode(err);
+        return pid;
+      }
+
+      MachReceiveMessage parent_message;
+      err = child_recv_port.WaitForMessage(&parent_message, kTimeoutMs);
+      if (err != KERN_SUCCESS) {
+        LOG(ERROR) << "child WaitForMessage() failed: " << MachErrorCode(err);
+        return pid;
+      }
+
+      if (parent_message.GetTranslatedPort(0) == MACH_PORT_NULL) {
+        LOG(ERROR) << "child GetTranslatedPort(0) failed.";
+        return pid;
+      }
+      err = task_set_bootstrap_port(mach_task_self(),
+                                    parent_message.GetTranslatedPort(0));
+      if (err != KERN_SUCCESS) {
+        LOG(ERROR) << "child task_set_bootstrap_port() failed: "
+                   << MachErrorCode(err);
+        return pid;
+      }
+      break;
+    }
+    default: {  // parent
+      MachReceiveMessage child_message;
+      err = parent_recv_port.WaitForMessage(&child_message, kTimeoutMs);
+      if (err != KERN_SUCCESS) {
+        LOG(ERROR) << "parent WaitForMessage() failed: " << MachErrorCode(err);
+        return pid;
+      }
+
+      if (child_message.GetTranslatedPort(0) == MACH_PORT_NULL) {
+        LOG(ERROR) << "parent GetTranslatedPort(0) failed.";
+        return pid;
+      }
+      *child_task = child_message.GetTranslatedPort(0);
+
+      if (child_message.GetTranslatedPort(1) == MACH_PORT_NULL) {
+        LOG(ERROR) << "parent GetTranslatedPort(1) failed.";
+        return pid;
+      }
+      MachPortSender parent_sender(child_message.GetTranslatedPort(1));
+
+      MachSendMessage parent_message(/* id= */0);
+      if (!parent_message.AddDescriptor(bootstrap_port)) {
+        LOG(ERROR) << "parent AddDescriptor(" << bootstrap_port << ") failed.";
+        return pid;
+      }
+
+      err = parent_sender.SendMessage(parent_message, kTimeoutMs);
+      if (err != KERN_SUCCESS) {
+        LOG(ERROR) << "parent SendMessage() failed: " << MachErrorCode(err);
+        return pid;
+      }
+      break;
+    }
+  }
+  return pid;
+}
+
 bool LaunchApp(const std::vector<std::string>& argv,
                const environment_vector& environ,
                const file_handle_mapping_vector& fds_to_remap,
                bool wait, ProcessHandle* process_handle) {
-  pid_t pid = fork();
+  return LaunchAppAndGetTask(
+      argv, environ, fds_to_remap, wait, NULL, process_handle);
+}
+#endif  // defined(OS_MACOSX)
+
+#if defined(OS_MACOSX)
+bool LaunchAppAndGetTask(
+#else
+bool LaunchApp(
+#endif
+    const std::vector<std::string>& argv,
+    const environment_vector& environ,
+    const file_handle_mapping_vector& fds_to_remap,
+    bool wait,
+#if defined(OS_MACOSX)
+    task_t* task_handle,
+#endif
+    ProcessHandle* process_handle) {
+  pid_t pid;
+#if defined(OS_MACOSX)
+  if (task_handle == NULL) {
+    pid = fork();
+  } else {
+    // On OS X, the task_t for a process is needed for several reasons. Sadly,
+    // the function task_for_pid() requires privileges a normal user doesn't
+    // have. Instead, a short-lived Mach IPC connection is opened between parent
+    // and child, and the child sends its task_t to the parent at fork time.
+    *task_handle = MACH_PORT_NULL;
+    pid = fork_and_get_task(task_handle);
+  }
+#else
+  pid = fork();
+#endif
   if (pid < 0)
     return false;
 
diff --git a/chrome/browser/child_process_launcher.cc b/chrome/browser/child_process_launcher.cc
index 1ab66c6..91bc2e6 100644
--- a/chrome/browser/child_process_launcher.cc
+++ b/chrome/browser/child_process_launcher.cc
@@ -23,6 +23,10 @@
 #include "chrome/browser/renderer_host/render_sandbox_host_linux.h"
 #endif
 
+#if defined(OS_MACOSX)
+#include "chrome/browser/mach_broker_mac.h"
+#endif
+
 #if defined(OS_POSIX)
 #include "base/global_descriptors_posix.h"
 #endif
@@ -161,10 +165,24 @@ class ChildProcessLauncher::Context
 #endif  // defined(OS_LINUX)
 
       // Actually launch the app.
-      if (!base::LaunchApp(cmd_line->argv(), env, fds_to_map, false, &handle))
+      bool launched;
+#if defined(OS_MACOSX)
+      task_t child_task;
+      launched = base::LaunchAppAndGetTask(
+          cmd_line->argv(), env, fds_to_map, false, &child_task, &handle);
+      if (launched && child_task != MACH_PORT_NULL) {
+          MachBroker::instance()->RegisterPid(
+              handle,
+              MachBroker::MachInfo().SetTask(child_task));
+      }
+#else
+      launched = base::LaunchApp(cmd_line->argv(), env, fds_to_map,
+                                 /* wait= */false, &handle);
+#endif
+      if (!launched)
         handle = base::kNullProcessHandle;
     }
-#endif
+#endif  // else defined(OS_POSIX)
 
     ChromeThread::PostTask(
         client_thread_id_, FROM_HERE,
diff --git a/chrome/browser/mach_broker_mac.cc b/chrome/browser/mach_broker_mac.cc
index 59f7728..6ae3391 100644
--- a/chrome/browser/mach_broker_mac.cc
+++ b/chrome/browser/mach_broker_mac.cc
@@ -5,6 +5,44 @@
 #include "chrome/browser/mach_broker_mac.h"
 
 #include "base/logging.h"
+#include "chrome/browser/extensions/extension_host.h"
+#include "chrome/browser/renderer_host/render_process_host.h"
+#include "chrome/common/child_process_info.h"
+#include "chrome/common/notification_service.h"
+
+// Required because notifications happen on the UI thread.
+class RegisterNotificationTask : public Task {
+ public:
+  RegisterNotificationTask(
+      MachBroker* broker)
+      : broker_(broker) { }
+
+  virtual void Run() {
+    broker_->registrar_.Add(broker_,
+        NotificationType::RENDERER_PROCESS_CLOSED,
+        NotificationService::AllSources());
+    broker_->registrar_.Add(broker_,
+        NotificationType::RENDERER_PROCESS_TERMINATED,
+        NotificationService::AllSources());
+    broker_->registrar_.Add(broker_,
+        NotificationType::CHILD_PROCESS_CRASHED,
+        NotificationService::AllSources());
+    broker_->registrar_.Add(broker_,
+        NotificationType::CHILD_PROCESS_HOST_DISCONNECTED,
+        NotificationService::AllSources());
+    broker_->registrar_.Add(broker_,
+        NotificationType::EXTENSION_PROCESS_TERMINATED,
+        NotificationService::AllSources());
+  }
+
+ private:
+  MachBroker* broker_;
+};
+
+MachBroker::MachBroker() {
+  ChromeThread::PostTask(
+      ChromeThread::UI, FROM_HERE, new RegisterNotificationTask(this));
+}
 
 // Returns the global MachBroker.
 MachBroker* MachBroker::instance() {
@@ -22,7 +60,16 @@ void MachBroker::RegisterPid(
 // Removes all mappings belonging to |pid| from the broker.
 void MachBroker::Invalidate(base::ProcessHandle pid) {
   AutoLock lock(lock_);
-  mach_map_.erase(pid);
+  MachBroker::MachMap::iterator it = mach_map_.find(pid);
+  if (it == mach_map_.end())
+    return;
+
+  kern_return_t kr = mach_port_deallocate(mach_task_self(),
+                                          it->second.mach_task_);
+  LOG_IF(WARNING, kr != KERN_SUCCESS)
+     << "Failed to mach_port_deallocate mach task " << it->second.mach_task_
+     << ", error " << kr;
+  mach_map_.erase(it);
 }
 
 // Returns the mach task belonging to |pid|.
@@ -33,3 +80,27 @@ mach_port_t MachBroker::TaskForPid(base::ProcessHandle pid) const {
     return MACH_PORT_NULL;
   return it->second.mach_task_;
 }
+
+void MachBroker::Observe(NotificationType type,
+                         const NotificationSource& source,
+                         const NotificationDetails& details) {
+  base::ProcessHandle handle = 0;
+  switch (type.value) {
+    case NotificationType::RENDERER_PROCESS_CLOSED:
+    case NotificationType::RENDERER_PROCESS_TERMINATED:
+      handle = Source<RenderProcessHost>(source)->GetHandle();
+      break;
+    case NotificationType::EXTENSION_PROCESS_TERMINATED:
+      handle =
+          Details<ExtensionHost>(details)->render_process_host()->GetHandle();
+      break;
+    case NotificationType::CHILD_PROCESS_CRASHED:
+    case NotificationType::CHILD_PROCESS_HOST_DISCONNECTED:
+      handle = Details<ChildProcessInfo>(details)->handle();
+      break;
+    default:
+      NOTREACHED() << "Unexpected notification";
+      break;
+  }
+  Invalidate(handle);
+}
diff --git a/chrome/browser/mach_broker_mac.h b/chrome/browser/mach_broker_mac.h
index 67f4fec..1827fc9 100644
--- a/chrome/browser/mach_broker_mac.h
+++ b/chrome/browser/mach_broker_mac.h
@@ -13,6 +13,7 @@
 #include "base/process.h"
 #include "base/process_util.h"
 #include "base/singleton.h"
+#include "chrome/common/notification_registrar.h"
 
 // On OS X, the mach_port_t of a process is required to collect metrics about
 // the process. Running |task_for_pid()| is only allowed for privileged code.
@@ -28,7 +29,8 @@
 //
 // Since this data arrives over a separate channel, it is not available
 // immediately after a child process has been started.
-class MachBroker : public base::ProcessMetrics::PortProvider {
+class MachBroker : public base::ProcessMetrics::PortProvider,
+                   public NotificationObserver {
  public:
   // Returns the global MachBroker.
   static MachBroker* instance();
@@ -53,9 +55,18 @@ class MachBroker : public base::ProcessMetrics::PortProvider {
   // Implement |ProcessMetrics::PortProvider|.
   virtual mach_port_t TaskForPid(base::ProcessHandle process) const;
 
+  // Implement |NotificationObserver|.
+  virtual void Observe(NotificationType type,
+                       const NotificationSource& source,
+                       const NotificationDetails& details);
  private:
   // Private constructor.
-  MachBroker() {}
+  MachBroker();
+  
+  // Used to register for notifications received by NotificationObserver.
+  // Accessed only on the UI thread.
+  NotificationRegistrar registrar_;
+  
   friend struct DefaultSingletonTraits<MachBroker>;
   friend class MachBrokerTest;
 
@@ -66,6 +77,7 @@ class MachBroker : public base::ProcessMetrics::PortProvider {
   // Mutex that guards |mach_map_|.
   mutable Lock lock_;
 
+  friend class RegisterNotificationTask;
   DISALLOW_COPY_AND_ASSIGN(MachBroker);
 };