Revert of Delete old files after an update. (patchset #11 id:200001 of https://codereview.chromium.org/1666363002/ )

Reason for revert:
This has caused memory errors to be reported on DrMemory. The errors are in GenerateSpecificPEFileVersion, and relate to the VisitResourceContext not being properly initialized.

Turns out you're just really unlucky, until now nobody was actually using that (or GenerateAlternatePEFileVersion, which looks like it would have the same problem).

This is the bot where it started happening:
https://build.chromium.org/p/chromium.memory.fyi/builders/Windows%20Unit%20%28DrMemory%20full%29%20%285%29/builds/6716

This is some sample error output:
[----------] 10 tests from DeleteOldVersionsTest
[ RUN      ] DeleteOldVersionsTest.DeleteOldExecutableWithoutMatchingDirectory
~~Dr.M~~
~~Dr.M~~ Error #1: UNINITIALIZED READ: reading register cl
~~Dr.M~~ # 0 `anonymous namespace'::ReplaceAll                                          [chrome\installer\test\alternate_version_generator.cc:296]
~~Dr.M~~ # 1 `anonymous namespace'::VisitResource                                       [chrome\installer\test\alternate_version_generator.cc:356]
~~Dr.M~~ # 2 `anonymous namespace'::EnumResourcesWorker                                 [chrome\installer\test\pe_image_resources.cc:96]
~~Dr.M~~ # 3 `anonymous namespace'::EnumResourcesWorker                                 [chrome\installer\test\pe_image_resources.cc:84]
~~Dr.M~~ # 4 `anonymous namespace'::EnumResourcesWorker                                 [chrome\installer\test\pe_image_resources.cc:84]
~~Dr.M~~ # 5 upgrade_test::EnumResources                                                [chrome\installer\test\pe_image_resources.cc:122]
~~Dr.M~~ # 6 `anonymous namespace'::UpdateVersionIfMatch                                [chrome\installer\test\alternate_version_generator.cc:401]
~~Dr.M~~ # 7 upgrade_test::GenerateSpecificPEFileVersion                                [chrome\installer\test\alternate_version_generator.cc:769]
~~Dr.M~~ # 8 installer::`anonymous namespace'::DeleteOldVersionsTest::CreateExecutable  [chrome\installer\util\delete_old_versions_unittest.cc:66]
~~Dr.M~~ # 9 installer::DeleteOldVersionsTest_DeleteOldExecutableWithoutMatchingDirectory_Test::TestBody [chrome\installer\util\delete_old_versions_unittest.cc:129]
~~Dr.M~~ #10 testing::internal::HandleExceptionsInMethodIfSupported<>                   [testing\gtest\src\gtest.cc:2458]
~~Dr.M~~ Note: @0:00:21.478 in thread 5132
~~Dr.M~~ Note: instruction: cmp    %cl (%eax)
~~Dr.M~~
~~Dr.M~~ Error #2: UNINITIALIZED READ: reading register cl
~~Dr.M~~ # 0 `anonymous namespace'::ReplaceAll                                          [chrome\installer\test\alternate_version_generator.cc:296]
~~Dr.M~~ # 1 `anonymous namespace'::VisitResource                                       [chrome\installer\test\alternate_version_generator.cc:365]
~~Dr.M~~ # 2 `anonymous namespace'::EnumResourcesWorker                                 [chrome\installer\test\pe_image_resources.cc:96]
~~Dr.M~~ # 3 `anonymous namespace'::EnumResourcesWorker                                 [chrome\installer\test\pe_image_resources.cc:84]
~~Dr.M~~ # 4 `anonymous namespace'::EnumResourcesWorker                                 [chrome\installer\test\pe_image_resources.cc:84]
~~Dr.M~~ # 5 upgrade_test::EnumResources                                                [chrome\installer\test\pe_image_resources.cc:122]
~~Dr.M~~ # 6 `anonymous namespace'::UpdateVersionIfMatch                                [chrome\installer\test\alternate_version_generator.cc:401]
~~Dr.M~~ # 7 upgrade_test::GenerateSpecificPEFileVersion                                [chrome\installer\test\alternate_version_generator.cc:769]
~~Dr.M~~ # 8 installer::`anonymous namespace'::DeleteOldVersionsTest::CreateExecutable  [chrome\installer\util\delete_old_versions_unittest.cc:66]
~~Dr.M~~ # 9 installer::DeleteOldVersionsTest_DeleteOldExecutableWithoutMatchingDirectory_Test::TestBody [chrome\installer\util\delete_old_versions_unittest.cc:129]
~~Dr.M~~ #10 testing::internal::HandleExceptionsInMethodIfSupported<>                   [testing\gtest\src\gtest.cc:2458]
~~Dr.M~~ Note: @0:00:21.483 in thread 5132
~~Dr.M~~ Note: instruction: cmp    %cl (%eax)

Original issue's description:
> Delete old files after an update.
>
> This CL introduces a WorkItem which deletes old executables and version
> directories from the Chrome installation directory.
>
> For now, the WorkItem is used at the end of an update and at the
> end of a new_chrome.exe->chrome.exe rename. Eventually, it will also
> be invoked repetitively from a cleanup process launched when the
> WorkItem fails to delete everything the first time.
>
> BUG=451546
>
> Committed: https://crrev.com/1b50c70773051b24925b46bab8b006e606633c5c
> Cr-Commit-Position: refs/heads/master@{#378802}

TBR=grt@chromium.org,fdoray@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=451546

Review URL: https://codereview.chromium.org/1764053002

Cr-Commit-Position: refs/heads/master@{#379232}

12 files changed, 442 insertions, 609 deletions

diff --git a/chrome/chrome_installer.gypi b/chrome/chrome_installer.gypi
index a90d710..dc4cb53 100644
--- a/chrome/chrome_installer.gypi
+++ b/chrome/chrome_installer.gypi
@@ -111,7 +111,6 @@
             'installer/util/create_dir_work_item_unittest.cc',
             'installer/util/create_reg_key_work_item_unittest.cc',
             'installer/util/delete_after_reboot_helper_unittest.cc',
-            'installer/util/delete_old_versions_unittest.cc',
             'installer/util/delete_reg_key_work_item_unittest.cc',
             'installer/util/delete_reg_value_work_item_unittest.cc',
             'installer/util/delete_tree_work_item_unittest.cc',
diff --git a/chrome/chrome_installer_util.gypi b/chrome/chrome_installer_util.gypi
index facb617..e21ccbd 100644
--- a/chrome/chrome_installer_util.gypi
+++ b/chrome/chrome_installer_util.gypi
@@ -40,8 +40,6 @@
           'installer/util/create_dir_work_item.h',
           'installer/util/create_reg_key_work_item.cc',
           'installer/util/create_reg_key_work_item.h',
-          'installer/util/delete_old_versions.cc',
-          'installer/util/delete_old_versions.h',
           'installer/util/delete_reg_key_work_item.cc',
           'installer/util/delete_reg_key_work_item.h',
           'installer/util/delete_reg_value_work_item.cc',
diff --git a/chrome/installer/setup/install.cc b/chrome/installer/setup/install.cc
index 9285629..a8660d3 100644
--- a/chrome/installer/setup/install.cc
+++ b/chrome/installer/setup/install.cc
@@ -553,6 +553,13 @@ InstallStatus InstallOrUpdateProduct(
             installer_state.target_path().Append(installer::kChromeExe));
       }
     }
+
+    installer_state.UpdateStage(installer::REMOVING_OLD_VERSIONS);
+
+    installer_state.RemoveOldVersionDirectories(
+        new_version,
+        existing_version.get(),
+        install_temp_path);
   }
 
   return result;
