Linux: Enable metrics_service_uitest.cc. Take 2.

Relands r18641, original code review: http://codereview.chromium.org/125268
Expect a crash on Windows.
The old method of crashing was flawed on posix (KillProcess just does a SIGTERM).  On Windows though, it would terminate the process with the desired exit code, in order to make DidProcessCrash() return true.  This process termination does not dump crash information though, since it just forcibly terminates the process, like a SIGKILL on posix.  When I switched it to navigate to about:crash though, it actually crashes, and dumps crash information, which the UITest in windows (but not linux/mac) detects.  Therefore, until those platforms can detect, we just use #if defined(OS_WIN) around the expected_crashes = 1.

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

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

7 files changed, 102 insertions, 24 deletions

diff --git a/base/process.h b/base/process.h
index 312f84c..459ad58 100644
--- a/base/process.h
+++ b/base/process.h
@@ -6,6 +6,7 @@
 #define BASE_PROCESS_H_
 
 #include "base/basictypes.h"
+#include "build/build_config.h"
 
 #include <sys/types.h>
 #ifdef OS_WIN
diff --git a/chrome/browser/metrics/metrics_service_uitest.cc b/chrome/browser/metrics/metrics_service_uitest.cc
index 7245a35..cf453a5 100644
--- a/chrome/browser/metrics/metrics_service_uitest.cc
+++ b/chrome/browser/metrics/metrics_service_uitest.cc
@@ -90,14 +90,12 @@ TEST_F(MetricsServiceTest, CrashRenderers) {
   // kill the process for one of the tabs
   scoped_refptr<TabProxy> tab(window_->GetTab(1));
   ASSERT_TRUE(tab.get());
-  int process_id = 0;
-  ASSERT_TRUE(tab->GetProcessID(&process_id));
-  ASSERT_NE(0, process_id);
-  base::ProcessHandle process_handle;
-  ASSERT_TRUE(base::OpenProcessHandle(process_id, &process_handle));
-  // Fake Access Violation.
-  base::KillProcess(process_handle, 0xc0000005, true);
-  base::CloseProcessHandle(process_handle);
+
+// Only windows implements the crash service for now.
+#if defined(OS_WIN)
+  expected_crashes_ = 1;
+#endif
+  tab->NavigateToURLAsync(GURL("about:crash"));
 
   // Give the browser a chance to notice the crashed tab.
   PlatformThread::Sleep(1000);
@@ -111,6 +109,6 @@ TEST_F(MetricsServiceTest, CrashRenderers) {
   local_state->RegisterIntegerPref(prefs::kStabilityRendererCrashCount, 0);
   EXPECT_TRUE(local_state->GetBoolean(prefs::kStabilityExitedCleanly));
   EXPECT_EQ(1, local_state->GetInteger(prefs::kStabilityLaunchCount));
-  EXPECT_EQ(3, local_state->GetInteger(prefs::kStabilityPageLoadCount));
+  EXPECT_EQ(4, local_state->GetInteger(prefs::kStabilityPageLoadCount));
   EXPECT_EQ(1, local_state->GetInteger(prefs::kStabilityRendererCrashCount));
 }
diff --git a/chrome/browser/renderer_host/browser_render_process_host.cc b/chrome/browser/renderer_host/browser_render_process_host.cc
index 29e7ef0..06405ed 100644
--- a/chrome/browser/renderer_host/browser_render_process_host.cc
+++ b/chrome/browser/renderer_host/browser_render_process_host.cc
@@ -721,7 +721,18 @@ void BrowserRenderProcessHost::OnChannelError() {
   DCHECK(channel_.get());
 
   bool child_exited;
-  bool did_crash = base::DidProcessCrash(&child_exited, process_.handle());
+  bool did_crash;
+  if (zygote_child_) {
+#if defined(OS_LINUX)
+    did_crash = Singleton<ZygoteHost>()->DidProcessCrash(
+        process_.handle(), &child_exited);
+#else
+    NOTREACHED();
+    did_crash = true;
+#endif
+  } else {
+    did_crash = base::DidProcessCrash(&child_exited, process_.handle());
+  }
 
   NotificationService::current()->Notify(
       NotificationType::RENDERER_PROCESS_CLOSED,
diff --git a/chrome/browser/zygote_host_linux.cc b/chrome/browser/zygote_host_linux.cc
index 1c582a3..230373c 100644
--- a/chrome/browser/zygote_host_linux.cc
+++ b/chrome/browser/zygote_host_linux.cc
@@ -94,3 +94,38 @@ void ZygoteHost::EnsureProcessTerminated(pid_t process) {
 
   HANDLE_EINTR(write(control_fd_, pickle.data(), pickle.size()));
 }
+
+bool ZygoteHost::DidProcessCrash(base::ProcessHandle handle,
+                                 bool* child_exited) {
+  Pickle pickle;
+  pickle.WriteInt(kCmdDidProcessCrash);
+  pickle.WriteInt(handle);
+
+  HANDLE_EINTR(write(control_fd_, pickle.data(), pickle.size()));
+
+  static const unsigned kMaxMessageLength = 128;
+  char buf[kMaxMessageLength];
+  const ssize_t len = HANDLE_EINTR(read(control_fd_, buf, sizeof(buf)));
+
+  if (len == -1) {
+    LOG(WARNING) << "Error reading message from zygote: " << errno;
+    return false;
+  } else if (len == 0) {
+    LOG(WARNING) << "Socket closed prematurely.";
+    return false;
+  }
+
+  Pickle read_pickle(buf, len);
+  bool did_crash, tmp_child_exited;
+  void* iter = NULL;
+  if (!read_pickle.ReadBool(&iter, &did_crash) ||
+      !read_pickle.ReadBool(&iter, &tmp_child_exited)) {
+    LOG(WARNING) << "Error parsing DidProcessCrash response from zygote.";
+    return false;
+  }
+
+  if (child_exited)
+    *child_exited = tmp_child_exited;
+
+  return did_crash;
+}
diff --git a/chrome/browser/zygote_host_linux.h b/chrome/browser/zygote_host_linux.h
index 279918d..94ac92e 100644
--- a/chrome/browser/zygote_host_linux.h
+++ b/chrome/browser/zygote_host_linux.h
@@ -9,7 +9,10 @@
 #include <vector>
 
 #include "base/global_descriptors_posix.h"
-#include "base/singleton.h"
+#include "base/process.h"
+
+template<typename Type>
+struct DefaultSingletonTraits;
 
 // http://code.google.com/p/chromium/wiki/LinuxZygote
 
@@ -23,11 +26,17 @@ class ZygoteHost {
                      const base::GlobalDescriptors::Mapping& mapping);
   void EnsureProcessTerminated(pid_t process);
 
+  // Get the termination status (exit code) of the process and return true if
+  // the status indicates the process crashed. |child_exited| is set to true
+  // iff the child process has terminated. (|child_exited| may be NULL.)
+  bool DidProcessCrash(base::ProcessHandle handle, bool* child_exited);
+
   // These are the command codes used on the wire between the browser and the
   // zygote.
   enum {
-    kCmdFork = 0,  // Fork off a new renderer.
-    kCmdReap = 1,  // Reap a renderer child.
+    kCmdFork = 0,             // Fork off a new renderer.
+    kCmdReap = 1,             // Reap a renderer child.
+    kCmdDidProcessCrash = 2,  // Check if child process crashed.
   };
 
  private:
diff --git a/chrome/browser/zygote_main_linux.cc b/chrome/browser/zygote_main_linux.cc
index 6d15c0f..ba550d9 100644
--- a/chrome/browser/zygote_main_linux.cc
+++ b/chrome/browser/zygote_main_linux.cc
@@ -77,18 +77,24 @@ class Zygote {
     void* iter = NULL;
 
     int kind;
-    if (!pickle.ReadInt(&iter, &kind))
-      goto error;
-
-    if (kind == ZygoteHost::kCmdFork) {
-      return HandleForkRequest(fd, pickle, iter, fds);
-    } else if (kind == ZygoteHost::kCmdReap) {
-      if (fds.size())
-        goto error;
-      return HandleReapRequest(fd, pickle, iter);
+    if (pickle.ReadInt(&iter, &kind)) {
+      switch (kind) {
+        case ZygoteHost::kCmdFork:
+          return HandleForkRequest(fd, pickle, iter, fds);
+        case ZygoteHost::kCmdReap:
+          if (!fds.empty())
+            break;
+          return HandleReapRequest(fd, pickle, iter);
+        case ZygoteHost::kCmdDidProcessCrash:
+          if (!fds.empty())
+            break;
+          return HandleDidProcessCrash(fd, pickle, iter);
+        default:
+          NOTREACHED();
+          break;
+      }
     }
 
-   error:
     LOG(WARNING) << "Error parsing message from browser";
     for (std::vector<int>::const_iterator
          i = fds.begin(); i != fds.end(); ++i)
@@ -109,6 +115,25 @@ class Zygote {
     return false;
   }
 
+  bool HandleDidProcessCrash(int fd, Pickle& pickle, void* iter) {
+    base::ProcessHandle child;
+
+    if (!pickle.ReadInt(&iter, &child)) {
+      LOG(WARNING) << "Error parsing DidProcessCrash request from browser";
+      return false;
+    }
+
+    bool child_exited;
+    bool did_crash = base::DidProcessCrash(&child_exited, child);
+
+    Pickle write_pickle;
+    write_pickle.WriteBool(did_crash);
+    write_pickle.WriteBool(child_exited);
+    HANDLE_EINTR(write(fd, write_pickle.data(), write_pickle.size()));
+
+    return false;
+  }
+
   // Handle a 'fork' request from the browser: this means that the browser
   // wishes to start a new renderer.
   bool HandleForkRequest(int fd, Pickle& pickle, void* iter,
diff --git a/chrome/chrome.gyp b/chrome/chrome.gyp
index 7d198af..204f5f1 100644
--- a/chrome/chrome.gyp
+++ b/chrome/chrome.gyp
@@ -3200,7 +3200,6 @@
             # TODO(port)
             'browser/crash_recovery_uitest.cc',
             'browser/login_prompt_uitest.cc',
-            'browser/metrics/metrics_service_uitest.cc',
             'browser/renderer_host/resource_dispatcher_host_uitest.cc',
             'test/reliability/page_load_test.cc',
             'test/ui/layout_plugin_uitest.cc',