Add testing of the Chrome Frame user-mode installation to mini_installer_tests.exe.

Also do some miscellaneous cleanup and rejiggering such as removing test-only constants from chrome\common, adding more useful prints when tests fail, properly waiting for child processes to terminate and moving a timeout value to test_timeouts.cc in test_support_common.

BUG=53127
TEST=Chrome Frame user level installs are tested.


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

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

6 files changed, 148 insertions, 100 deletions

diff --git a/chrome/test/mini_installer_test/chrome_mini_installer.cc b/chrome/test/mini_installer_test/chrome_mini_installer.cc
index b1ba240..e53b020 100644
--- a/chrome/test/mini_installer_test/chrome_mini_installer.cc
+++ b/chrome/test/mini_installer_test/chrome_mini_installer.cc
@@ -4,9 +4,11 @@
 
 #include "chrome/test/mini_installer_test/chrome_mini_installer.h"
 
+#include "base/command_line.h"
 #include "base/file_util.h"
 #include "base/path_service.h"
 #include "base/platform_thread.h"
+#include "base/process.h"
 #include "base/process_util.h"
 #include "base/registry.h"
 #include "base/string_number_conversions.h"
@@ -130,10 +132,12 @@ void ChromeMiniInstaller::InstallMiniInstaller(bool over_install,
   printf("\nChrome will be installed at %ls level\n", install_type_.c_str());
   printf("\nWill proceed with the test only if this path exists: %ls\n\n",
          path.c_str());
-  ASSERT_TRUE(file_util::PathExists(FilePath(path)));
+  ASSERT_TRUE(file_util::PathExists(FilePath(path))) << path
+                                                     << " does not exist.";
   LaunchInstaller(path, exe_name.c_str());
   BrowserDistribution* dist = BrowserDistribution::GetDistribution();
-  ASSERT_TRUE(CheckRegistryKey(dist->GetVersionKey()));
+  ASSERT_TRUE(CheckRegistryKey(dist->GetVersionKey())) << dist->GetVersionKey()
+                                                       << " does not exist.";
   VerifyInstall(over_install);
 }
 
