Create a ChromeProcessUtil for the Mac, and enable it in the tests.

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

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

9 files changed, 69 insertions, 24 deletions

diff --git a/chrome/browser/browser_uitest.cc b/chrome/browser/browser_uitest.cc
index 99f9ccc..55ed855 100644
--- a/chrome/browser/browser_uitest.cc
+++ b/chrome/browser/browser_uitest.cc
@@ -117,18 +117,12 @@ TEST_F(BrowserTest, ThirtyFourTabs) {
     return;
   // See browser\renderer_host\render_process_host.cc for the algorithm to
   // decide how many processes to create.
-#if defined(OS_WIN) || defined(OS_LINUX)
-// TODO(pinkerton): Turn this back on for Mac when ChromeBrowserProcessId()
-// gets implemented. Right now we don't have a good way to do it, and keeping
-// a file always open just so UI tests can check renderers seems a bit
-// wasteful.
   int process_count = GetBrowserProcessCount();
   if (base::SysInfo::AmountOfPhysicalMemoryMB() >= 2048) {
     EXPECT_GE(process_count, 24);
   } else {
     EXPECT_LE(process_count, 23);
   }
-#endif
 }
 
 #if defined(OS_WIN)
@@ -257,13 +251,7 @@ TEST_F(BrowserTest, OtherRedirectsDontForkProcess) {
   int orig_tab_count = -1;
   ASSERT_TRUE(window->GetTabCount(&orig_tab_count));
   int orig_process_count = GetBrowserProcessCount();
-#if defined(OS_WIN) || defined(OS_LINUX)
-// TODO(pinkerton): Turn this back on for Mac when ChromeBrowserProcessId()
-// gets implemented. Right now we don't have a good way to do it, and keeping
-// a file always open just so UI tests can check renderers seems a bit
-// wasteful.
   ASSERT_GE(orig_process_count, 1);
-#endif
 
   // Use JavaScript URL to almost fork a new tab, but not quite.  (Leave the
   // opener non-null.)  Should not fork a process.
diff --git a/chrome/chrome.gyp b/chrome/chrome.gyp
index 4933f49..72cd66a 100644
--- a/chrome/chrome.gyp
+++ b/chrome/chrome.gyp
@@ -2062,8 +2062,6 @@
             'browser/login_prompt_uitest.cc',
             'browser/metrics/metrics_service_uitest.cc',
             'browser/sessions/session_restore_uitest.cc',
-            # blocked on ChromeBrowserProcessId()
-            'test/chrome_process_util_uitest.cc',
             'test/reliability/page_load_test.cc',
             'test/ui/layout_plugin_uitest.cc',
             'test/ui/omnibox_uitest.cc',
diff --git a/chrome/common/chrome_constants.cc b/chrome/common/chrome_constants.cc
index 94d8edb..2ca8f0c 100644
--- a/chrome/common/chrome_constants.cc
+++ b/chrome/common/chrome_constants.cc
@@ -18,14 +18,24 @@ const wchar_t kBrowserProcessExecutableName[] = L"chrome.exe";
 const wchar_t kBrowserProcessExecutableName[] = L"chrome";
 #elif defined(OS_MACOSX)
 const wchar_t kBrowserProcessExecutableName[] =
-// TODO(thomasvl): Un-hardcode path inside the bundle in case we want to use
-// this constant for something other than test code.
+#if defined(GOOGLE_CHROME_BUILD)
+    L"Chrome";
+#else
+    L"Chromium";
+#endif  // GOOGLE_CHROME_BUILD
+#endif  // OS_*
+#if defined(OS_WIN)
+const wchar_t kBrowserProcessExecutablePath[] = L"chrome.exe";
+#elif defined(OS_LINUX)
+const wchar_t kBrowserProcessExecutablePath[] = L"chrome";
+#elif defined(OS_MACOSX)
+const wchar_t kBrowserProcessExecutablePath[] =
 #if defined(GOOGLE_CHROME_BUILD)
     L"Chrome.app/Contents/MacOS/Chrome";
 #else
     L"Chromium.app/Contents/MacOS/Chromium";
-#endif
-#endif  // defined(MACOSX)
+#endif  // GOOGLE_CHROME_BUILD
+#endif  // OS_*
 #if defined(GOOGLE_CHROME_BUILD)
 const wchar_t kBrowserAppName[] = L"Chrome";
 const char    kStatsFilename[] = "ChromeStats2";
diff --git a/chrome/common/chrome_constants.h b/chrome/common/chrome_constants.h
index 9be6a3f..1a5e7c4 100644
--- a/chrome/common/chrome_constants.h
+++ b/chrome/common/chrome_constants.h
@@ -12,6 +12,7 @@
 namespace chrome {
 
 extern const wchar_t kBrowserProcessExecutableName[];
+extern const wchar_t kBrowserProcessExecutablePath[];
 extern const wchar_t kBrowserAppName[];
 extern const wchar_t kMessageWindowClass[];
 extern const wchar_t kExternalTabWindowClass[];
diff --git a/chrome/test/chrome_process_util_linux.cc b/chrome/test/chrome_process_util_linux.cc
index f1c7601..9bcb0ad 100644
--- a/chrome/test/chrome_process_util_linux.cc
+++ b/chrome/test/chrome_process_util_linux.cc
@@ -37,7 +37,10 @@ base::ProcessId ChromeBrowserProcessId(const FilePath& data_dir) {
 
   int pid;
   if (!StringToInt(trimmed_output, &pid)) {
-    LOG(FATAL) << "Unexpected fuser output: " << fuser_output;
+    // In normal use we'd expect to find what we're looking for, but in the unit
+    // test for chrome_process_util we're explicitly called when there is no
+    // matching process to find. Just return -1 and let our callers realize
+    // something's wrong if they really expect to find a target.
     return -1;
   }
   return pid;
diff --git a/chrome/test/chrome_process_util_mac.cc b/chrome/test/chrome_process_util_mac.cc
index 4d252ba..fa587b9 100644
--- a/chrome/test/chrome_process_util_mac.cc
+++ b/chrome/test/chrome_process_util_mac.cc
@@ -4,9 +4,54 @@
 
 #include "chrome/test/chrome_process_util.h"
 
-#include "base/logging.h"
+#include <string>
+#include <vector>
+
+#include "base/command_line.h"
+#include "base/process_util.h"
+#include "base/string_util.h"
+
+// Yes, this is impossibly lame. This horrible hack is Good Enough, though,
+// because it's not used in production code, but just for testing.
+//
+// We could make this better by creating a system through which all instances of
+// Chromium can communicate. ProcessSingleton does that for Windows and Linux,
+// but the Mac doesn't implement it as its system services handle it. It's not
+// worth implementing just for this.
+//
+// We could do something similar to what Linux does, and use |fuser| to find a
+// file that the app ordinarily opens within the data dir. However, fuser is
+// broken on Leopard, and does not detect files that lsof shows are open.
+//
+// What's going on here is that during ui_tests, the Chromium application is
+// launched using the --user-data-dir command line option. By examining the
+// output of ps, we can find the appropriately-launched Chromium process. Note
+// that this _does_ work for paths with spaces. The command line that ps gives
+// is just the argv separated with spaces. There's no escaping spaces as a shell
+// would do, so a straight string comparison will work just fine.
+//
+// TODO(avi):see if there is a better way
 
 base::ProcessId ChromeBrowserProcessId(const FilePath& data_dir) {
-  NOTIMPLEMENTED();
+  std::vector<std::string> argv;
+  argv.push_back("ps");
+  argv.push_back("-xw");
+
+  std::string ps_output;
+  if (!base::GetAppOutput(CommandLine(argv), &ps_output))
+    return -1;
+
+  std::vector<std::string> lines;
+  SplitString(ps_output, '\n', &lines);
+
+  for (size_t i=0; i<lines.size(); ++i) {
+    const std::string& line = lines[i];
+    if (line.find(data_dir.value()) != std::string::npos &&
+        line.find("type=renderer") == std::string::npos) {
+      int pid = StringToInt(line);  // pid is at beginning of line
+      return pid==0 ? -1 : pid;
+    }
+  }
+
   return -1;
 }
diff --git a/chrome/test/in_process_browser_test.cc b/chrome/test/in_process_browser_test.cc
index 9cac992..1b454a3 100644
--- a/chrome/test/in_process_browser_test.cc
+++ b/chrome/test/in_process_browser_test.cc
@@ -118,7 +118,7 @@ void InProcessBrowserTest::SetUp() {
   FilePath fp_renderer_path = FilePath::FromWStringHack(renderer_path);
   renderer_path = fp_renderer_path.DirName().ToWStringHack();
   file_util::AppendToPath(&renderer_path,
-                          chrome::kBrowserProcessExecutableName);
+                          chrome::kBrowserProcessExecutablePath);
   command_line->AppendSwitchWithValue(switches::kRendererPath, renderer_path);
 
   sandbox::SandboxInterfaceInfo sandbox_info = {0};
diff --git a/chrome/test/startup/startup_test.cc b/chrome/test/startup/startup_test.cc
index 80bfeab..160369d 100644
--- a/chrome/test/startup/startup_test.cc
+++ b/chrome/test/startup/startup_test.cc
@@ -48,7 +48,7 @@ class StartupTest : public UITest {
         ASSERT_TRUE(PathService::Get(chrome::DIR_APP, &dir_app));
 
         FilePath chrome_exe(dir_app.Append(
-            FilePath::FromWStringHack(chrome::kBrowserProcessExecutableName)));
+            FilePath::FromWStringHack(chrome::kBrowserProcessExecutablePath)));
         ASSERT_TRUE(EvictFileFromSystemCacheWrapper(chrome_exe));
 #if defined(OS_WIN)
         // TODO(port): these files do not exist on other platforms.
diff --git a/chrome/test/ui/ui_test.cc b/chrome/test/ui/ui_test.cc
index d5ac11d..01eb35a 100644
--- a/chrome/test/ui/ui_test.cc
+++ b/chrome/test/ui/ui_test.cc
@@ -239,7 +239,7 @@ void UITest::CloseBrowserAndServer() {
 void UITest::LaunchBrowser(const CommandLine& arguments, bool clear_profile) {
   std::wstring command = browser_directory_;
   file_util::AppendToPath(&command,
-                          chrome::kBrowserProcessExecutableName);
+                          chrome::kBrowserProcessExecutablePath);
   CommandLine command_line(command);
 
   // Add any explict command line flags passed to the process.