Make Ash unittests clean up zombie viewer processes between test runs. These zombie viewer processes seem to be a side effect of attempting to quit a metro app while the same app is activating. *while here can mean within a certain number of seconds due to metro life cycle management.

Refactor KillAllNamedProcessesWithArgument into base/test.

BUG=154081
TEST=Run "ash_unittests.exe --ash-metro-tests" on Win8. Observe that 417 chrome.exe processes do not get created :)


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

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

9 files changed, 203 insertions, 160 deletions

diff --git a/ash/test/ash_test_base.cc b/ash/test/ash_test_base.cc
index 0116710..980777e 100644
--- a/ash/test/ash_test_base.cc
+++ b/ash/test/ash_test_base.cc
@@ -32,6 +32,7 @@
 
 #if defined(OS_WIN)
 #include "ash/test/test_metro_viewer_process_host.h"
+#include "base/test/test_process_killer_win.h"
 #include "base/win/windows_version.h"
 #include "ui/aura/remote_root_window_host_win.h"
 #include "ui/aura/root_window_host_win.h"
@@ -131,11 +132,21 @@ void AshTestBase::TearDown() {
   Shell::DeleteInstance();
   aura::Env::DeleteInstance();
   ui::TextInputTestSupport::Shutdown();
+
 #if defined(OS_WIN)
   aura::test::SetUsePopupAsRootWindowForTest(false);
   // Kill the viewer process if we spun one up.
   metro_viewer_host_.reset();
+
+  // Clean up any dangling viewer processes as the metro APIs sometimes leave
+  // zombies behind. A default browser process in metro will have the
+  // following command line arg so use that to avoid killing all processes named
+  // win8::test::kDefaultTestExePath.
+  const wchar_t kViewerProcessArgument[] = L"DefaultBrowserServer";
+  base::KillAllNamedProcessesWithArgument(win8::test::kDefaultTestExePath,
+                                          kViewerProcessArgument);
 #endif
+
   event_generator_.reset();
   // Some tests set an internal display id,
   // reset it here, so other tests will continue in a clean environment.
diff --git a/base/base.gyp b/base/base.gyp
index 93f9d28..a17508a 100644
--- a/base/base.gyp
+++ b/base/base.gyp
@@ -849,6 +849,8 @@
         'test/test_listener_ios.mm',
         'test/test_pending_task.cc',
         'test/test_pending_task.h',
+        'test/test_process_killer_win.cc',
+        'test/test_process_killer_win.h',
         'test/test_reg_util_win.cc',
         'test/test_reg_util_win.h',
         'test/test_shortcut_win.cc',
diff --git a/base/test/test_process_killer_win.cc b/base/test/test_process_killer_win.cc
new file mode 100644
index 0000000..bc6e17e
--- /dev/null
+++ b/base/test/test_process_killer_win.cc
@@ -0,0 +1,165 @@
+// Copyright (c) 2013 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.
+
+#include "base/test/test_process_killer_win.h"
+
+#include <windows.h>
+#include <winternl.h>
+
+#include <algorithm>
+
+#include "base/logging.h"
+#include "base/process_util.h"
+#include "base/string_util.h"
+#include "base/win/scoped_handle.h"
+
+namespace {
+
+typedef LONG WINAPI
+NtQueryInformationProcess(
+  IN HANDLE ProcessHandle,
+  IN PROCESSINFOCLASS ProcessInformationClass,
+  OUT PVOID ProcessInformation,
+  IN ULONG ProcessInformationLength,
+  OUT PULONG ReturnLength OPTIONAL
+);
+
+// Get the function pointer to NtQueryInformationProcess in NTDLL.DLL
+static bool GetQIP(NtQueryInformationProcess** qip_func_ptr) {
+  static NtQueryInformationProcess* qip_func =
+      reinterpret_cast<NtQueryInformationProcess*>(
+          GetProcAddress(GetModuleHandle(L"ntdll.dll"),
+          "NtQueryInformationProcess"));
+  DCHECK(qip_func) << "Could not get pointer to NtQueryInformationProcess.";
+  *qip_func_ptr = qip_func;
+  return qip_func != NULL;
+}
+
+// Get the command line of a process
+bool GetCommandLineForProcess(uint32 process_id, string16* cmd_line) {
+  DCHECK(process_id != 0);
+  DCHECK(cmd_line);
+
+  // Open the process
+  base::win::ScopedHandle process_handle(::OpenProcess(
+      PROCESS_QUERY_INFORMATION | PROCESS_VM_READ,
+      false,
+      process_id));
+  if (!process_handle) {
+    DLOG(ERROR) << "Failed to open process " << process_id << ", last error = "
+                << GetLastError();
+  }
+
+  // Obtain Process Environment Block
+  NtQueryInformationProcess* qip_func = NULL;
+  if (process_handle) {
+    GetQIP(&qip_func);
+  }
+
+  // Read the address of the process params from the peb.
+  DWORD process_params_address = 0;
+  if (qip_func) {
+    PROCESS_BASIC_INFORMATION info = { 0 };
+    // NtQueryInformationProcess returns an NTSTATUS for whom negative values
+    // are negative. Just check for that instead of pulling in DDK macros.
+    if ((qip_func(process_handle.Get(),
+                  ProcessBasicInformation,
+                  &info,
+                  sizeof(info),
+                  NULL)) < 0) {
+      DLOG(ERROR) << "Failed to invoke NtQueryProcessInformation, last error = "
+                  << GetLastError();
+    } else {
+      BYTE* peb = reinterpret_cast<BYTE*>(info.PebBaseAddress);
+
+      // The process command line parameters are (or were once) located at
+      // the base address of the PEB + 0x10 for 32 bit processes. 64 bit
+      // processes have a different PEB struct as per
+      // http://msdn.microsoft.com/en-us/library/aa813706(VS.85).aspx.
+      // TODO(robertshield): See about doing something about this.
+      SIZE_T bytes_read = 0;
+      if (!::ReadProcessMemory(process_handle.Get(),
+                               peb + 0x10,
+                               &process_params_address,
+                               sizeof(process_params_address),
+                               &bytes_read)) {
+        DLOG(ERROR) << "Failed to read process params address, last error = "
+                    << GetLastError();
+      }
+    }
+  }
+
+  // Copy all the process parameters into a buffer.
+  bool success = false;
+  string16 buffer;
+  if (process_params_address) {
+    SIZE_T bytes_read;
+    RTL_USER_PROCESS_PARAMETERS params = { 0 };
+    if (!::ReadProcessMemory(process_handle.Get(),
+                             reinterpret_cast<void*>(process_params_address),
+                             &params,
+                             sizeof(params),
+                             &bytes_read)) {
+      DLOG(ERROR) << "Failed to read RTL_USER_PROCESS_PARAMETERS, "
+                  << "last error = " << GetLastError();
+    } else {
+      // Read the command line parameter
+      const int max_cmd_line_len = std::min(
+          static_cast<int>(params.CommandLine.MaximumLength),
+          4096);
+      buffer.resize(max_cmd_line_len + 1);
+      if (!::ReadProcessMemory(process_handle.Get(),
+                               params.CommandLine.Buffer,
+                               &buffer[0],
+                               max_cmd_line_len,
+                               &bytes_read)) {
+        DLOG(ERROR) << "Failed to copy process command line, "
+                    << "last error = " << GetLastError();
+      } else {
+        *cmd_line = buffer;
+        success = true;
+      }
+    }
+  }
+
+  return success;
+}
+
+// Used to filter processes by process ID.
+class ArgumentFilter : public base::ProcessFilter {
+ public:
+  explicit ArgumentFilter(const string16& argument)
+      : argument_to_find_(argument) {}
+
+  // Returns true to indicate set-inclusion and false otherwise.  This method
+  // should not have side-effects and should be idempotent.
+  virtual bool Includes(const base::ProcessEntry& entry) const {
+    bool found = false;
+    string16 command_line;
+    if (GetCommandLineForProcess(entry.pid(), &command_line)) {
+      string16::const_iterator it =
+          std::search(command_line.begin(),
+                      command_line.end(),
+                      argument_to_find_.begin(),
+                      argument_to_find_.end(),
+          base::CaseInsensitiveCompareASCII<wchar_t>());
+      found = (it != command_line.end());
+    }
+    return found;
+  }
+
+ protected:
+  string16 argument_to_find_;
+};
+
+}  // namespace
+
+namespace base {
+
+bool KillAllNamedProcessesWithArgument(const string16& process_name,
+                                       const string16& argument) {
+  return base::KillProcesses(process_name, 0, &ArgumentFilter(argument));
+}
+
+}  // namespace base
diff --git a/base/test/test_process_killer_win.h b/base/test/test_process_killer_win.h
new file mode 100644
index 0000000..30aab8e
--- /dev/null
+++ b/base/test/test_process_killer_win.h
@@ -0,0 +1,19 @@
+// Copyright (c) 2013 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.
+
+#ifndef BASE_TEST_TEST_PROCESS_KILLER_WIN_H_
+#define BASE_TEST_TEST_PROCESS_KILLER_WIN_H_
+
+#include "base/string16.h"
+
+namespace base {
+
+// Kills all running processes named |process_name| that have the string
+// |argument| on their command line.
+bool KillAllNamedProcessesWithArgument(const string16& process_name,
+                                       const string16& argument);
+
+}  // namespace base
+
+#endif  // BASE_TEST_TEST_PROCESS_KILLER_WIN_H_
diff --git a/chrome_frame/test/chrome_frame_automation_mock.h b/chrome_frame/test/chrome_frame_automation_mock.h
index a550d5d..052d36d 100644
--- a/chrome_frame/test/chrome_frame_automation_mock.h
+++ b/chrome_frame/test/chrome_frame_automation_mock.h
@@ -9,12 +9,12 @@
 
 #include "base/files/file_path.h"
 #include "base/path_service.h"
+#include "base/test/test_process_killer_win.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome_frame/chrome_frame_automation.h"
 #include "chrome_frame/chrome_frame_plugin.h"
 #include "chrome_frame/navigation_constraints.h"
-#include "chrome_frame/test/chrome_frame_test_utils.h"
 #include "chrome_frame/test/test_scrubber.h"
 #include "chrome_frame/test/test_with_web_server.h"
 #include "chrome_frame/utils.h"
@@ -35,7 +35,7 @@ class AutomationMockDelegate
             chrome_frame_test::GetTestDataFolder()) {
 
     // Endeavour to only kill off Chrome Frame derived Chrome processes.
-    KillAllNamedProcessesWithArgument(
+    base::KillAllNamedProcessesWithArgument(
         UTF8ToWide(chrome_frame_test::kChromeImageName),
         UTF8ToWide(switches::kChromeFrame));
 
diff --git a/chrome_frame/test/mock_ie_event_sink_actions.h b/chrome_frame/test/mock_ie_event_sink_actions.h
index 384a58b..aee5772 100644
--- a/chrome_frame/test/mock_ie_event_sink_actions.h
+++ b/chrome_frame/test/mock_ie_event_sink_actions.h
@@ -13,10 +13,10 @@
 #include "base/bind.h"
 #include "base/string_util.h"
 #include "base/string16.h"
+#include "base/test/test_process_killer_win.h"
 #include "base/threading/platform_thread.h"
 #include "base/time.h"
 #include "chrome/common/chrome_switches.h"
-#include "chrome_frame/test/chrome_frame_test_utils.h"
 #include "chrome_frame/test/chrome_frame_ui_test_utils.h"
 #include "chrome_frame/test/ie_event_sink.h"
 #include "chrome_frame/test/mock_ie_event_sink_test.h"
@@ -315,7 +315,7 @@ ACTION_P(DelayDoCloseWindow, delay) {
 }
 
 ACTION(KillChromeFrameProcesses) {
-  KillAllNamedProcessesWithArgument(
+  base::KillAllNamedProcessesWithArgument(
       UTF8ToWide(chrome_frame_test::kChromeImageName),
       UTF8ToWide(switches::kChromeFrame));
 }
diff --git a/chrome_frame/test/test_scrubber.cc b/chrome_frame/test/test_scrubber.cc
index ac1a0d2..accb224 100644
--- a/chrome_frame/test/test_scrubber.cc
+++ b/chrome_frame/test/test_scrubber.cc
@@ -17,6 +17,7 @@
 #include "base/logging.h"
 #include "base/process_util.h"
 #include "base/string16.h"
+#include "base/test/test_process_killer_win.h"
 #include "base/utf_string_conversions.h"
 #include "base/win/registry.h"
 #include "base/win/scoped_handle.h"
@@ -102,7 +103,7 @@ void TestScrubber::CleanUpFromTestRun() {
   base::KillProcesses(chrome_frame_test::kChromeLauncher, 0, NULL);
 
   // Kill all chrome.exe processes with --chrome-frame.
-  KillAllNamedProcessesWithArgument(
+  base::KillAllNamedProcessesWithArgument(
       chrome::kBrowserProcessExecutableName,
       ASCIIToWide(switches::kChromeFrame));
 
diff --git a/chrome_frame/test_utils.cc b/chrome_frame/test_utils.cc
index 7e516a8..4d308db 100644
--- a/chrome_frame/test_utils.cc
+++ b/chrome_frame/test_utils.cc
@@ -7,7 +7,6 @@
 #include <atlbase.h>
 #include <atlwin.h>
 #include <shellapi.h>
-#include <winternl.h>
 
 #include <algorithm>
 
@@ -17,7 +16,6 @@
 #include "base/logging.h"
 #include "base/path_service.h"
 #include "base/process_util.h"
-#include "base/string_util.h"
 #include "base/stringprintf.h"
 #include "base/utf_string_conversions.h"
 #include "base/win/scoped_handle.h"
@@ -269,153 +267,6 @@ std::wstring ScopedChromeFrameRegistrar::GetChromeFrameDllPath() const {
   return new_chrome_frame_dll_path_;
 }
 
-// TODO(robertshield): The following could be factored out into its own file.
-namespace {
-
-typedef LONG WINAPI
-NtQueryInformationProcess(
-  IN HANDLE ProcessHandle,
-  IN PROCESSINFOCLASS ProcessInformationClass,
-  OUT PVOID ProcessInformation,
-  IN ULONG ProcessInformationLength,
-  OUT PULONG ReturnLength OPTIONAL
-);
-
-// Get the function pointer to NtQueryInformationProcess in NTDLL.DLL
-static bool GetQIP(NtQueryInformationProcess** qip_func_ptr) {
-  static NtQueryInformationProcess* qip_func =
-      reinterpret_cast<NtQueryInformationProcess*>(
-          GetProcAddress(GetModuleHandle(L"ntdll.dll"),
-          "NtQueryInformationProcess"));
-  DCHECK(qip_func) << "Could not get pointer to NtQueryInformationProcess.";
-  *qip_func_ptr = qip_func;
-  return qip_func != NULL;
-}
-
-// Get the command line of a process
-bool GetCommandLineForProcess(uint32 process_id, std::wstring* cmd_line) {
-  DCHECK(process_id != 0);
-  DCHECK(cmd_line);
-
-  // Open the process
-  base::win::ScopedHandle process_handle(::OpenProcess(
-      PROCESS_QUERY_INFORMATION | PROCESS_VM_READ,
-      false,
-      process_id));
-  if (!process_handle) {
-    DLOG(ERROR) << "Failed to open process " << process_id << ", last error = "
-                << GetLastError();
-  }
-
-  // Obtain Process Environment Block
-  NtQueryInformationProcess* qip_func = NULL;
-  if (process_handle) {
-    GetQIP(&qip_func);
-  }
-
-  // Read the address of the process params from the peb.
-  DWORD process_params_address = 0;
-  if (qip_func) {
-    PROCESS_BASIC_INFORMATION info = { 0 };
-    // NtQueryInformationProcess returns an NTSTATUS for whom negative values
-    // are negative. Just check for that instead of pulling in DDK macros.
-    if ((qip_func(process_handle.Get(),
-                  ProcessBasicInformation,
-                  &info,
-                  sizeof(info),
-                  NULL)) < 0) {
-      DLOG(ERROR) << "Failed to invoke NtQueryProcessInformation, last error = "
-                  << GetLastError();
-    } else {
-      BYTE* peb = reinterpret_cast<BYTE*>(info.PebBaseAddress);
-
-      // The process command line parameters are (or were once) located at
-      // the base address of the PEB + 0x10 for 32 bit processes. 64 bit
-      // processes have a different PEB struct as per
-      // http://msdn.microsoft.com/en-us/library/aa813706(VS.85).aspx.
-      // TODO(robertshield): See about doing something about this.
-      SIZE_T bytes_read = 0;
-      if (!::ReadProcessMemory(process_handle.Get(),
-                               peb + 0x10,
-                               &process_params_address,
-                               sizeof(process_params_address),
-                               &bytes_read)) {
-        DLOG(ERROR) << "Failed to read process params address, last error = "
-                    << GetLastError();
-      }
-    }
-  }
-
-  // Copy all the process parameters into a buffer.
-  bool success = false;
-  std::wstring buffer;
-  if (process_params_address) {
-    SIZE_T bytes_read;
-    RTL_USER_PROCESS_PARAMETERS params = { 0 };
-    if (!::ReadProcessMemory(process_handle.Get(),
-                             reinterpret_cast<void*>(process_params_address),
-                             &params,
-                             sizeof(params),
-                             &bytes_read)) {
-      DLOG(ERROR) << "Failed to read RTL_USER_PROCESS_PARAMETERS, "
-                  << "last error = " << GetLastError();
-    } else {
-      // Read the command line parameter
-      const int max_cmd_line_len = std::min(
-          static_cast<int>(params.CommandLine.MaximumLength),
-          4096);
-      buffer.resize(max_cmd_line_len + 1);
-      if (!::ReadProcessMemory(process_handle.Get(),
-                               params.CommandLine.Buffer,
-                               &buffer[0],
-                               max_cmd_line_len,
-                               &bytes_read)) {
-        DLOG(ERROR) << "Failed to copy process command line, "
-                    << "last error = " << GetLastError();
-      } else {
-        *cmd_line = buffer;
-        success = true;
-      }
-    }
-  }
-
-  return success;
-}
-
-// Used to filter processes by process ID.
-class ArgumentFilter : public base::ProcessFilter {
- public:
-  explicit ArgumentFilter(const std::wstring& argument)
-      : argument_to_find_(argument) {}
-
-  // Returns true to indicate set-inclusion and false otherwise.  This method
-  // should not have side-effects and should be idempotent.
-  virtual bool Includes(const base::ProcessEntry& entry) const {
-    bool found = false;
-    std::wstring command_line;
-    if (GetCommandLineForProcess(entry.pid(), &command_line)) {
-      std::wstring::const_iterator it =
-          std::search(command_line.begin(),
-                      command_line.end(),
-                      argument_to_find_.begin(),
-                      argument_to_find_.end(),
-          base::CaseInsensitiveCompareASCII<wchar_t>());
-      found = (it != command_line.end());
-    }
-    return found;
-  }
-
- protected:
-  std::wstring argument_to_find_;
-};
-
-}  // namespace
-
-bool KillAllNamedProcessesWithArgument(const std::wstring& process_name,
-                                       const std::wstring& argument) {
-  return base::KillProcesses(process_name, 0, &ArgumentFilter(argument));
-}
-
 bool IsWorkstationLocked() {
   bool is_locked = true;
   HDESK input_desk = ::OpenInputDesktop(0, 0, GENERIC_READ);
diff --git a/chrome_frame/test_utils.h b/chrome_frame/test_utils.h
index 6366ad9..b598e69 100644
--- a/chrome_frame/test_utils.h
+++ b/chrome_frame/test_utils.h
@@ -128,12 +128,6 @@ class DispCallback
   Method method_;
 };
 
-// Kills all running processes named |process_name| that have the string
-// |argument| on their command line. Useful for killing all Chrome Frame
-// instances of Chrome that all have --chrome-frame in their command line.
-bool KillAllNamedProcessesWithArgument(const std::wstring& process_name,
-                                       const std::wstring& argument);
-
 // If the workstation is locked and cannot receive user input.
 bool IsWorkstationLocked();