@@ -265,12 +269,15 @@ void ChromeMiniInstaller::UnInstall() {
     uninstall_args.append(L" --chrome-frame");
   if (install_type_ == mini_installer_constants::kSystemInstall)
     uninstall_args = uninstall_args + L" --system-level";
-  base::LaunchApp(uninstall_args, false, false, NULL);
+
+  base::ProcessHandle setup_handle;
+  base::LaunchApp(uninstall_args, false, false, &setup_handle);
+
   if (is_chrome_frame_)
     ASSERT_TRUE(CloseUninstallWindow());
-  ASSERT_TRUE(MiniInstallerTestUtil::VerifyProcessClose(
-      mini_installer_constants::kChromeSetupExecutable));
+  ASSERT_TRUE(MiniInstallerTestUtil::VerifyProcessHandleClosed(setup_handle));
   ASSERT_FALSE(CheckRegistryKeyOnUninstall(dist->GetVersionKey()));
+
   DeleteUserDataFolder();
   // Close IE survey window that gets launched on uninstall.
   if (!is_chrome_frame_) {
@@ -399,7 +406,7 @@ void ChromeMiniInstaller::DeleteFolder(const wchar_t* folder_name) {
   }
   printf("This path will be deleted: %ls\n", install_path.value().c_str());
   ASSERT_TRUE(file_util::Delete(install_path, true));
- }
+}
 
 // Will delete user data profile.
 void ChromeMiniInstaller::DeleteUserDataFolder() {
@@ -534,21 +541,24 @@ HKEY ChromeMiniInstaller::GetRootRegistryKey() {
 void ChromeMiniInstaller::LaunchInstaller(const std::wstring& path,
                                           const wchar_t* process_name) {
   ASSERT_TRUE(file_util::PathExists(FilePath(path)));
+  std::wstring launch_args(L" --verbose-logging");
+  if (is_chrome_frame_) {
+    launch_args.append(L" --do-not-create-shortcuts");
+    launch_args.append(L" --do-not-launch-chrome");
+    launch_args.append(L" --do-not-register-for-update-launch");
+    launch_args.append(L" --chrome-frame");
+  }
   if (install_type_ == mini_installer_constants::kSystemInstall) {
-    std::wstring launch_args;
-    if (is_chrome_frame_) {
-      launch_args.append(L" --do-not-create-shortcuts");
-      launch_args.append(L" --do-not-register-for-update-launch");
-      launch_args.append(L" --chrome-frame");
-    }
     launch_args.append(L" --system-level");
-    base::LaunchApp(L"\"" + path + L"\"" + launch_args, false, false, NULL);
-  } else {
-    base::LaunchApp(L"\"" + path + L"\"", false, false, NULL);
   }
+
+  base::ProcessHandle app_handle;
+  base::LaunchApp(L"\"" + path + L"\"" + launch_args, false, false,
+                  &app_handle);
+
   printf("Waiting while this process is running  %ls ....\n", process_name);
   MiniInstallerTestUtil::VerifyProcessLaunch(process_name, true);
-  ASSERT_TRUE(MiniInstallerTestUtil::VerifyProcessClose(process_name));
+  ASSERT_TRUE(MiniInstallerTestUtil::VerifyProcessHandleClosed(app_handle));
 }
 
 // Gets the path to launch Chrome.
@@ -599,26 +609,30 @@ void ChromeMiniInstaller::VerifyInstall(bool over_install) {
 // launch IE with cf:about:version, then check if
 // chrome.exe process got spawned.
 void ChromeMiniInstaller::VerifyChromeFrameInstall() {
-  std::wstring browser_path = GetChromeInstallDirectoryLocation();
-  if (is_chrome_frame_) {
-    file_util::AppendToPath(&browser_path,
-                            mini_installer_constants::kIELocation);
-    file_util::AppendToPath(&browser_path,
-                            mini_installer_constants::kIEProcessName);
-  }
+  FilePath browser_path;
+  PathService::Get(base::DIR_PROGRAM_FILES, &browser_path);
+  browser_path = browser_path.Append(mini_installer_constants::kIELocation);
+  browser_path = browser_path.Append(mini_installer_constants::kIEProcessName);
+
+  CommandLine cmd_line(browser_path);
+  cmd_line.AppendArgNative(L"gcf:about:version");
 
   // Launch IE
-  LaunchBrowser(browser_path, L"gcf:about:version",
-                mini_installer_constants::kIEProcessName,
-                true);
+  base::LaunchApp(cmd_line, false, false, NULL);
 
   // Check if Chrome process got spawned.
   MiniInstallerTestUtil::VerifyProcessLaunch(installer_util::kChromeExe, true);
-  PlatformThread::Sleep(1500);
 
   // Verify if IExplore folder got created
   FilePath path = GetUserDataDirPath();
   path = path.AppendASCII("IEXPLORE");
+  if (!file_util::PathExists(path)) {
+    // The profile folder for IE 6 and 7 lives in the user's Temporary Internet
+    // Files folder. Check there if the previous one was not found.
+    PathService::Get(base::DIR_IE_INTERNET_CACHE, &path);
+    path = path.AppendASCII("Google Chrome Frame");
+  }
+
   ASSERT_TRUE(file_util::PathExists(path));
 }
 
diff --git a/chrome/test/mini_installer_test/mini_installer_test_constants.cc b/chrome/test/mini_installer_test/mini_installer_test_constants.cc
index da682e6..7f4ca3d 100644
--- a/chrome/test/mini_installer_test/mini_installer_test_constants.cc
+++ b/chrome/test/mini_installer_test/mini_installer_test_constants.cc
@@ -62,7 +62,23 @@ const wchar_t kChromeStandAloneInstallerLocation[] =
 const wchar_t kChromeApplyTagExe[] =
     L"\\\\172.23.44.61\\shared\\chrome_autotest\\ApplyTag.exe";
 const wchar_t kChromeApplyTagParameters[] =
-    L"\"appguid={8A69D345-D564-463C-AFF1-A69D9E530F96}&appname=Chrome&needsadmin=false\"";
+    L"\"appguid={8A69D345-D564-463C-AFF1-A69D9E530F96}"
+    L"&appname=Chrome&needsadmin=false\"";
 const wchar_t kChromeDiffInstallerLocation[] =
     L"\\\\172.24.6.7\\shares\\chromeclient\\builds\\chrome\\";
-}
+
+}  // namespace mini_installer_constants
+
+namespace switches {
+// Back up the profile.
+const char kInstallerTestBackup[] = "backup";
+
+// Control the build under test.
+const char kInstallerTestBuild[] = "build";
+
+// Uninstall before running the tests.
+const char kInstallerTestClean[] = "clean";
+
+// Force the installer tests to run, regardless of the current platform.
+const char kInstallerTestForce[] = "force";
+}  // namespace switches
diff --git a/chrome/test/mini_installer_test/mini_installer_test_constants.h b/chrome/test/mini_installer_test/mini_installer_test_constants.h
index 8faf010..8893dac 100644
--- a/chrome/test/mini_installer_test/mini_installer_test_constants.h
+++ b/chrome/test/mini_installer_test/mini_installer_test_constants.h
@@ -6,7 +6,6 @@
 
 #ifndef CHROME_TEST_MINI_INSTALLER_TEST_MINI_INSTALLER_TEST_CONSTANTS_H__
 #define CHROME_TEST_MINI_INSTALLER_TEST_MINI_INSTALLER_TEST_CONSTANTS_H__
-#pragma once
 
 namespace mini_installer_constants {
 
@@ -60,6 +59,14 @@ extern const wchar_t kChromeApplyTagParameters[];
 extern const wchar_t kChromeDiffInstallerLocation[];
 extern const wchar_t kChromeMetaInstallerExe[];
 extern const wchar_t kChromeStandAloneInstallerLocation[];
-}
+}  // namespace mini_installer_constants
+
+// Command line switches.
+namespace switches {
+extern const char kInstallerTestBackup[];
+extern const char kInstallerTestBuild[];
+extern const char kInstallerTestClean[];
+extern const char kInstallerTestForce[];
+}  // namespace switches
 
 #endif  // CHROME_TEST_MINI_INSTALLER_TEST_MINI_INSTALLER_TEST_CONSTANTS_H__