diff --git a/chrome/installer/setup/install_worker.cc b/chrome/installer/setup/install_worker.cc
index 3825511..5878c00 100644
--- a/chrome/installer/setup/install_worker.cc
+++ b/chrome/installer/setup/install_worker.cc
@@ -40,7 +40,6 @@
 #include "chrome/installer/util/callback_work_item.h"
 #include "chrome/installer/util/conditional_work_item_list.h"
 #include "chrome/installer/util/create_reg_key_work_item.h"
-#include "chrome/installer/util/delete_old_versions.h"
 #include "chrome/installer/util/firewall_manager_win.h"
 #include "chrome/installer/util/google_update_constants.h"
 #include "chrome/installer/util/helper.h"
@@ -1009,12 +1008,6 @@ bool AppendPostInstallTasks(const InstallerState& installer_state,
     post_install_task_list->AddWorkItem(regular_update_work_items.release());
   }
 
-  // If this is a regular update (chrome.exe not in use), this step will try to
-  // delete files from all versions except the one that was just installed.
-  // Otherwise, it will try to delete files from all versions except the one
-  // that is in use and the one that was just installed.
-  AddDeleteOldVersionsWorkItem(installer_state, post_install_task_list);
-
   AddRegisterComDllWorkItemsForPackage(installer_state, current_version,
                                        new_version, post_install_task_list);
 
diff --git a/chrome/installer/setup/setup_main.cc b/chrome/installer/setup/setup_main.cc
index 1de8649..bab5bd5 100644
--- a/chrome/installer/setup/setup_main.cc
+++ b/chrome/installer/setup/setup_main.cc
@@ -53,7 +53,6 @@
 #include "chrome/installer/setup/uninstall.h"
 #include "chrome/installer/util/browser_distribution.h"
 #include "chrome/installer/util/delete_after_reboot_helper.h"
-#include "chrome/installer/util/delete_old_versions.h"
 #include "chrome/installer/util/delete_tree_work_item.h"
 #include "chrome/installer/util/google_update_constants.h"
 #include "chrome/installer/util/google_update_settings.h"
