Fix a bug where killing pages doesn't yield a sad-tab

The determination of whether or not to put up a sad tab page is made
based on the termination status of the renderer process.

The termination status of the renderer process is checked when
the channel between the browser and the renderer stops working.

There is a race here because there is no reason to believe that the
renderer process has finished terminating at the time that the
browser detects that its channel to the renderer is dead.

If renderer process is believed to be still running at the moment when
the decision to put up the sad tab page is taken, then no sad tab will
be put up, the view of the renderer will be removed (eventually), and
a transparent window will result.

This bug was previously fixed on Linux with with
https://chromiumcodereview.appspot.com/11316261
this patch expands that fix to Mac.

In particular, if the renderer process is known to be dead, wait for the
process to terminate before taking its termination status. To be sure
that the browser does not wait forever on a renderer that is not exiting,
send a kill signal to the renderer before doing the wait.

BUG=167538

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

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

base/process/kill.h

@@ -67,12 +67,23 @@ BASE_EXPORT TerminationStatus GetTerminationStatus(ProcessHandle handle,
                                                    int* exit_code);
 
 #if defined(OS_POSIX)
-// Wait for the process to exit and get the termination status. See
-// GetTerminationStatus for more information. On POSIX systems, we can't call
-// WaitForExitCode and then GetTerminationStatus as the child will be reaped
-// when WaitForExitCode return and this information will be lost.
-BASE_EXPORT TerminationStatus WaitForTerminationStatus(ProcessHandle handle,
-                                                       int* exit_code);
+// Send a kill signal to the process and then wait for the process to exit
+// and get the termination status.
+//
+// This is used in situations where it is believed that the process is dead
+// or dying (because communication with the child process has been cut).
+// In order to avoid erroneously returning that the process is still running
+// because the kernel is still cleaning it up, this will wait for the process
+// to terminate. In order to avoid the risk of hanging while waiting for the
+// process to terminate, send a SIGKILL to the process before waiting for the
+// termination status.
+//
+// Note that it is not an option to call WaitForExitCode and then
+// GetTerminationStatus as the child will be reaped when WaitForExitCode
+// returns, and this information will be lost.
+//
+BASE_EXPORT TerminationStatus GetKnownDeadTerminationStatus(
+    ProcessHandle handle, int* exit_code);
 #endif  // defined(OS_POSIX)
 
 // Waits for process to exit. On POSIX systems, if the process hasn't been

base/process/kill_posix.cc

@@ -195,8 +195,13 @@ TerminationStatus GetTerminationStatus(ProcessHandle handle, int* exit_code) {
   return GetTerminationStatusImpl(handle, false /* can_block */, exit_code);
 }
 
-TerminationStatus WaitForTerminationStatus(ProcessHandle handle,
-                                           int* exit_code) {
+TerminationStatus GetKnownDeadTerminationStatus(ProcessHandle handle,
+                                                int* exit_code) {
+  bool result = kill(handle, SIGKILL) == 0;
+
+  if (!result)
+    DPLOG(ERROR) << "Unable to terminate process " << handle;
+
   return GetTerminationStatusImpl(handle, true /* can_block */, exit_code);
 }
 

chrome/nacl/nacl_helper_linux.cc

@@ -164,17 +164,10 @@ bool HandleGetTerminationStatusRequest(PickleIterator* input_iter,
 
   int exit_code;
   base::TerminationStatus status;
-  // See the comment in the Zygote about known_dead.
-  if (known_dead) {
-    // Make sure to not perform a blocking wait on something that
-    // could still be alive.
-    if (kill(child_to_wait, SIGKILL)) {
-      PLOG(ERROR) << "kill (" << child_to_wait << ")";
-    }
-    status = base::WaitForTerminationStatus(child_to_wait, &exit_code);
-  } else {
+  if (known_dead)
+    status = base::GetKnownDeadTerminationStatus(child_to_wait, &exit_code);
+  else
     status = base::GetTerminationStatus(child_to_wait, &exit_code);
-  }
   output_pickle->WriteInt(static_cast<int>(status));
   output_pickle->WriteInt(exit_code);
   return true;

content/browser/child_process_launcher.cc

@@ -465,6 +465,11 @@ base::TerminationStatus ChildProcessLauncher::GetChildTerminationStatus(
     context_->termination_status_ = ZygoteHostImpl::GetInstance()->
         GetTerminationStatus(handle, known_dead, &context_->exit_code_);
   } else
+#elif defined(OS_MACOSX)
+  if (known_dead) {
+    context_->termination_status_ =
+        base::GetKnownDeadTerminationStatus(handle, &context_->exit_code_);
+  } else
 #endif
   {
     context_->termination_status_ =

content/zygote/zygote_linux.cc

@@ -223,16 +223,7 @@ bool Zygote::GetTerminationStatus(base::ProcessHandle real_pid,
   } else {
     // Handle the request directly.
     if (known_dead) {
-      // If we know that the process is already dead and the kernel is cleaning
-      // it up, we do want to wait until it becomes a zombie and not risk
-      // returning eroneously that it is still running. However, we do not
-      // want to risk a bug where we're told a process is dead when it's not.
-      // By sending SIGKILL, we make sure that WaitForTerminationStatus will
-      // return quickly even in this case.
-      if (kill(child, SIGKILL)) {
-        PLOG(ERROR) << "kill (" << child << ")";
-      }
-      *status = base::WaitForTerminationStatus(child, exit_code);
+      *status = base::GetKnownDeadTerminationStatus(child, exit_code);
     } else {
       // We don't know if the process is dying, so get its status but don't
       // wait.