Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(325)

Unified Diff: chrome/installer/util/delete_old_versions.cc

Issue 1666363002: Delete old files after an update. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 4 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/installer/util/delete_old_versions.cc
diff --git a/chrome/installer/util/delete_old_versions.cc b/chrome/installer/util/delete_old_versions.cc
new file mode 100644
index 0000000000000000000000000000000000000000..5d3b5296d67cb7c5d6cc2bb0bd4f81b0307f452b
--- /dev/null
+++ b/chrome/installer/util/delete_old_versions.cc
@@ -0,0 +1,203 @@
+// Copyright (c) 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/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/strings/string16.h"
+#include "chrome/installer/util/util_constants.h"
+
+namespace {
+
+// Returns the version of the executable |exe_path|.
+base::string16 GetExecutableVersion(const base::FilePath& exe_path) {
+ scoped_ptr<FileVersionInfo> file_version_info(
+ FileVersionInfo::CreateFileVersionInfo(exe_path));
+ if (!file_version_info.get())
+ return base::string16();
+ return file_version_info->file_version();
+}
+
+// Returns the directory names found in |install_dir|. The directories named
+// after the version of chrome.exe or new_chrome.exe are excluded.
+using DirectorySet = std::set<base::string16>;
+DirectorySet GetDirectories(const base::FilePath& install_dir) {
+ const base::string16 new_chrome_exe_version =
+ GetExecutableVersion(install_dir.Append(installer::kChromeNewExe));
+ const base::string16 chrome_exe_version =
+ GetExecutableVersion(install_dir.Append(installer::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::string16 directory_name = directory_path.BaseName().value();
grt (UTC plus 2) 2016/02/10 20:02:04 nit: make this and the container use base::FilePat
fdoray 2016/02/18 17:59:20 Done.
+ if (directory_name != new_chrome_exe_version &&
+ directory_name != chrome_exe_version) {
+ directories.insert(directory_name);
grt (UTC plus 2) 2016/02/10 20:02:04 for the sake of caution, perhaps this should only
fdoray 2016/02/18 17:59:20 Done.
+ }
+ }
+ return directories;
+}
+
+// Returns a map where the keys are versions and values are paths of
+// old_chrome*.exe executables found in |install_dir|.
+using ExecutableVector = std::vector<base::FilePath>;
grt (UTC plus 2) 2016/02/10 20:02:04 nit: change this type to PathVector and use it on
fdoray 2016/02/18 17:59:20 Done.
+using ExecutableMap = std::map<base::string16, ExecutableVector>;
grt (UTC plus 2) 2016/02/10 20:02:03 maybe this should also map a FilePath to a vector
fdoray 2016/02/18 17:59:20 Done.
+ExecutableMap GetExecutables(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[GetExecutableVersion(exe_path)].push_back(exe_path);
+ }
+ return executables;
+}
+
+// Deletes directories that are in |directories| and don't have a matching
+// executable in |executables|.
+bool DeleteDirectoriesWithoutMatchingExecutable(
+ const DirectorySet& directories,
+ const ExecutableMap& executables,
+ const base::FilePath& install_dir) {
+ bool success = true;
+ for (const base::string16& directory_name : directories) {
+ // Delete the directory if it doesn't have a matching executable.
+ if (executables.find(directory_name) == executables.end())
grt (UTC plus 2) 2016/02/10 20:02:04 nit: #include "base/stl_util.h" if (!ContainsV
fdoray 2016/02/18 17:59:20 Done.
+ success &= base::DeleteFile(install_dir.Append(directory_name), true);
grt (UTC plus 2) 2016/02/10 20:02:03 we expect that this will never happen, so i think
fdoray 2016/02/18 17:59:20 Done.
+ }
+ return success;
+}
+
+// Deletes executables that are in |executables| and don't have a matching
+// directory in |directories|.
+bool DeleteExecutablesWithoutMatchingDirectory(
+ const DirectorySet& directories,
+ const ExecutableMap& executables) {
+ bool success = true;
+ for (const auto& version_and_executables : executables) {
+ const auto& version = 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 (directories.find(version) != directories.end())
grt (UTC plus 2) 2016/02/10 20:02:03 if (ContainsValue(directories, version))
fdoray 2016/02/18 17:59:20 Done.
+ continue;
+
+ // Delete executables for version |version|.
+ for (const auto& executable_path : executables_for_version)
+ success &= base::DeleteFile(executable_path, false);
grt (UTC plus 2) 2016/02/10 20:02:04 we expect that this will never happen, so i think
fdoray 2016/02/18 17:59:20 Done.
+ }
+ 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.
+bool DeleteVersion(const base::FilePath& version_directory,
+ const ExecutableVector& version_executables) {
+ std::vector<base::File> locks;
+ std::vector<base::FilePath> 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(L".exe") || path.MatchesExtension(L".dll")) {
grt (UTC plus 2) 2016/02/10 20:02:04 nit: use FILE_PATH_LITERAL("") rather than L"" for
grt (UTC plus 2) 2016/02/10 20:02:04 nit: reverse the logic here to "continue" when not
fdoray 2016/02/18 17:59:20 Done.
fdoray 2016/02/18 17:59:20 Done.
+ base::File lock(GetFileLock(path));
grt (UTC plus 2) 2016/02/10 20:02:04 wdyt of doing away with the local here and on line
fdoray 2016/02/18 17:59:20 Done.
+ if (!lock.IsValid())
+ return false;
grt (UTC plus 2) 2016/02/10 20:02:03 It makes sense to use LOG(WARNING) to indicate wha
fdoray 2016/02/18 17:59:20 Done.
+ locks.push_back(std::move(lock));
grt (UTC plus 2) 2016/02/10 20:02:03 #include <utility> if you keep this
fdoray 2016/02/18 17:59:20 I no longer use std::move.
+ locked_file_paths.push_back(path);
+ }
+ }
+
+ // Lock executables in |version_executables|.
+ for (const base::FilePath& executable_path : version_executables) {
+ base::File lock(GetFileLock(executable_path));
+ if (!lock.IsValid())
+ return false;
grt (UTC plus 2) 2016/02/10 20:02:03 same comment here about logging
fdoray 2016/02/18 17:59:20 Done.
+ locks.push_back(std::move(lock));
+ 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)
+ success &= base::DeleteFile(locked_file_path, false);
grt (UTC plus 2) 2016/02/10 20:02:04 this isn't ever expected to fail, right? PLOG(ERRO
fdoray 2016/02/18 17:59:20 Done.
+
+ // 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.
+ success &= base::DeleteFile(version_directory, true);
grt (UTC plus 2) 2016/02/10 20:02:04 PLOG(ERROR) when this fails so we understand why
fdoray 2016/02/18 17:59:20 Done.
+
+ 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.
+bool DeleteMatchingExecutablesAndDirectories(
+ const DirectorySet& directories,
+ const ExecutableMap& executables,
+ const base::FilePath& install_dir) {
+ bool success = true;
+ for (const auto directory_name : directories) {
grt (UTC plus 2) 2016/02/10 20:02:04 nit: const auto&
fdoray 2016/02/18 17:59:20 Done.
+ ExecutableMap::const_iterator version_executables_it =
+ executables.find(directory_name);
+
+ // Check if the directory has at least one matching executable.
+ 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;
+}
+
+} // namespace
+
+bool DeleteOldVersions(const base::FilePath& install_dir) {
+ const DirectorySet directories = GetDirectories(install_dir);
+ const ExecutableMap executables = GetExecutables(install_dir);
+
+ bool success = true;
+ success &= DeleteDirectoriesWithoutMatchingExecutable(
grt (UTC plus 2) 2016/02/10 20:02:04 I'm not a fan of using bitwise operations on bools
fdoray 2016/02/18 17:59:20 I want the function to return a bool so that we kn
+ directories, executables, install_dir);
+ success &=
+ DeleteExecutablesWithoutMatchingDirectory(directories, executables);
+ success &= DeleteMatchingExecutablesAndDirectories(directories, executables,
+ install_dir);
+
+ return success;
+}

Powered by Google App Engine
This is Rietveld 408576698