Make SandboxIPCProcess a thread.

This is a reland of https://chromiumcodereview.appspot.com/255693002/

Race conditions in Skia have been fixed.

BUG=322185

Review URL: https://codereview.chromium.org/286903021

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

chrome/browser/memory_details.cc

@@ -204,8 +204,6 @@ void MemoryDetails::CollectChildInfoOnUIThread() {
 
 #if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_ANDROID)
   const pid_t zygote_pid = content::ZygoteHost::GetInstance()->GetPid();
-  const pid_t sandbox_helper_pid =
-      content::ZygoteHost::GetInstance()->GetSandboxHelperPid();
 #endif
 
   ProcessData* const chrome_browser = ChromeBrowser();
@@ -339,8 +337,6 @@ void MemoryDetails::CollectChildInfoOnUIThread() {
 #if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_ANDROID)
     if (process.pid == zygote_pid) {
       process.process_type = content::PROCESS_TYPE_ZYGOTE;
-    } else if (process.pid == sandbox_helper_pid) {
-      process.process_type = content::PROCESS_TYPE_SANDBOX_HELPER;
     }
 #endif
   }

content/browser/renderer_host/render_sandbox_host_linux.cc

@@ -8,16 +8,12 @@
 
 #include "base/memory/singleton.h"
 #include "base/posix/eintr_wrapper.h"
-#include "content/browser/renderer_host/sandbox_ipc_linux.h"
 
 namespace content {
 
 // Runs on the main thread at startup.
 RenderSandboxHostLinux::RenderSandboxHostLinux()
-    : initialized_(false),
-      renderer_socket_(0),
-      childs_lifeline_fd_(0),
-      pid_(0) {
+    : initialized_(false), renderer_socket_(0), childs_lifeline_fd_(0) {
 }
 
 // static
@@ -53,21 +49,11 @@ void RenderSandboxHostLinux::Init() {
   const int child_lifeline_fd = pipefds[0];
   childs_lifeline_fd_ = pipefds[1];
 
-  // We need to be monothreaded before we fork().
-#if !defined(THREAD_SANITIZER)
-  DCHECK_EQ(1, base::GetNumberOfThreads(base::GetCurrentProcessHandle()));
-#endif  // !defined(THREAD_SANITIZER)
-  pid_ = fork();
-  if (pid_ == 0) {
-    if (IGNORE_EINTR(close(fds[0])) < 0)
-      DPLOG(ERROR) << "close";
-    if (IGNORE_EINTR(close(pipefds[1])) < 0)
-      DPLOG(ERROR) << "close";
-
-    SandboxIPCProcess handler(child_lifeline_fd, browser_socket);
-    handler.Run();
-    _exit(0);
-  }
+  ipc_handler_.reset(
+      new SandboxIPCHandler(child_lifeline_fd, browser_socket));
+  ipc_thread_.reset(
+      new base::DelegateSimpleThread(ipc_handler_.get(), "sandbox_ipc_thread"));
+  ipc_thread_->Start();
 }
 
 bool RenderSandboxHostLinux::ShutdownIPCChannel() {
@@ -80,6 +66,8 @@ RenderSandboxHostLinux::~RenderSandboxHostLinux() {
       LOG(ERROR) << "ShutdownIPCChannel failed";
     if (IGNORE_EINTR(close(renderer_socket_)) < 0)
       PLOG(ERROR) << "close";
+
+    ipc_thread_->Join();
   }
 }
 

content/browser/renderer_host/render_sandbox_host_linux.h

@@ -8,6 +8,9 @@
 #include <string>
 
 #include "base/logging.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/threading/simple_thread.h"
+#include "content/browser/renderer_host/sandbox_ipc_linux.h"
 #include "content/common/content_export.h"
 
 template <typename T> struct DefaultSingletonTraits;
@@ -27,10 +30,6 @@ class CONTENT_EXPORT RenderSandboxHostLinux {
     DCHECK(initialized_);
     return renderer_socket_;
   }
-  pid_t pid() const {
-    DCHECK(initialized_);
-    return pid_;
-  }
   void Init();
 
  private:
@@ -46,7 +45,9 @@ class CONTENT_EXPORT RenderSandboxHostLinux {
 
   int renderer_socket_;
   int childs_lifeline_fd_;
-  pid_t pid_;
+
+  scoped_ptr<SandboxIPCHandler> ipc_handler_;
+  scoped_ptr<base::DelegateSimpleThread> ipc_thread_;
 
   DISALLOW_COPY_AND_ASSIGN(RenderSandboxHostLinux);
 };

content/browser/renderer_host/sandbox_ipc_linux.cc

@@ -129,24 +129,15 @@ static void MSCharSetToFontconfig(FcLangSet* langset, unsigned fdwCharSet) {
 
 namespace content {
 
-SandboxIPCProcess::SandboxIPCProcess(int lifeline_fd, int browser_socket)
+SandboxIPCHandler::SandboxIPCHandler(int lifeline_fd, int browser_socket)
     : lifeline_fd_(lifeline_fd), browser_socket_(browser_socket) {
   // FontConfig doesn't provide a standard property to control subpixel
   // positioning, so we pass the current setting through to WebKit.
   WebFontInfo::setSubpixelPositioning(
       gfx::GetDefaultWebkitSubpixelPositioning());
-
-  CommandLine& command_line = *CommandLine::ForCurrentProcess();
-  command_line.AppendSwitchASCII(switches::kProcessType,
-                                 switches::kSandboxIPCProcess);
-
-  // Update the process title. The argv was already cached by the call to
-  // SetProcessTitleFromCommandLine in content_main_runner.cc, so we can pass
-  // NULL here (we don't have the original argv at this point).
-  SetProcessTitleFromCommandLine(NULL);
 }
 
-void SandboxIPCProcess::Run() {
+void SandboxIPCHandler::Run() {
   struct pollfd pfds[2];
   pfds[0].fd = lifeline_fd_;
   pfds[0].events = POLLIN;
@@ -155,13 +146,14 @@ void SandboxIPCProcess::Run() {
 
   int failed_polls = 0;
   for (;;) {
-    const int r = HANDLE_EINTR(poll(pfds, 2, -1 /* no timeout */));
+    const int r =
+        HANDLE_EINTR(poll(pfds, arraysize(pfds), -1 /* no timeout */));
     // '0' is not a possible return value with no timeout.
     DCHECK_NE(0, r);
     if (r < 0) {
       PLOG(WARNING) << "poll";
       if (failed_polls++ == 3) {
-        LOG(FATAL) << "poll(2) failing. RenderSandboxHostLinux aborting.";
+        LOG(FATAL) << "poll(2) failing. SandboxIPCHandler aborting.";
         return;
       }
       continue;
@@ -175,15 +167,21 @@ void SandboxIPCProcess::Run() {
       break;
     }
 
-    if (pfds[1].revents) {
+    // If poll(2) reports an error condition in this fd,
+    // we assume the zygote is gone and we exit the loop.
+    if (pfds[1].revents & (POLLERR | POLLHUP)) {
+      break;
+    }
+
+    if (pfds[1].revents & POLLIN) {
       HandleRequestFromRenderer(browser_socket_);
     }
   }
 
-  VLOG(1) << "SandboxIPCProcess stopping.";
+  VLOG(1) << "SandboxIPCHandler stopping.";
 }
 
-void SandboxIPCProcess::HandleRequestFromRenderer(int fd) {
+void SandboxIPCHandler::HandleRequestFromRenderer(int fd) {
   ScopedVector<base::ScopedFD> fds;
 
   // A FontConfigIPC::METHOD_MATCH message could be kMaxFontFamilyLength
@@ -225,7 +223,7 @@ void SandboxIPCProcess::HandleRequestFromRenderer(int fd) {
   }
 }
 
-int SandboxIPCProcess::FindOrAddPath(const SkString& path) {
+int SandboxIPCHandler::FindOrAddPath(const SkString& path) {
   int count = paths_.count();
   for (int i = 0; i < count; ++i) {
     if (path == *paths_[i])
@@ -235,7 +233,7 @@ int SandboxIPCProcess::FindOrAddPath(const SkString& path) {
   return count;
 }
 
-void SandboxIPCProcess::HandleFontMatchRequest(
+void SandboxIPCHandler::HandleFontMatchRequest(
     int fd,
     const Pickle& pickle,
     PickleIterator iter,
@@ -275,7 +273,7 @@ void SandboxIPCProcess::HandleFontMatchRequest(
   SendRendererReply(fds, reply, -1);
 }
 
-void SandboxIPCProcess::HandleFontOpenRequest(
+void SandboxIPCHandler::HandleFontOpenRequest(
     int fd,
     const Pickle& pickle,
     PickleIterator iter,
@@ -304,13 +302,13 @@ void SandboxIPCProcess::HandleFontOpenRequest(
   }
 }
 
-void SandboxIPCProcess::HandleGetFontFamilyForChar(
+void SandboxIPCHandler::HandleGetFontFamilyForChar(
     int fd,
     const Pickle& pickle,
     PickleIterator iter,
     const std::vector<base::ScopedFD*>& fds) {
   // The other side of this call is
-  // chrome/renderer/renderer_sandbox_support_linux.cc
+  // content/common/child_process_sandbox_support_impl_linux.cc
 
   EnsureWebKitInitialized();
   WebUChar32 c;
@@ -335,7 +333,7 @@ void SandboxIPCProcess::HandleGetFontFamilyForChar(
   SendRendererReply(fds, reply, -1);
 }
 
-void SandboxIPCProcess::HandleGetStyleForStrike(
+void SandboxIPCHandler::HandleGetStyleForStrike(
     int fd,
     const Pickle& pickle,
     PickleIterator iter,
@@ -364,7 +362,7 @@ void SandboxIPCProcess::HandleGetStyleForStrike(
   SendRendererReply(fds, reply, -1);
 }
 
-void SandboxIPCProcess::HandleLocaltime(
+void SandboxIPCHandler::HandleLocaltime(
     int fd,
     const Pickle& pickle,
     PickleIterator iter,
@@ -397,7 +395,7 @@ void SandboxIPCProcess::HandleLocaltime(
   SendRendererReply(fds, reply, -1);
 }
 
-void SandboxIPCProcess::HandleMakeSharedMemorySegment(
+void SandboxIPCHandler::HandleMakeSharedMemorySegment(
     int fd,
     const Pickle& pickle,
     PickleIterator iter,
@@ -417,7 +415,7 @@ void SandboxIPCProcess::HandleMakeSharedMemorySegment(
   SendRendererReply(fds, reply, shm_fd);
 }
 
-void SandboxIPCProcess::HandleMatchWithFallback(
+void SandboxIPCHandler::HandleMatchWithFallback(
     int fd,
     const Pickle& pickle,
     PickleIterator iter,
@@ -577,7 +575,7 @@ void SandboxIPCProcess::HandleMatchWithFallback(
   }
 }
 
-void SandboxIPCProcess::SendRendererReply(
+void SandboxIPCHandler::SendRendererReply(
     const std::vector<base::ScopedFD*>& fds,
     const Pickle& reply,
     int reply_fd) {
@@ -613,13 +611,18 @@ void SandboxIPCProcess::SendRendererReply(
     PLOG(ERROR) << "sendmsg";
 }
 
-SandboxIPCProcess::~SandboxIPCProcess() {
+SandboxIPCHandler::~SandboxIPCHandler() {
   paths_.deleteAll();
   if (webkit_platform_support_)
     blink::shutdownWithoutV8();
+
+  if (IGNORE_EINTR(close(lifeline_fd_)) < 0)
+    PLOG(ERROR) << "close";
+  if (IGNORE_EINTR(close(browser_socket_)) < 0)
+    PLOG(ERROR) << "close";
 }
 
-void SandboxIPCProcess::EnsureWebKitInitialized() {
+void SandboxIPCHandler::EnsureWebKitInitialized() {
   if (webkit_platform_support_)
     return;
   webkit_platform_support_.reset(new BlinkPlatformImpl);

content/browser/renderer_host/sandbox_ipc_linux.h

@@ -12,24 +12,21 @@
 #include "base/files/scoped_file.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/pickle.h"
+#include "base/threading/simple_thread.h"
 #include "content/child/blink_platform_impl.h"
 #include "skia/ext/skia_utils_base.h"
 
 namespace content {
 
-class SandboxIPCProcess {
+class SandboxIPCHandler : public base::DelegateSimpleThread::Delegate {
  public:
-  // lifeline_fd: this is the read end of a pipe which the browser process
-  //   holds the other end of. If the browser process dies, its descriptors are
-  //   closed and we will noticed an EOF on the pipe. That's our signal to exit.
-  // browser_socket: the browser's end of the sandbox IPC socketpair. From the
-  //   point of view of the renderer, it's talking to the browser but this
-  //   object actually services the requests.
-  // sandbox_cmd: the path of the sandbox executable.
-  SandboxIPCProcess(int lifeline_fd, int browser_socket);
-  ~SandboxIPCProcess();
-
-  void Run();
+  // lifeline_fd: the read end of a pipe which the main thread holds
+  // the other end of.
+  // browser_socket: the browser's end of the sandbox IPC socketpair.
+  SandboxIPCHandler(int lifeline_fd, int browser_socket);
+  virtual ~SandboxIPCHandler();
+
+  virtual void Run() OVERRIDE;
 
  private:
   void EnsureWebKitInitialized();
@@ -82,7 +79,7 @@ class SandboxIPCProcess {
   scoped_ptr<BlinkPlatformImpl> webkit_platform_support_;
   SkTDArray<SkString*> paths_;
 
-  DISALLOW_COPY_AND_ASSIGN(SandboxIPCProcess);
+  DISALLOW_COPY_AND_ASSIGN(SandboxIPCHandler);
 };
 
 }  // namespace content

content/browser/zygote_host/zygote_host_impl_linux.cc

@@ -559,10 +559,6 @@ pid_t ZygoteHostImpl::GetPid() const {
   return pid_;
 }
 
-pid_t ZygoteHostImpl::GetSandboxHelperPid() const {
-  return RenderSandboxHostLinux::GetInstance()->pid();
-}
-
 int ZygoteHostImpl::GetSandboxStatus() const {
   if (have_read_sandbox_status_word_)
     return sandbox_status_;

content/browser/zygote_host/zygote_host_impl_linux.h

@@ -55,7 +55,6 @@ class CONTENT_EXPORT ZygoteHostImpl : public ZygoteHost {
 
   // ZygoteHost implementation:
   virtual pid_t GetPid() const OVERRIDE;
-  virtual pid_t GetSandboxHelperPid() const OVERRIDE;
   virtual int GetSandboxStatus() const OVERRIDE;
   virtual void AdjustRendererOOMScore(base::ProcessHandle process_handle,
                                       int score) OVERRIDE;

content/public/browser/zygote_host_linux.h

@@ -26,9 +26,6 @@ class ZygoteHost {
   // Returns the pid of the Zygote process.
   virtual pid_t GetPid() const = 0;
 
-  // Returns the pid of the Sandbox Helper process.
-  virtual pid_t GetSandboxHelperPid() const = 0;
-
   // Returns an int which is a bitmask of kSandboxLinux* values. Only valid
   // after the first render has been forked.
   virtual int GetSandboxStatus() const = 0;