@@ -341,11 +340,11 @@ void AddExistingMultiInstalls(const InstallationState& original_state,
 
 // This function is called when --rename-chrome-exe option is specified on
 // setup.exe command line. This function assumes an in-use update has happened
-// for Chrome so there should be a file called new_chrome.exe on the file system
-// and a key called 'opv' in the registry. This function will move
+// for Chrome so there should be a file called new_chrome.exe on the file
+// system and a key called 'opv' in the registry. This function will move
 // new_chrome.exe to chrome.exe and delete 'opv' key in one atomic operation.
-// This function also takes care of deleting files and elevation policies that
-// belong to old versions of Chrome.
+// This function also deletes elevation policies associated with the old version
+// if they exist.
 installer::InstallStatus RenameChromeExecutables(
     const InstallationState& original_state,
     InstallerState* installer_state) {
@@ -405,10 +404,6 @@ installer::InstallStatus RenameChromeExecutables(
                                             KEY_WOW64_32KEY,
                                             google_update::kRegRenameCmdField);
   }
-
-  // Delete old versions.
-  AddDeleteOldVersionsWorkItem(*installer_state, install_list.get());
-
   installer::InstallStatus ret = installer::RENAME_SUCCESSFUL;
   if (!install_list->Do()) {
     LOG(ERROR) << "Renaming of executables failed. Rolling back any changes.";
diff --git a/chrome/installer/util/BUILD.gn b/chrome/installer/util/BUILD.gn
index 1581959..13ca030 100644
--- a/chrome/installer/util/BUILD.gn
+++ b/chrome/installer/util/BUILD.gn
@@ -159,8 +159,6 @@ static_library("with_no_strings") {
       "create_dir_work_item.h",
       "create_reg_key_work_item.cc",
       "create_reg_key_work_item.h",
-      "delete_old_versions.cc",
-      "delete_old_versions.h",
       "delete_reg_key_work_item.cc",
       "delete_reg_key_work_item.h",
       "delete_reg_value_work_item.cc",
@@ -293,7 +291,6 @@ if (is_win) {
       "create_dir_work_item_unittest.cc",
       "create_reg_key_work_item_unittest.cc",
       "delete_after_reboot_helper_unittest.cc",
-      "delete_old_versions_unittest.cc",
       "delete_reg_key_work_item_unittest.cc",
       "delete_reg_value_work_item_unittest.cc",
       "delete_tree_work_item_unittest.cc",
diff --git a/chrome/installer/util/delete_old_versions.cc b/chrome/installer/util/delete_old_versions.cc
deleted file mode 100644
index 222f510..0000000
--- a/chrome/installer/util/delete_old_versions.cc
+++ /dev/null
@@ -1,277 +0,0 @@
-// Copyright 2016 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 "chrome/installer/util/delete_old_versions.h"
-
-#include <map>
-#include <set>
-#include <vector>
-
-#include "base/bind.h"
-#include "base/file_version_info.h"
-#include "base/files/file.h"
-#include "base/files/file_enumerator.h"
-#include "base/files/file_path.h"
-#include "base/files/file_util.h"
-#include "base/logging.h"
-#include "base/memory/scoped_ptr.h"
-#include "base/stl_util.h"
-#include "base/strings/string16.h"
-#include "base/version.h"
-#include "chrome/installer/util/callback_work_item.h"
-#include "chrome/installer/util/installer_state.h"
-#include "chrome/installer/util/util_constants.h"
-#include "chrome/installer/util/work_item_list.h"
-
-namespace installer {
-
-namespace {
-
-using PathVector = std::vector<base::FilePath>;
-using DirectorySet = std::set<base::FilePath>;
-using ExecutableMap = std::map<base::FilePath, PathVector>;
-
-// Returns the name of the version directory for executable |exe_path|.
-base::FilePath GetExecutableVersionDirName(const base::FilePath& exe_path) {
-  scoped_ptr<FileVersionInfo> file_version_info(
-      FileVersionInfo::CreateFileVersionInfo(exe_path));
-  if (!file_version_info.get())
-    return base::FilePath();
-  return base::FilePath(file_version_info->file_version());
-}
-
-// Returns the names of the old version directories found in |install_dir|. The
-// directories named after the version of chrome.exe or new_chrome.exe are
-// excluded.
-DirectorySet GetOldVersionDirectories(const base::FilePath& install_dir) {
-  const base::FilePath new_chrome_exe_version_dir_name =
-      GetExecutableVersionDirName(install_dir.Append(kChromeNewExe));
-  const base::FilePath chrome_exe_version_dir_name =
-      GetExecutableVersionDirName(install_dir.Append(kChromeExe));
-
-  DirectorySet directories;
-  base::FileEnumerator enum_directories(install_dir, false,
-                                        base::FileEnumerator::DIRECTORIES);
-  for (base::FilePath directory_path = enum_directories.Next();
-       !directory_path.empty(); directory_path = enum_directories.Next()) {
-    const base::FilePath directory_name = directory_path.BaseName();
-    const base::Version version(directory_name.AsUTF8Unsafe());
-    const size_t kNumChromeVersionComponents = 4;
-    if (version.IsValid() &&
-        version.components().size() == kNumChromeVersionComponents &&
-        directory_name != new_chrome_exe_version_dir_name &&
-        directory_name != chrome_exe_version_dir_name) {
-      directories.insert(directory_name);
-    }
-  }
-  return directories;
-}
-
-// Returns a map where the keys are version directory names and values are paths
-// of old_chrome*.exe executables found in |install_dir|.
-ExecutableMap GetOldExecutables(const base::FilePath& install_dir) {
-  ExecutableMap executables;
-  base::FileEnumerator enum_executables(install_dir, false,
-                                        base::FileEnumerator::FILES,
-                                        FILE_PATH_LITERAL("old_chrome*.exe"));
-  for (base::FilePath exe_path = enum_executables.Next(); !exe_path.empty();
-       exe_path = enum_executables.Next()) {
-    executables[GetExecutableVersionDirName(exe_path)].push_back(exe_path);
-  }
-  return executables;
-}
-
-// Deletes directories that are in |directories| and don't have a matching
-// executable in |executables|. Returns false if any such directories could not
-// be deleted.
-bool DeleteDirectoriesWithoutMatchingExecutable(
-    const DirectorySet& directories,
-    const ExecutableMap& executables,
-    const base::FilePath& install_dir) {
-  bool success = true;
-  for (const base::FilePath& directory_name : directories) {
-    // Delete the directory if it doesn't have a matching executable.
-    if (!ContainsKey(executables, directory_name)) {
-      const base::FilePath directory_path = install_dir.Append(directory_name);
-      LOG(WARNING) << "Attempting to delete stray directory "
-                   << directory_path.value();
-      if (!base::DeleteFile(directory_path, true)) {
-        PLOG(ERROR) << "Failed to delete stray directory "
-                    << directory_path.value();
-        success = false;
-      }
-    }
-  }
-  return success;
-}
-
-// Deletes executables that are in |executables| and don't have a matching
-// directory in |directories|. Returns false if any such files could not be
-// deleted.
-bool DeleteExecutablesWithoutMatchingDirectory(
-    const DirectorySet& directories,
-    const ExecutableMap& executables) {
-  bool success = true;
-  for (const auto& version_and_executables : executables) {
-    const auto& version_dir_name = version_and_executables.first;
-    const auto& executables_for_version = version_and_executables.second;
-
-    // Don't delete the executables if they have a matching directory.
-    if (ContainsValue(directories, version_dir_name))
-      continue;
-
-    // Delete executables for version |version_dir_name|.
-    for (const auto& executable_path : executables_for_version) {
-      const base::FilePath executable_name = executable_path.BaseName();
-      LOG(WARNING) << "Attempting to delete stray executable "
-                   << executable_path.value();
-      if (!base::DeleteFile(executable_path, false)) {
-        PLOG(ERROR) << "Failed to delete stray executable "
-                    << executable_path.value();
-        success = false;
-      }
-    }
-  }
-  return success;
-}
-
-// Opens |path| with options that prevent the file from being read or written
-// via another handle. As long as the returned object is alive, it is guaranteed
-// that |path| isn't in use. It can however be deleted.
-base::File GetFileLock(const base::FilePath& path) {
-  return base::File(path, base::File::FLAG_OPEN | base::File::FLAG_READ |
-                              base::File::FLAG_EXCLUSIVE_READ |
-                              base::File::FLAG_EXCLUSIVE_WRITE |
-                              base::File::FLAG_SHARE_DELETE);
-}
-
-// Deletes |version_directory| and all executables in |version_executables| if
-// no .exe or .dll file for the version is in use. Returns false if any file
-// or directory for the version could not be deleted.
-bool DeleteVersion(const base::FilePath& version_directory,
-                   const PathVector& version_executables) {
-  std::vector<base::File> locks;
-  PathVector locked_file_paths;
-
-  // Lock .exe/.dll files in |version_directory|.
-  base::FileEnumerator enum_version_directory(version_directory, true,
-                                              base::FileEnumerator::FILES);
-  for (base::FilePath path = enum_version_directory.Next(); !path.empty();
-       path = enum_version_directory.Next()) {
-    if (!path.MatchesExtension(FILE_PATH_LITERAL(".exe")) &&
-        !path.MatchesExtension(FILE_PATH_LITERAL(".dll"))) {
-      continue;
-    }
-    locks.push_back(GetFileLock(path));
-    if (!locks.back().IsValid()) {
-      LOG(WARNING) << "Failed to delete old version "
-                   << version_directory.value() << " because " << path.value()
-                   << " is in use.";
-      return false;
-    }
-    locked_file_paths.push_back(path);
-  }
-
-  // Lock executables in |version_executables|.
-  for (const base::FilePath& executable_path : version_executables) {
-    locks.push_back(GetFileLock(executable_path));
-    if (!locks.back().IsValid()) {
-      LOG(WARNING) << "Failed to delete old version "
-                   << version_directory.value() << " because "
-                   << executable_path.value() << " is in use.";
-      return false;
-    }
-    locked_file_paths.push_back(executable_path);
-  }
-
-  bool success = true;
-
-  // Delete locked files. The files won't actually be deleted until the locks
-  // are released.
-  for (const base::FilePath& locked_file_path : locked_file_paths) {
-    if (!base::DeleteFile(locked_file_path, false)) {
-      PLOG(ERROR) << "Failed to delete locked file "
-                  << locked_file_path.value();
-      success = false;
-    }
-  }
-
-  // Release the locks, causing the locked files to actually be deleted. The
-  // version directory can't be deleted before this is done.
-  locks.clear();
-
-  // Delete the version directory.
-  if (!base::DeleteFile(version_directory, true)) {
-    PLOG(ERROR) << "Failed to delete version directory "
-                << version_directory.value();
-    success = false;
-  }
-
-  return success;
-}
-
-// For each executable in |executables| that has a matching directory in
-// |directories|, tries to delete the executable and the matching directory. No
-// deletion occurs for a given version if a .exe or .dll file for that version
-// is in use. Returns false if any directory/executables pair could not be
-// deleted.
-bool DeleteMatchingExecutablesAndDirectories(
-    const DirectorySet& directories,
-    const ExecutableMap& executables,
-    const base::FilePath& install_dir) {
-  bool success = true;
-  for (const auto& directory_name : directories) {
-    // Don't delete the version unless the directory has at least one matching
-    // executable.
-    auto version_executables_it = executables.find(directory_name);
-    if (version_executables_it == executables.end())
-      continue;
-
-    // Try to delete all files for the version.
-    success &= DeleteVersion(install_dir.Append(directory_name),
-                             version_executables_it->second);
-  }
-  return success;
-}
-
-bool DeleteOldVersions(const base::FilePath& install_dir) {
-  const DirectorySet old_directories = GetOldVersionDirectories(install_dir);
-  const ExecutableMap old_executables = GetOldExecutables(install_dir);
-
-  bool success = true;
-  success &= DeleteDirectoriesWithoutMatchingExecutable(
-      old_directories, old_executables, install_dir);
-  success &= DeleteExecutablesWithoutMatchingDirectory(old_directories,
-                                                       old_executables);
-  success &= DeleteMatchingExecutablesAndDirectories(
-      old_directories, old_executables, install_dir);
-
-  return success;
-}
-
-bool DeleteOldVersionsCallback(const InstallerState& installer_state,
-                               const CallbackWorkItem& work_item) {
-  // No rollback is possible for this action.
-  if (work_item.IsRollback())
-    return true;
-
-  installer_state.UpdateStage(REMOVING_OLD_VERSIONS);
-  return DeleteOldVersions(installer_state.target_path());
-}
-
-}  // namespace
-
-void AddDeleteOldVersionsWorkItem(const InstallerState& installer_state,
-                                  WorkItemList* work_item_list) {
-  DCHECK(work_item_list);
-
-  // TODO(fdoray): Launch a cleanup process when this fails during a regular
-  //               update or after a rename. crbug.com/451546
-  work_item_list
-      ->AddCallbackWorkItem(base::Bind(&DeleteOldVersionsCallback,
-                                       base::ConstRef(installer_state)))
-      ->set_ignore_failure(true);
-}
-
-}  // namespace installer
diff --git a/chrome/installer/util/delete_old_versions.h b/chrome/installer/util/delete_old_versions.h
deleted file mode 100644
index 098119b..0000000
--- a/chrome/installer/util/delete_old_versions.h
+++ /dev/null
@@ -1,24 +0,0 @@
-// Copyright 2016 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 CHROME_INSTALLER_UTIL_DELETE_OLD_VERSIONS_H_
-#define CHROME_INSTALLER_UTIL_DELETE_OLD_VERSIONS_H_
-
-class WorkItemList;
-
-namespace installer {
-
-class InstallerState;
-
-// Adds to |work_item_list| a WorkItem which deletes files that belong to old
-// versions of Chrome. chrome.exe, new_chrome.exe and their associated version
-// directories are never deleted. Also, no file is deleted for a given version
-// if a .exe or .dll file for that version is in use. |installer_state| must be
-// alive when |work_item_list| is executed.
-void AddDeleteOldVersionsWorkItem(const InstallerState& installer_state,
-                                  WorkItemList* work_item_list);
-
-}  // namespace installer
-
-#endif  // CHROME_INSTALLER_UTIL_DELETE_OLD_VERSIONS_H_
diff --git a/chrome/installer/util/delete_old_versions_unittest.cc b/chrome/installer/util/delete_old_versions_unittest.cc
deleted file mode 100644
index 7a9a58e..0000000
--- a/chrome/installer/util/delete_old_versions_unittest.cc
+++ /dev/null
@@ -1,286 +0,0 @@
-// Copyright 2016 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 "chrome/installer/util/delete_old_versions.h"
-
-#include <set>
-
-#include "base/files/file.h"
-#include "base/files/file_enumerator.h"
-#include "base/files/file_util.h"
-#include "base/files/scoped_temp_dir.h"
-#include "base/logging.h"
-#include "base/macros.h"
-#include "base/memory/scoped_ptr.h"
-#include "base/path_service.h"
-#include "base/strings/string16.h"
-#include "base/strings/string_util.h"
-#include "base/strings/utf_string_conversions.h"
-#include "base/version.h"
-#include "chrome/installer/test/alternate_version_generator.h"
-#include "chrome/installer/util/installer_state.h"
-#include "chrome/installer/util/util_constants.h"
-#include "chrome/installer/util/work_item.h"
-#include "chrome/installer/util/work_item_list.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace installer {
-
-namespace {
-
-class MockInstallerState : public InstallerState {
- public:
-  explicit MockInstallerState(const base::FilePath& target_path)
-      : InstallerState(InstallerState::USER_LEVEL) {
-    target_path_ = target_path;
-  }
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(MockInstallerState);
-};
-
-const base::char16 kVersionA[] = L"47.0.0.0";
-const base::char16 kVersionB[] = L"48.0.0.0";
-
-class DeleteOldVersionsTest : public testing::Test {
- protected:
-  DeleteOldVersionsTest() = default;
-
-  void DeleteOldVersions() {
-    MockInstallerState installer_state(install_dir());
-    scoped_ptr<WorkItemList> work_item_list(WorkItem::CreateWorkItemList());
-    AddDeleteOldVersionsWorkItem(installer_state, work_item_list.get());
-    work_item_list->Do();
-  }
-
-  bool CreateInstallDir() { return install_dir_.CreateUniqueTempDir(); }
-
-  // Creates an executable with |name| and |version| in |install_dir_|.
-  bool CreateExecutable(const base::string16& name,
-                        const base::string16& version) {
-    base::FilePath current_exe_path;
-    return base::PathService::Get(base::FILE_EXE, &current_exe_path) &&
-           upgrade_test::GenerateSpecificPEFileVersion(
-               current_exe_path, install_dir().Append(name),
-               base::Version(base::UTF16ToUTF8(version)));
-  }
-
-  // Creates a version directory named |name| in |install_dir_|.
-  bool CreateVersionDirectory(const base::string16& name) {
-    static const char kDummyContent[] = "dummy";
-    const base::FilePath version_dir_path(install_dir().Append(name));
-
-    return base::CreateDirectory(install_dir().Append(name)) &&
-           base::CreateDirectory(version_dir_path.Append(L"Installer")) &&
-           base::WriteFile(version_dir_path.Append(L"chrome.dll"),
-                           kDummyContent, sizeof(kDummyContent)) &&
-           base::WriteFile(version_dir_path.Append(L"nacl64.exe"),
-                           kDummyContent, sizeof(kDummyContent)) &&
-           base::WriteFile(version_dir_path.Append(L"icudtl.dat"),
-                           kDummyContent, sizeof(kDummyContent)) &&
-           base::WriteFile(version_dir_path.Append(L"Installer\\setup.exe"),
-                           kDummyContent, sizeof(kDummyContent));
-  }
-
-  // Returns the relative paths of all files and directories in |install_dir_|.
-  using FilePathSet = std::set<base::FilePath>;
-  FilePathSet GetInstallDirContent() const {
-    std::set<base::FilePath> content;
-    base::FileEnumerator file_enumerator(
-        install_dir(), true,
-        base::FileEnumerator::FILES | base::FileEnumerator::DIRECTORIES);
-    for (base::FilePath path = file_enumerator.Next(); !path.empty();
-         path = file_enumerator.Next()) {
-      DCHECK(base::StartsWith(path.value(), install_dir().value(),
-                              base::CompareCase::SENSITIVE));
-      content.insert(base::FilePath(
-          path.value().substr(install_dir().value().size() + 1)));
-    }
-    return content;
-  }
-
-  // Adds to |file_path_set| all files and directories that are expected to be
-  // found in the version directory |version| before any attempt to delete it.
-  void AddVersionFiles(const base::string16& version,
-                       FilePathSet* file_path_set) {
-    file_path_set->insert(base::FilePath(version));
-    file_path_set->insert(base::FilePath(version).Append(L"chrome.dll"));
-    file_path_set->insert(base::FilePath(version).Append(L"nacl64.exe"));
-    file_path_set->insert(base::FilePath(version).Append(L"icudtl.dat"));
-    file_path_set->insert(base::FilePath(version).Append(L"Installer"));
-    file_path_set->insert(
-        base::FilePath(version).Append(L"Installer\\setup.exe"));
-  }
-
-  base::FilePath install_dir() const { return install_dir_.path(); }
-
- private:
-  base::ScopedTempDir install_dir_;
-
-  DISALLOW_COPY_AND_ASSIGN(DeleteOldVersionsTest);
-};
-
-}  // namespace
-
-// An old executable without a matching directory should be deleted.
-TEST_F(DeleteOldVersionsTest, DeleteOldExecutableWithoutMatchingDirectory) {
-  ASSERT_TRUE(CreateInstallDir());
-  ASSERT_TRUE(CreateExecutable(installer::kChromeOldExe, kVersionA));
-
-  DeleteOldVersions();
-  EXPECT_TRUE(GetInstallDirContent().empty());
-}
-
-// chrome.exe and new_chrome.exe should never be deleted.
-TEST_F(DeleteOldVersionsTest, DeleteNewExecutablesWithoutMatchingDirectory) {
-  ASSERT_TRUE(CreateInstallDir());
-  ASSERT_TRUE(CreateExecutable(installer::kChromeExe, kVersionA));
-  ASSERT_TRUE(CreateExecutable(installer::kChromeNewExe, kVersionB));
-
-  DeleteOldVersions();
-  FilePathSet expected_install_dir_content;
-  expected_install_dir_content.insert(base::FilePath(installer::kChromeExe));
-  expected_install_dir_content.insert(base::FilePath(installer::kChromeNewExe));
-  EXPECT_EQ(expected_install_dir_content, GetInstallDirContent());
-}
-
-// A directory without a matching executable should be deleted.
-TEST_F(DeleteOldVersionsTest, DeleteDirectoryWithoutMatchingExecutable) {
-  ASSERT_TRUE(CreateInstallDir());
-  ASSERT_TRUE(CreateVersionDirectory(kVersionA));
-
-  DeleteOldVersions();
-  EXPECT_TRUE(GetInstallDirContent().empty());
-}
-
-// A pair of matching old executable/version directory that is not in use should
-// be deleted.
-TEST_F(DeleteOldVersionsTest, DeleteOldExecutableWithMatchingDirectory) {
-  ASSERT_TRUE(CreateInstallDir());
-  ASSERT_TRUE(CreateExecutable(installer::kChromeOldExe, kVersionA));
-  ASSERT_TRUE(CreateVersionDirectory(kVersionA));
-
-  DeleteOldVersions();
-  EXPECT_TRUE(GetInstallDirContent().empty());
-}
-
-// chrome.exe, new_chrome.exe and their matching version directories should
-// never be deleted.
-TEST_F(DeleteOldVersionsTest, DeleteNewExecutablesWithMatchingDirectory) {
-  ASSERT_TRUE(CreateInstallDir());
-  ASSERT_TRUE(CreateExecutable(installer::kChromeExe, kVersionA));
-  ASSERT_TRUE(CreateVersionDirectory(kVersionA));
-  ASSERT_TRUE(CreateExecutable(installer::kChromeNewExe, kVersionB));
-  ASSERT_TRUE(CreateVersionDirectory(kVersionB));
-
-  DeleteOldVersions();
-
-  FilePathSet expected_install_dir_content;
-  expected_install_dir_content.insert(base::FilePath(installer::kChromeExe));
-  AddVersionFiles(kVersionA, &expected_install_dir_content);
-  expected_install_dir_content.insert(base::FilePath(installer::kChromeNewExe));
-  AddVersionFiles(kVersionB, &expected_install_dir_content);
-  EXPECT_EQ(expected_install_dir_content, GetInstallDirContent());
-}
-
-// chrome.exe, new_chrome.exe and their matching version directories should
-// never be deleted, even when files named old_chrome*.exe have the same
-// versions as chrome.exe/new_chrome.exe. The old_chrome*.exe files, however,
-// should be deleted.
-TEST_F(DeleteOldVersionsTest,
-       DeleteNewExecutablesWithMatchingDirectoryAndOldExecutables) {
-  ASSERT_TRUE(CreateInstallDir());
-  ASSERT_TRUE(CreateExecutable(installer::kChromeExe, kVersionA));
-  ASSERT_TRUE(CreateVersionDirectory(kVersionA));
-  ASSERT_TRUE(CreateExecutable(installer::kChromeNewExe, kVersionB));
-  ASSERT_TRUE(CreateVersionDirectory(kVersionB));
-  ASSERT_TRUE(CreateExecutable(L"old_chrome.exe", kVersionA));
-  ASSERT_TRUE(CreateExecutable(L"old_chrome2.exe", kVersionB));
-
-  DeleteOldVersions();
-
-  FilePathSet expected_install_dir_content;
-  expected_install_dir_content.insert(base::FilePath(installer::kChromeExe));
-  AddVersionFiles(kVersionA, &expected_install_dir_content);
-  expected_install_dir_content.insert(base::FilePath(installer::kChromeNewExe));
-  AddVersionFiles(kVersionB, &expected_install_dir_content);
-  EXPECT_EQ(expected_install_dir_content, GetInstallDirContent());
-}
-
-// No file should be deleted for a given version if the executable is in use.
-TEST_F(DeleteOldVersionsTest, DeleteVersionWithExecutableInUse) {
-  ASSERT_TRUE(CreateInstallDir());
-  ASSERT_TRUE(CreateExecutable(installer::kChromeOldExe, kVersionA));
-  ASSERT_TRUE(CreateVersionDirectory(kVersionA));
-
-  base::File file_in_use(install_dir().Append(installer::kChromeOldExe),
-                         base::File::FLAG_OPEN | base::File::FLAG_READ);
-  ASSERT_TRUE(file_in_use.IsValid());
-
-  DeleteOldVersions();
-
-  FilePathSet expected_install_dir_content;
-  expected_install_dir_content.insert(base::FilePath(installer::kChromeOldExe));
-  AddVersionFiles(kVersionA, &expected_install_dir_content);
-  EXPECT_EQ(expected_install_dir_content, GetInstallDirContent());
-}
-
-// No file should be deleted for a given version if a .dll file in the version
-// directory is in use.
-TEST_F(DeleteOldVersionsTest, DeleteVersionWithVersionDirectoryDllInUse) {
-  ASSERT_TRUE(CreateInstallDir());
-  ASSERT_TRUE(CreateExecutable(installer::kChromeOldExe, kVersionA));
-  ASSERT_TRUE(CreateVersionDirectory(kVersionA));
-
-  base::File file_in_use(install_dir().Append(kVersionA).Append(L"chrome.dll"),
-                         base::File::FLAG_OPEN | base::File::FLAG_READ);
-  ASSERT_TRUE(file_in_use.IsValid());
-
-  DeleteOldVersions();
-
-  FilePathSet expected_install_dir_content;
-  expected_install_dir_content.insert(base::FilePath(installer::kChromeOldExe));
-  AddVersionFiles(kVersionA, &expected_install_dir_content);
-  EXPECT_EQ(expected_install_dir_content, GetInstallDirContent());
-}
-
-// No file should be deleted for a given version if a .exe file in the version
-// directory is in use.
-TEST_F(DeleteOldVersionsTest, DeleteVersionWithVersionDirectoryExeInUse) {
-  ASSERT_TRUE(CreateInstallDir());
-  ASSERT_TRUE(CreateExecutable(installer::kChromeOldExe, kVersionA));
-  ASSERT_TRUE(CreateVersionDirectory(kVersionA));
-
-  base::File file_in_use(
-      install_dir().Append(kVersionA).Append(L"Installer\\setup.exe"),
-      base::File::FLAG_OPEN | base::File::FLAG_READ);
-  ASSERT_TRUE(file_in_use.IsValid());
-
-  DeleteOldVersions();
-
-  FilePathSet expected_install_dir_content;
-  expected_install_dir_content.insert(base::FilePath(installer::kChromeOldExe));
-  AddVersionFiles(kVersionA, &expected_install_dir_content);
-  EXPECT_EQ(expected_install_dir_content, GetInstallDirContent());
-}
-
-// If an installation directory contains a file named chrome.exe with a matching
-// directory v1 and a file name old_chrome.exe with a matching directory v2,
-// old_chrome.exe and v2 should be deleted but chrome.exe and v1 shouldn't.
-TEST_F(DeleteOldVersionsTest, TypicalAfterRenameState) {
-  ASSERT_TRUE(CreateInstallDir());
-  ASSERT_TRUE(CreateExecutable(installer::kChromeOldExe, kVersionA));
-  ASSERT_TRUE(CreateVersionDirectory(kVersionA));
-  ASSERT_TRUE(CreateExecutable(installer::kChromeExe, kVersionB));
-  ASSERT_TRUE(CreateVersionDirectory(kVersionB));
-
-  DeleteOldVersions();
-
-  FilePathSet expected_install_dir_content;
-  expected_install_dir_content.insert(base::FilePath(installer::kChromeExe));
-  AddVersionFiles(kVersionB, &expected_install_dir_content);
-  EXPECT_EQ(expected_install_dir_content, GetInstallDirContent());
-}
-
-}  // namespace installer
diff --git a/chrome/installer/util/installer_state.cc b/chrome/installer/util/installer_state.cc
index f2d4eb2..3da5a99 100644
--- a/chrome/installer/util/installer_state.cc
+++ b/chrome/installer/util/installer_state.cc
@@ -11,6 +11,8 @@
 #include <utility>
 
 #include "base/command_line.h"
+#include "base/file_version_info.h"
+#include "base/files/file_enumerator.h"
 #include "base/files/file_util.h"
 #include "base/logging.h"
 #include "base/macros.h"
@@ -512,6 +514,69 @@ bool InstallerState::AnyExistsAndIsInUse(const InstallationState& machine_state,
   return false;
 }
 
+void InstallerState::GetExistingExeVersions(
+    std::set<std::string>* existing_versions) const {
+
+  static const wchar_t* const kChromeFilenames[] = {
+    installer::kChromeExe,
+    installer::kChromeNewExe,
+    installer::kChromeOldExe,
+  };
+
+  for (size_t i = 0; i < arraysize(kChromeFilenames); ++i) {
+    base::FilePath chrome_exe(target_path().Append(kChromeFilenames[i]));
+    scoped_ptr<FileVersionInfo> file_version_info(
+        FileVersionInfo::CreateFileVersionInfo(chrome_exe));
+    if (file_version_info) {
+      base::string16 version_string = file_version_info->file_version();
+      if (!version_string.empty() && base::IsStringASCII(version_string))
+        existing_versions->insert(base::UTF16ToASCII(version_string));
+    }
+  }
+}
+
+void InstallerState::RemoveOldVersionDirectories(
+    const Version& new_version,
+    Version* existing_version,
+    const base::FilePath& temp_path) const {
+  Version version;
+  scoped_ptr<WorkItem> item;
+
+  std::set<std::string> existing_version_strings;
+  existing_version_strings.insert(new_version.GetString());
+  if (existing_version)
+    existing_version_strings.insert(existing_version->GetString());
+
+  // Make sure not to delete any version dir that is "referenced" by an existing
+  // Chrome executable.
+  GetExistingExeVersions(&existing_version_strings);
+
+  // Try to delete all directories that are not in the set we care to keep.
+  base::FileEnumerator version_enum(target_path(), false,
+                                    base::FileEnumerator::DIRECTORIES);
+  for (base::FilePath next_version = version_enum.Next(); !next_version.empty();
+       next_version = version_enum.Next()) {
+    base::FilePath dir_name(next_version.BaseName());
+    version = Version(base::UTF16ToASCII(dir_name.value()));
+    // Delete the version folder if it is less than the new version and not
+    // equal to the old version (if we have an old version).
+    if (version.IsValid() &&
+        existing_version_strings.count(version.GetString()) == 0) {
+      // Note: temporarily log old version deletion at ERROR level to make it
+      // more likely we see this in the installer log.
+      LOG(ERROR) << "Deleting old version directory: " << next_version.value();
+
+      // Attempt to recursively delete the old version dir.
+      bool delete_succeeded = base::DeleteFile(next_version, true);
+
+      // Note: temporarily log old version deletion at ERROR level to make it
+      // more likely we see this in the installer log.
+      LOG_IF(ERROR, !delete_succeeded)
+          << "Failed to delete old version directory: " << next_version.value();
+    }
+  }
+}
+
 void InstallerState::AddComDllList(
     std::vector<base::FilePath>* com_dll_list) const {
   for (auto* product : products_)
diff --git a/chrome/installer/util/installer_state.h b/chrome/installer/util/installer_state.h
index 1de3000..2b62465 100644
--- a/chrome/installer/util/installer_state.h
+++ b/chrome/installer/util/installer_state.h
@@ -7,6 +7,7 @@
 
 #include <stdint.h>
 
+#include <set>
 #include <string>
 #include <vector>
 
@@ -177,6 +178,13 @@ class InstallerState {
   // (for example <target_path>\Google\Chrome\Application\<Version>\Installer)
   base::FilePath GetInstallerDirectory(const base::Version& version) const;
 
+  // Try to delete all directories under |temp_path| whose versions are less
+  // than |new_version| and not equal to |existing_version|. |existing_version|
+  // may be NULL.
+  void RemoveOldVersionDirectories(const base::Version& new_version,
+                                   base::Version* existing_version,
+                                   const base::FilePath& temp_path) const;
+
   // Adds to |com_dll_list| the list of COM DLLs that are to be registered
   // and/or unregistered. The list may be empty.
   void AddComDllList(std::vector<base::FilePath>* com_dll_list) const;
@@ -235,6 +243,11 @@ class InstallerState {
   bool IsMultiInstallUpdate(const MasterPreferences& prefs,
                             const InstallationState& machine_state);
 
+  // Enumerates all files named one of
+  // [chrome.exe, old_chrome.exe, new_chrome.exe] in target_path_ and
+  // returns their version numbers in a set.
+  void GetExistingExeVersions(std::set<std::string>* existing_versions) const;
+
   // Sets this object's level and updates the root_key_ accordingly.
   void set_level(Level level);
 
diff --git a/chrome/installer/util/installer_state_unittest.cc b/chrome/installer/util/installer_state_unittest.cc
index 91d8ee3..1ced49e 100644
--- a/chrome/installer/util/installer_state_unittest.cc
+++ b/chrome/installer/util/installer_state_unittest.cc
@@ -11,6 +11,7 @@
 
 #include "base/base_paths.h"
 #include "base/command_line.h"
+#include "base/files/file_enumerator.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
@@ -69,6 +70,9 @@ class MockInstallerState : public InstallerState {
   const Version& critical_update_version() const {
     return critical_update_version_;
   }
+  void GetExistingExeVersions(std::set<std::string>* existing_version_strings) {
+    return InstallerState::GetExistingExeVersions(existing_version_strings);
+  }
 };
 
 // Simple function to dump some text into a new file.
@@ -93,6 +97,249 @@ void BuildSingleChromeState(const base::FilePath& target_dir,
       != NULL);
 }
 
+wchar_t text_content_1[] = L"delete me";
+wchar_t text_content_2[] = L"delete me as well";
+
+// Delete version directories. Everything lower than the given version
+// should be deleted.
+TEST_F(InstallerStateTest, Delete) {
+  // TODO(grt): move common stuff into the test fixture.
+  // Create a Chrome dir
+  base::FilePath chrome_dir(test_dir_.path());
+  chrome_dir = chrome_dir.AppendASCII("chrome");
+  base::CreateDirectory(chrome_dir);
+  ASSERT_TRUE(base::PathExists(chrome_dir));
+
+  base::FilePath chrome_dir_1(chrome_dir);
+  chrome_dir_1 = chrome_dir_1.AppendASCII("1.0.1.0");
+  base::CreateDirectory(chrome_dir_1);
+  ASSERT_TRUE(base::PathExists(chrome_dir_1));
+
+  base::FilePath chrome_dir_2(chrome_dir);
+  chrome_dir_2 = chrome_dir_2.AppendASCII("1.0.2.0");
+  base::CreateDirectory(chrome_dir_2);
+  ASSERT_TRUE(base::PathExists(chrome_dir_2));
+
+  base::FilePath chrome_dir_3(chrome_dir);
+  chrome_dir_3 = chrome_dir_3.AppendASCII("1.0.3.0");
+  base::CreateDirectory(chrome_dir_3);
+  ASSERT_TRUE(base::PathExists(chrome_dir_3));
+
+  base::FilePath chrome_dir_4(chrome_dir);
+  chrome_dir_4 = chrome_dir_4.AppendASCII("1.0.4.0");
+  base::CreateDirectory(chrome_dir_4);
+  ASSERT_TRUE(base::PathExists(chrome_dir_4));
+
+  base::FilePath chrome_dll_1(chrome_dir_1);
+  chrome_dll_1 = chrome_dll_1.AppendASCII("chrome.dll");
+  CreateTextFile(chrome_dll_1.value(), text_content_1);
+  ASSERT_TRUE(base::PathExists(chrome_dll_1));
+
+  base::FilePath chrome_dll_2(chrome_dir_2);
+  chrome_dll_2 = chrome_dll_2.AppendASCII("chrome.dll");
+  CreateTextFile(chrome_dll_2.value(), text_content_1);
+  ASSERT_TRUE(base::PathExists(chrome_dll_2));
+
+  base::FilePath chrome_dll_3(chrome_dir_3);
+  chrome_dll_3 = chrome_dll_3.AppendASCII("chrome.dll");
+  CreateTextFile(chrome_dll_3.value(), text_content_1);
+  ASSERT_TRUE(base::PathExists(chrome_dll_3));
+
+  base::FilePath chrome_dll_4(chrome_dir_4);
+  chrome_dll_4 = chrome_dll_4.AppendASCII("chrome.dll");
+  CreateTextFile(chrome_dll_4.value(), text_content_1);
+  ASSERT_TRUE(base::PathExists(chrome_dll_4));
+
+  MockInstallerState installer_state;
+  BuildSingleChromeState(chrome_dir, &installer_state);
+  Version latest_version("1.0.4.0");
+  {
+    base::ScopedTempDir temp_dir;
+    ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+    installer_state.RemoveOldVersionDirectories(latest_version, NULL,
+                                                temp_dir.path());
+  }
+
+  // old versions should be gone
+  EXPECT_FALSE(base::PathExists(chrome_dir_1));
+  EXPECT_FALSE(base::PathExists(chrome_dir_2));
+  EXPECT_FALSE(base::PathExists(chrome_dir_3));
+  // the latest version should stay
+  EXPECT_TRUE(base::PathExists(chrome_dll_4));
+}
+
+// Delete older version directories, keeping the one in used intact.
+TEST_F(InstallerStateTest, DeleteInUsed) {
+  // Create a Chrome dir
+  base::FilePath chrome_dir(test_dir_.path());
+  chrome_dir = chrome_dir.AppendASCII("chrome");
+  base::CreateDirectory(chrome_dir);
+  ASSERT_TRUE(base::PathExists(chrome_dir));
+
+  base::FilePath chrome_dir_1(chrome_dir);
+  chrome_dir_1 = chrome_dir_1.AppendASCII("1.0.1.0");
+  base::CreateDirectory(chrome_dir_1);
+  ASSERT_TRUE(base::PathExists(chrome_dir_1));
+
+  base::FilePath chrome_dir_2(chrome_dir);
+  chrome_dir_2 = chrome_dir_2.AppendASCII("1.0.2.0");
+  base::CreateDirectory(chrome_dir_2);
+  ASSERT_TRUE(base::PathExists(chrome_dir_2));
+
+  base::FilePath chrome_dir_3(chrome_dir);
+  chrome_dir_3 = chrome_dir_3.AppendASCII("1.0.3.0");
+  base::CreateDirectory(chrome_dir_3);
+  ASSERT_TRUE(base::PathExists(chrome_dir_3));
+
+  base::FilePath chrome_dir_4(chrome_dir);
+  chrome_dir_4 = chrome_dir_4.AppendASCII("1.0.4.0");
+  base::CreateDirectory(chrome_dir_4);
+  ASSERT_TRUE(base::PathExists(chrome_dir_4));
+
+  base::FilePath chrome_dll_1(chrome_dir_1);
+  chrome_dll_1 = chrome_dll_1.AppendASCII("chrome.dll");
+  CreateTextFile(chrome_dll_1.value(), text_content_1);
+  ASSERT_TRUE(base::PathExists(chrome_dll_1));
+
+  base::FilePath chrome_dll_2(chrome_dir_2);
+  chrome_dll_2 = chrome_dll_2.AppendASCII("chrome.dll");
+  CreateTextFile(chrome_dll_2.value(), text_content_1);
+  ASSERT_TRUE(base::PathExists(chrome_dll_2));
+
+  // Open the file to make it in use.
+  std::ofstream file;
+  file.open(chrome_dll_2.value().c_str());
+
+  base::FilePath chrome_othera_2(chrome_dir_2);
+  chrome_othera_2 = chrome_othera_2.AppendASCII("othera.dll");
+  CreateTextFile(chrome_othera_2.value(), text_content_2);
+  ASSERT_TRUE(base::PathExists(chrome_othera_2));
+
+  base::FilePath chrome_otherb_2(chrome_dir_2);
+  chrome_otherb_2 = chrome_otherb_2.AppendASCII("otherb.dll");
+  CreateTextFile(chrome_otherb_2.value(), text_content_2);
+  ASSERT_TRUE(base::PathExists(chrome_otherb_2));
+
+  base::FilePath chrome_dll_3(chrome_dir_3);
+  chrome_dll_3 = chrome_dll_3.AppendASCII("chrome.dll");
+  CreateTextFile(chrome_dll_3.value(), text_content_1);
+  ASSERT_TRUE(base::PathExists(chrome_dll_3));
+
+  base::FilePath chrome_dll_4(chrome_dir_4);
+  chrome_dll_4 = chrome_dll_4.AppendASCII("chrome.dll");
+  CreateTextFile(chrome_dll_4.value(), text_content_1);
+  ASSERT_TRUE(base::PathExists(chrome_dll_4));
+
+  MockInstallerState installer_state;
+  BuildSingleChromeState(chrome_dir, &installer_state);
+  Version latest_version("1.0.4.0");
+  Version existing_version("1.0.1.0");
+  {
+    base::ScopedTempDir temp_dir;
+    ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+    installer_state.RemoveOldVersionDirectories(latest_version,
+                                                &existing_version,
+                                                temp_dir.path());
+  }
+
+  // the version defined as the existing version should stay
+  EXPECT_TRUE(base::PathExists(chrome_dir_1));
+  // old versions not in used should be gone
+  EXPECT_FALSE(base::PathExists(chrome_dir_3));
+  // every thing under in used version should stay
+  EXPECT_TRUE(base::PathExists(chrome_dir_2));
+  EXPECT_TRUE(base::PathExists(chrome_dll_2));
+  EXPECT_TRUE(base::PathExists(chrome_othera_2));
+  EXPECT_TRUE(base::PathExists(chrome_otherb_2));
+  // the latest version should stay
+  EXPECT_TRUE(base::PathExists(chrome_dll_4));
+}
+
+// Tests a few basic things of the Package class.  Makes sure that the path
+// operations are correct
+TEST_F(InstallerStateTest, Basic) {
+  const bool multi_install = false;
+  const bool system_level = true;
+  base::CommandLine cmd_line = base::CommandLine::FromString(
+      std::wstring(L"setup.exe") +
+      (multi_install ? L" --multi-install --chrome" : L"") +
+      (system_level ? L" --system-level" : L""));
+  MasterPreferences prefs(cmd_line);
+  InstallationState machine_state;
+  machine_state.Initialize();
+  MockInstallerState installer_state;
+  installer_state.Initialize(cmd_line, prefs, machine_state);
+  installer_state.set_target_path(test_dir_.path());
+  EXPECT_EQ(test_dir_.path().value(), installer_state.target_path().value());
+  EXPECT_EQ(1U, installer_state.products().size());
+
+  const char kOldVersion[] = "1.2.3.4";
+  const char kNewVersion[] = "2.3.4.5";
+
+  Version new_version(kNewVersion);
+  Version old_version(kOldVersion);
+  ASSERT_TRUE(new_version.IsValid());
+  ASSERT_TRUE(old_version.IsValid());
+
+  base::FilePath installer_dir(
+      installer_state.GetInstallerDirectory(new_version));
+  EXPECT_FALSE(installer_dir.empty());
+
+  base::FilePath new_version_dir(installer_state.target_path().Append(
+      base::UTF8ToWide(new_version.GetString())));
+  base::FilePath old_version_dir(installer_state.target_path().Append(
+      base::UTF8ToWide(old_version.GetString())));
+
+  EXPECT_FALSE(base::PathExists(new_version_dir));
+  EXPECT_FALSE(base::PathExists(old_version_dir));
+
+  EXPECT_FALSE(base::PathExists(installer_dir));
+  base::CreateDirectory(installer_dir);
+  EXPECT_TRUE(base::PathExists(new_version_dir));
+
+  base::CreateDirectory(old_version_dir);
+  EXPECT_TRUE(base::PathExists(old_version_dir));
+
+  // Create a fake chrome.dll key file in the old version directory.  This
+  // should prevent the old version directory from getting deleted.
+  base::FilePath old_chrome_dll(old_version_dir.Append(installer::kChromeDll));
+  EXPECT_FALSE(base::PathExists(old_chrome_dll));
+
+  // Hold on to the file exclusively to prevent the directory from
+  // being deleted.
+  base::win::ScopedHandle file(
+    ::CreateFile(old_chrome_dll.value().c_str(), GENERIC_READ,
+                 0, NULL, OPEN_ALWAYS, 0, NULL));
+  EXPECT_TRUE(file.IsValid());
+  EXPECT_TRUE(base::PathExists(old_chrome_dll));
+
+  base::ScopedTempDir temp_dir;
+  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+
+  // Don't explicitly tell the directory cleanup logic not to delete the
+  // old version, rely on the key files to keep it around.
+  installer_state.RemoveOldVersionDirectories(new_version,
+                                              NULL,
+                                              temp_dir.path());
+
+  // The old directory should still exist.
+  EXPECT_TRUE(base::PathExists(old_version_dir));
+  EXPECT_TRUE(base::PathExists(new_version_dir));
+
+  // Now close the file handle to make it possible to delete our key file.
+  file.Close();
+
+  installer_state.RemoveOldVersionDirectories(new_version,
+                                              NULL,
+                                              temp_dir.path());
+  // The new directory should still exist.
+  EXPECT_TRUE(base::PathExists(new_version_dir));
+
+  // Now, the old directory and key file should be gone.
+  EXPECT_FALSE(base::PathExists(old_chrome_dll));
+  EXPECT_FALSE(base::PathExists(old_version_dir));
+}
+
 TEST_F(InstallerStateTest, WithProduct) {
   const bool multi_install = false;
   const bool system_level = true;
@@ -260,6 +507,112 @@ TEST_F(InstallerStateTest, IsFileInUse) {
   EXPECT_FALSE(MockInstallerState::IsFileInUse(temp_file));
 }
 
+TEST_F(InstallerStateTest, RemoveOldVersionDirs) {
+  MockInstallerState installer_state;
+  installer_state.set_target_path(test_dir_.path());
+  EXPECT_EQ(test_dir_.path().value(), installer_state.target_path().value());
+
+  const char kOldVersion[] = "2.0.0.0";
+  const char kNewVersion[] = "3.0.0.0";
+  const char kOldChromeExeVersion[] = "2.1.0.0";
+  const char kChromeExeVersion[] = "2.1.1.1";
+  const char kNewChromeExeVersion[] = "3.0.0.0";
+
+  Version new_version(kNewVersion);
+  Version old_version(kOldVersion);
+  Version old_chrome_exe_version(kOldChromeExeVersion);
+  Version chrome_exe_version(kChromeExeVersion);
+  Version new_chrome_exe_version(kNewChromeExeVersion);
+
+  ASSERT_TRUE(new_version.IsValid());
+  ASSERT_TRUE(old_version.IsValid());
+  ASSERT_TRUE(old_chrome_exe_version.IsValid());
+  ASSERT_TRUE(chrome_exe_version.IsValid());
+  ASSERT_TRUE(new_chrome_exe_version.IsValid());
+
+  // Set up a bunch of version dir paths.
+  base::FilePath version_dirs[] = {
+    installer_state.target_path().Append(L"1.2.3.4"),
+    installer_state.target_path().Append(L"1.2.3.5"),
+    installer_state.target_path().Append(L"1.2.3.6"),
+    installer_state.target_path().AppendASCII(kOldVersion),
+    installer_state.target_path().AppendASCII(kOldChromeExeVersion),
+    installer_state.target_path().Append(L"2.1.1.0"),
+    installer_state.target_path().AppendASCII(kChromeExeVersion),
+    installer_state.target_path().AppendASCII(kNewVersion),
+    installer_state.target_path().Append(L"3.9.1.1"),
+  };
+
+  // Create the version directories.
+  for (size_t i = 0; i < arraysize(version_dirs); i++) {
+    base::CreateDirectory(version_dirs[i]);
+    EXPECT_TRUE(base::PathExists(version_dirs[i]));
+  }
+
+  // Create exes with the appropriate version resource.
+  // Use the current test exe as a baseline.
+  base::FilePath exe_path;
+  ASSERT_TRUE(PathService::Get(base::FILE_EXE, &exe_path));
+
+  struct target_info {
+    base::FilePath target_file;
+    const Version& target_version;
+  } targets[] = {
+    { installer_state.target_path().Append(installer::kChromeOldExe),
+      old_chrome_exe_version },
+    { installer_state.target_path().Append(installer::kChromeExe),
+      chrome_exe_version },
+    { installer_state.target_path().Append(installer::kChromeNewExe),
+      new_chrome_exe_version },
+  };
+  for (size_t i = 0; i < arraysize(targets); ++i) {
+    ASSERT_TRUE(upgrade_test::GenerateSpecificPEFileVersion(
+        exe_path, targets[i].target_file, targets[i].target_version));
+  }
+
+  // Call GetExistingExeVersions, validate that picks up the
+  // exe resources.
+  std::set<std::string> expected_exe_versions;
+  expected_exe_versions.insert(kOldChromeExeVersion);
+  expected_exe_versions.insert(kChromeExeVersion);
+  expected_exe_versions.insert(kNewChromeExeVersion);
+
+  std::set<std::string> actual_exe_versions;
+  installer_state.GetExistingExeVersions(&actual_exe_versions);
+  EXPECT_EQ(expected_exe_versions, actual_exe_versions);
+
+  // Call RemoveOldVersionDirectories
+  installer_state.RemoveOldVersionDirectories(new_version,
+                                              &old_version,
+                                              installer_state.target_path());
+
+  // What we expect to have left.
+  std::set<std::string> expected_remaining_dirs;
+  expected_remaining_dirs.insert(kOldVersion);
+  expected_remaining_dirs.insert(kNewVersion);
+  expected_remaining_dirs.insert(kOldChromeExeVersion);
+  expected_remaining_dirs.insert(kChromeExeVersion);
+  expected_remaining_dirs.insert(kNewChromeExeVersion);
+
+  // Enumerate dirs in target_path(), ensure only desired remain.
+  base::FileEnumerator version_enum(installer_state.target_path(), false,
+                                    base::FileEnumerator::DIRECTORIES);
+  for (base::FilePath next_version = version_enum.Next(); !next_version.empty();
+       next_version = version_enum.Next()) {
+    base::FilePath dir_name(next_version.BaseName());
+    Version version(base::UTF16ToASCII(dir_name.value()));
+    if (version.IsValid()) {
+      EXPECT_TRUE(expected_remaining_dirs.erase(version.GetString()))
+          << "Unexpected version dir found: " << version.GetString();
+    }
+  }
+
+  std::set<std::string>::const_iterator iter(
+      expected_remaining_dirs.begin());
+  for (; iter != expected_remaining_dirs.end(); ++iter)
+    ADD_FAILURE() << "Expected to find version dir for " << *iter;
+}
+
 TEST_F(InstallerStateTest, InitializeTwice) {
   // Override these paths so that they can be found after the registry override
   // manager is in place.