diff --git a/chrome/test/mini_installer_test/mini_installer_test_util.cc b/chrome/test/mini_installer_test/mini_installer_test_util.cc
index e8fb79d..e86b3e1 100644
--- a/chrome/test/mini_installer_test/mini_installer_test_util.cc
+++ b/chrome/test/mini_installer_test/mini_installer_test_util.cc
@@ -13,6 +13,7 @@
 #include "base/utf_string_conversions.h"
 #include "chrome/installer/util/logging_installer.h"
 #include "chrome/test/mini_installer_test/mini_installer_test_constants.h"
+#include "chrome/test/test_timeouts.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 // Change current directory so that chrome.dll from current folder
@@ -289,7 +290,7 @@ bool MiniInstallerTestUtil::VerifyProcessClose(
   if (base::GetProcessCount(process_name, NULL) > 0) {
     LOG(INFO) << "Waiting for this process to end: " << process_name;
     while ((base::GetProcessCount(process_name, NULL) > 0) &&
-           (timer < 60000)) {
+           (timer < TestTimeouts::large_test_timeout_ms())) {
       PlatformThread::Sleep(200);
       timer = timer + 200;
     }
@@ -299,3 +300,10 @@ bool MiniInstallerTestUtil::VerifyProcessClose(
   }
   return true;
 }
+
+bool MiniInstallerTestUtil::VerifyProcessHandleClosed(
+    base::ProcessHandle handle) {
+  DWORD result = WaitForSingleObject(handle,
+      TestTimeouts::large_test_timeout_ms());
+  return result == WAIT_OBJECT_0;
+}
diff --git a/chrome/test/mini_installer_test/mini_installer_test_util.h b/chrome/test/mini_installer_test/mini_installer_test_util.h
index 1206751..a6e15b6 100644
--- a/chrome/test/mini_installer_test/mini_installer_test_util.h
+++ b/chrome/test/mini_installer_test/mini_installer_test_util.h
@@ -14,6 +14,7 @@
 #include <vector>
 
 #include "base/basictypes.h"
+#include "base/process.h"
 
 class FilePath;
 
@@ -98,6 +99,10 @@ class MiniInstallerTestUtil {
   // Verifies if the given process stops running.
   static bool VerifyProcessClose(const wchar_t* process_name);
 
+  // Waits on the given process name until it returns or until a timeout is
+  // reached.
+  static bool VerifyProcessHandleClosed(base::ProcessHandle handle);
+
  private:
   DISALLOW_COPY_AND_ASSIGN(MiniInstallerTestUtil);
 };
diff --git a/chrome/test/mini_installer_test/test.cc b/chrome/test/mini_installer_test/test.cc
index 251c9cd..a98b429 100644
--- a/chrome/test/mini_installer_test/test.cc
+++ b/chrome/test/mini_installer_test/test.cc
@@ -3,17 +3,18 @@
 // found in the LICENSE file.
 
 #include "base/command_line.h"
+#include "base/file_path.h"
 #include "base/platform_thread.h"
 #include "base/scoped_ptr.h"
 #include "base/win_util.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/installer/util/install_util.h"
 #include "chrome/installer/util/util_constants.h"
+#include "chrome/test/mini_installer_test/chrome_mini_installer.h"
 #include "chrome/test/mini_installer_test/mini_installer_test_constants.h"
 #include "chrome/test/mini_installer_test/mini_installer_test_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-#include "chrome_mini_installer.h"
 
 // Although the C++ style guide disallows use of namespace directive, use
 // here because this is not only a .cc file, but also a test.
@@ -22,76 +23,74 @@ using namespace mini_installer_constants;
 namespace {
 
 class MiniInstallTest : public testing::Test {
-   protected:
-    // Whether these tests should be run regardless of our running platform.
-    bool force_tests_;
-
-    // Decided if ChromeFrame tests should be run.
-    bool chrome_frame_;
-
-    // Installers created in test fixture setup for convenience.
-    scoped_ptr<ChromeMiniInstaller> user_inst_, sys_inst_;
-
-   public:
-
-    static void CleanTheSystem() {
-      const CommandLine* cmd = CommandLine::ForCurrentProcess();
-      if (cmd->HasSwitch(installer_util::switches::kChromeFrame)) {
-        ChromeMiniInstaller systeminstall(kSystemInstall,
-            cmd->HasSwitch(installer_util::switches::kChromeFrame));
-        systeminstall.UnInstall();
-      } else {
-        ChromeMiniInstaller userinstall(kUserInstall,
-            cmd->HasSwitch(installer_util::switches::kChromeFrame));
-        userinstall.UnInstall();
-        ChromeMiniInstaller systeminstall(kSystemInstall,
-            cmd->HasSwitch(installer_util::switches::kChromeFrame));
-        systeminstall.UnInstall();
-      }
+ public:
+   MiniInstallTest() : force_tests_(false), chrome_frame_(false) {}
+
+  static void CleanTheSystem() {
+    const CommandLine* cmd = CommandLine::ForCurrentProcess();
+    if (cmd->HasSwitch(installer_util::switches::kChromeFrame)) {
+      ChromeMiniInstaller systeminstall(kSystemInstall,
+          cmd->HasSwitch(installer_util::switches::kChromeFrame));
+      systeminstall.UnInstall();
+    } else {
+      ChromeMiniInstaller userinstall(kUserInstall,
+          cmd->HasSwitch(installer_util::switches::kChromeFrame));
+      userinstall.UnInstall();
+      ChromeMiniInstaller systeminstall(kSystemInstall,
+          cmd->HasSwitch(installer_util::switches::kChromeFrame));
+      systeminstall.UnInstall();
     }
-
-    virtual void SetUp() {
-      // Parse test command-line arguments.
-      const CommandLine* cmd = CommandLine::ForCurrentProcess();
-      std::wstring build =
-          cmd->GetSwitchValueNative(switches::kInstallerTestBuild);
-      if (build.empty())
-        build = L"latest";
-      force_tests_ = cmd->HasSwitch(switches::kInstallerTestForce);
-      chrome_frame_ = cmd->HasSwitch(installer_util::switches::kChromeFrame);
-      if (win_util::GetWinVersion() < win_util::WINVERSION_VISTA ||
-          force_tests_) {
-        CleanTheSystem();
-        // Separate the test output from cleaning output
-        printf("\nBEGIN test----------------------------------------\n");
-
-        // Create a few differently configured installers that are used in
-        // the tests, for convenience.
-        if (chrome_frame_) {
-          sys_inst_.reset(new ChromeMiniInstaller(kSystemInstall,
-                                                  chrome_frame_));
-          sys_inst_->SetBuildUnderTest(build);
-        } else {
-          user_inst_.reset(new ChromeMiniInstaller(kUserInstall,
-                                                   chrome_frame_));
-          user_inst_->SetBuildUnderTest(build);
-          sys_inst_.reset(new ChromeMiniInstaller(kSystemInstall,
-                                                  chrome_frame_));
-          sys_inst_->SetBuildUnderTest(build);
-        }
-      } else {
-        printf("These tests don't run on this platform.\n");
-        exit(0);
-      }
-    }
-
-    static void TearDownTestCase() {
-      // Uninstall Chrome from the system after tests are run.
+  }
+
+  virtual void SetUp() {
+    // Parse test command-line arguments.
+    const CommandLine* cmd = CommandLine::ForCurrentProcess();
+    std::wstring build =
+        cmd->GetSwitchValueNative(switches::kInstallerTestBuild);
+    if (build.empty())
+      build = L"latest";
+    force_tests_ = cmd->HasSwitch(switches::kInstallerTestForce);
+    chrome_frame_ = cmd->HasSwitch(installer_util::switches::kChromeFrame);
+    if (win_util::GetWinVersion() < win_util::WINVERSION_VISTA ||
+        force_tests_) {
       CleanTheSystem();
+      // Separate the test output from cleaning output
+      printf("\nBEGIN test----------------------------------------\n");
+
+      // Create a few differently configured installers that are used in
+      // the tests, for convenience.
+      user_inst_.reset(new ChromeMiniInstaller(kUserInstall,
+                                               chrome_frame_));
+      sys_inst_.reset(new ChromeMiniInstaller(kSystemInstall,
+                                              chrome_frame_));
+      sys_inst_->SetBuildUnderTest(build);
+      user_inst_->SetBuildUnderTest(build);
+
+    } else {
+      printf("These tests don't run on this platform.\n");
+      exit(0);
     }
-};
+  }
+
+  static void TearDownTestCase() {
+    // Uninstall Chrome from the system after tests are run.
+    CleanTheSystem();
+  }
+
+ protected:
+  // Whether these tests should be run regardless of our running platform.
+  bool force_tests_;
+
+  // Decided if ChromeFrame tests should be run.
+  bool chrome_frame_;
+
+  // Installers created in test fixture setup for convenience.
+  scoped_ptr<ChromeMiniInstaller> user_inst_;
+  scoped_ptr<ChromeMiniInstaller> sys_inst_;
 };
 
+}  // namespace
+
 #if defined(GOOGLE_CHROME_BUILD)
 // Could use a parameterized gtest to slim down this list of tests, but since
 // these tests will often be run manually, don't want to have obscure test
@@ -177,8 +176,7 @@ TEST_F(MiniInstallTest, InstallMiniInstallerSys) {
 }
 
 TEST_F(MiniInstallTest, InstallMiniInstallerUser) {
-  if (!chrome_frame_)
-    user_inst_->Install();
+  user_inst_->Install();
 }
 
 TEST_F(MiniInstallTest, MiniInstallTestValidWindowsVersion) {