Refactor GetCrashSignalFD to be more generic.

Enables us to pass more file descriptors down to the child process. This
will be used by the Android port.

As part of this, I cleaned up a
ChildProcessLauncher::Context::LaunchInternal to be a little more
generic. This can result in a little extra work but cleans up the
zygote/non-zygote code paths.


Review URL: https://chromiumcodereview.appspot.com/10584007

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

6 files changed, 68 insertions, 56 deletions

diff --git a/base/global_descriptors_posix.h b/base/global_descriptors_posix.h
index 452afa7..988809e 100644
--- a/base/global_descriptors_posix.h
+++ b/base/global_descriptors_posix.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -37,7 +37,8 @@ namespace base {
 class BASE_EXPORT GlobalDescriptors {
  public:
   typedef uint32_t Key;
-  typedef std::vector<std::pair<Key, int> > Mapping;
+  typedef std::pair<Key, int> KeyFDPair;
+  typedef std::vector<KeyFDPair> Mapping;
 
   // Often we want a canonical descriptor for a given Key. In this case, we add
   // the following constant to the key value:
diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc
index bd406b2..0d6fd3a 100644
--- a/chrome/browser/chrome_content_browser_client.cc
+++ b/chrome/browser/chrome_content_browser_client.cc
@@ -93,6 +93,7 @@
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_view.h"
 #include "content/public/common/child_process_host.h"
+#include "content/public/common/content_descriptors.h"
 #include "grit/generated_resources.h"
 #include "grit/ui_resources.h"
 #include "net/base/ssl_cert_request_info.h"
@@ -292,6 +293,33 @@ void FillFontFamilyMap(const PrefService* prefs,
   }
 }
 
+#if defined(OS_POSIX) && !defined(OS_MACOSX)
+int GetCrashSignalFD(const CommandLine& command_line) {
+  if (command_line.HasSwitch(switches::kExtensionProcess)) {
+    ExtensionCrashHandlerHostLinux* crash_handler =
+        ExtensionCrashHandlerHostLinux::GetInstance();
+    return crash_handler->GetDeathSignalSocket();
+  }
+
+  std::string process_type =
+      command_line.GetSwitchValueASCII(switches::kProcessType);
+
+  if (process_type == switches::kRendererProcess)
+    return RendererCrashHandlerHostLinux::GetInstance()->GetDeathSignalSocket();
+
+  if (process_type == switches::kPluginProcess)
+    return PluginCrashHandlerHostLinux::GetInstance()->GetDeathSignalSocket();
+
+  if (process_type == switches::kPpapiPluginProcess)
+    return PpapiCrashHandlerHostLinux::GetInstance()->GetDeathSignalSocket();
+
+  if (process_type == switches::kGpuProcess)
+    return GpuCrashHandlerHostLinux::GetInstance()->GetDeathSignalSocket();
+
+  return -1;
+}
+#endif  // defined(OS_POSIX) && !defined(OS_MACOSX)
+
 }  // namespace
 
 namespace chrome {
@@ -1554,30 +1582,14 @@ bool ChromeContentBrowserClient::AllowPepperPrivateFileAPI() {
 }
 
 #if defined(OS_POSIX) && !defined(OS_MACOSX)
-int ChromeContentBrowserClient::GetCrashSignalFD(
-    const CommandLine& command_line) {
-  if (command_line.HasSwitch(switches::kExtensionProcess)) {
-    ExtensionCrashHandlerHostLinux* crash_handler =
-        ExtensionCrashHandlerHostLinux::GetInstance();
-    return crash_handler->GetDeathSignalSocket();
+void ChromeContentBrowserClient::GetAdditionalMappedFilesForChildProcess(
+    const CommandLine& command_line,
+    base::GlobalDescriptors::Mapping* mappings) {
+  int crash_signal_fd = GetCrashSignalFD(command_line);
+  if (crash_signal_fd >= 0) {
+    mappings->push_back(std::pair<base::GlobalDescriptors::Key, int>(
+        kCrashDumpSignal, crash_signal_fd));
   }
-
-  std::string process_type =
-      command_line.GetSwitchValueASCII(switches::kProcessType);
-
-  if (process_type == switches::kRendererProcess)
-    return RendererCrashHandlerHostLinux::GetInstance()->GetDeathSignalSocket();
-
-  if (process_type == switches::kPluginProcess)
-    return PluginCrashHandlerHostLinux::GetInstance()->GetDeathSignalSocket();
-
-  if (process_type == switches::kPpapiPluginProcess)
-    return PpapiCrashHandlerHostLinux::GetInstance()->GetDeathSignalSocket();
-
-  if (process_type == switches::kGpuProcess)
-    return GpuCrashHandlerHostLinux::GetInstance()->GetDeathSignalSocket();
-
-  return -1;
 }
 #endif  // defined(OS_POSIX) && !defined(OS_MACOSX)
 
diff --git a/chrome/browser/chrome_content_browser_client.h b/chrome/browser/chrome_content_browser_client.h
index d411a5b..d72bee65 100644
--- a/chrome/browser/chrome_content_browser_client.h
+++ b/chrome/browser/chrome_content_browser_client.h
@@ -180,7 +180,9 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
   virtual bool AllowPepperPrivateFileAPI() OVERRIDE;
 
 #if defined(OS_POSIX) && !defined(OS_MACOSX)
-  virtual int GetCrashSignalFD(const CommandLine& command_line) OVERRIDE;
+  virtual void GetAdditionalMappedFilesForChildProcess(
+      const CommandLine& command_line,
+      base::GlobalDescriptors::Mapping* mappings) OVERRIDE;
 #endif
 #if defined(OS_WIN)
   virtual const wchar_t* GetResourceDllName() OVERRIDE;
diff --git a/content/browser/child_process_launcher.cc b/content/browser/child_process_launcher.cc
index fe6c710..094ab1b 100644
--- a/content/browser/child_process_launcher.cc
+++ b/content/browser/child_process_launcher.cc
@@ -132,37 +132,33 @@ class ChildProcessLauncher::Context
     // to reliably detect child termination.
     file_util::ScopedFD ipcfd_closer(&ipcfd);
 
-#if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_ANDROID)
-    // On Linux, we need to add some extra file descriptors for crash handling.
     std::string process_type =
         cmd_line->GetSwitchValueASCII(switches::kProcessType);
-    int crash_signal_fd =
-        content::GetContentClient()->browser()->GetCrashSignalFD(*cmd_line);
+    base::GlobalDescriptors::Mapping files_to_register;
+    files_to_register.push_back(std::pair<base::GlobalDescriptors::Key, int>(
+        kPrimaryIPCChannel, ipcfd));
+#if !defined(OS_MACOSX) && !defined(OS_ANDROID)
+    content::GetContentClient()->browser()->
+        GetAdditionalMappedFilesForChildProcess(*cmd_line, &files_to_register);
     if (use_zygote) {
-      base::GlobalDescriptors::Mapping mapping;
-      mapping.push_back(std::pair<uint32_t, int>(kPrimaryIPCChannel, ipcfd));
-      if (crash_signal_fd >= 0) {
-        mapping.push_back(std::pair<uint32_t, int>(kCrashDumpSignal,
-                                                   crash_signal_fd));
-      }
       handle = ZygoteHostImpl::GetInstance()->ForkRequest(cmd_line->argv(),
-                                                          mapping,
+                                                          files_to_register,
                                                           process_type);
     } else
     // Fall through to the normal posix case below when we're not zygoting.
-#endif
+#endif  // defined(OS_MACOSX) && !defined(OS_ANDROID)
     {
+      // Convert FD mapping to FileHandleMappingVector
       base::FileHandleMappingVector fds_to_map;
-      fds_to_map.push_back(std::make_pair(
-          ipcfd,
-          kPrimaryIPCChannel + base::GlobalDescriptors::kBaseDescriptor));
-
-#if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_ANDROID)
-      if (crash_signal_fd >= 0) {
+      for (size_t i = 0; i < files_to_register.size(); ++i) {
+        const base::GlobalDescriptors::KeyFDPair& id_file =
+            files_to_register[i];
         fds_to_map.push_back(std::make_pair(
-            crash_signal_fd,
-            kCrashDumpSignal + base::GlobalDescriptors::kBaseDescriptor));
+            id_file.second,
+            id_file.first + base::GlobalDescriptors::kBaseDescriptor));
       }
+
+#if !defined(OS_MACOSX) && !defined(OS_ANDROID)
       if (process_type == switches::kRendererProcess) {
         const int sandbox_fd =
             RenderSandboxHostLinux::GetInstance()->GetRendererSocket();
@@ -170,7 +166,7 @@ class ChildProcessLauncher::Context
             sandbox_fd,
             kSandboxIPCChannel + base::GlobalDescriptors::kBaseDescriptor));
       }
-#endif  // defined(OS_POSIX) && !defined(OS_MACOSX)
+#endif  // defined(OS_MACOSX) && !defined(OS_ANDROID)
 
       // Actually launch the app.
       base::LaunchOptions options;
diff --git a/content/public/browser/content_browser_client.cc b/content/public/browser/content_browser_client.cc
index 6331a27..6c28657 100644
--- a/content/public/browser/content_browser_client.cc
+++ b/content/public/browser/content_browser_client.cc
@@ -208,12 +208,6 @@ bool ContentBrowserClient::AllowPepperPrivateFileAPI() {
   return false;
 }
 
-#if defined(OS_POSIX) && !defined(OS_MACOSX)
-int ContentBrowserClient::GetCrashSignalFD(const CommandLine& command_line) {
-  return -1;
-}
-#endif
-
 #if defined(OS_WIN)
 const wchar_t* ContentBrowserClient::GetResourceDllName() {
   return NULL;
diff --git a/content/public/browser/content_browser_client.h b/content/public/browser/content_browser_client.h
index 2b29174..086731c 100644
--- a/content/public/browser/content_browser_client.h
+++ b/content/public/browser/content_browser_client.h
@@ -15,6 +15,11 @@
 #include "content/public/common/window_container_type.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebNotificationPresenter.h"
 
+#if defined(OS_POSIX) && !defined(OS_MACOSX)
+#include "base/global_descriptors_posix.h"
+#endif
+
+
 class CommandLine;
 class FilePath;
 class GURL;
@@ -397,9 +402,11 @@ class CONTENT_EXPORT ContentBrowserClient {
   virtual bool AllowPepperPrivateFileAPI();
 
 #if defined(OS_POSIX) && !defined(OS_MACOSX)
-  // Can return an optional fd for crash handling, otherwise returns -1. The
-  // passed |command_line| will be used to start the process in question.
-  virtual int GetCrashSignalFD(const CommandLine& command_line);
+  // Populates |mappings| with all files that need to be mapped before launching
+  // a child process.
+  virtual void GetAdditionalMappedFilesForChildProcess(
+      const CommandLine& command_line,
+      base::GlobalDescriptors::Mapping* mappings) {}
 #endif
 
 #if defined(OS_WIN)