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

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

Issue 1882923003: Add best-effort/allow rollback flags on WorkItem. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@simple_list_tests
Patch Set: fix build error Created 4 years, 7 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_tree_work_item.cc
diff --git a/chrome/installer/util/delete_tree_work_item.cc b/chrome/installer/util/delete_tree_work_item.cc
index 7dbb9e7a618c0bef29ec83a89f4ba21e4dcddda7..8fc5dadfed220c7193bab53f9d177c6374c96d90 100644
--- a/chrome/installer/util/delete_tree_work_item.cc
+++ b/chrome/installer/util/delete_tree_work_item.cc
@@ -4,158 +4,38 @@
#include "chrome/installer/util/delete_tree_work_item.h"
-#include <algorithm>
-#include <limits>
-
#include "base/files/file_util.h"
#include "base/logging.h"
-namespace {
-
-// Casts a value of an unsigned type to a signed type of the same size provided
-// that there is no overflow.
-template<typename L, typename R>
-bool SafeCast(L left, R* right) {
- DCHECK(right);
- static_assert(sizeof(left) == sizeof(right),
- "Items SafeCast is used with must be of the same size");
- if (left > static_cast<L>(std::numeric_limits<R>::max()))
- return false;
- *right = static_cast<L>(left);
- return true;
-}
-
-} // namespace
-
-DeleteTreeWorkItem::DeleteTreeWorkItem(
- const base::FilePath& root_path,
- const base::FilePath& temp_path,
- const std::vector<base::FilePath>& key_paths)
- : root_path_(root_path),
- temp_path_(temp_path),
- moved_to_backup_(false) {
- if (!SafeCast(key_paths.size(), &num_key_files_)) {
- NOTREACHED() << "Impossibly large key_paths collection";
- } else if (num_key_files_ != 0) {
- key_paths_.reset(new base::FilePath[num_key_files_]);
- key_backup_paths_.reset(new base::ScopedTempDir[num_key_files_]);
- std::copy(key_paths.begin(), key_paths.end(), &key_paths_[0]);
- }
-}
-
-DeleteTreeWorkItem::~DeleteTreeWorkItem() {
-}
-
-// We first try to move key_path_ to backup_path. If it succeeds, we go ahead
-// and move the rest.
-bool DeleteTreeWorkItem::Do() {
- // Go through all the key files and see if we can open them exclusively
- // with only the FILE_SHARE_DELETE flag. Once we know we have all of them,
- // we can delete them.
- std::vector<HANDLE> opened_key_files;
- opened_key_files.reserve(num_key_files_);
- bool abort = false;
- for (ptrdiff_t i = 0; !abort && i != num_key_files_; ++i) {
- base::FilePath& key_file = key_paths_[i];
- base::ScopedTempDir& backup = key_backup_paths_[i];
- if (!ignore_failure_) {
- if (!backup.CreateUniqueTempDirUnderPath(temp_path_)) {
- PLOG(ERROR) << "Could not create temp dir in " << temp_path_.value();
- abort = true;
- } else if (!base::CopyFile(key_file,
- backup.path().Append(key_file.BaseName()))) {
- PLOG(ERROR) << "Could not back up " << key_file.value()
- << " to directory " << backup.path().value();
- abort = true;
- if (!backup.Delete()) {
- PLOG(ERROR) << "Could not clean up temp dir in "
- << temp_path_.value();
- }
- }
- }
- if (!abort) {
- HANDLE file = ::CreateFile(key_file.value().c_str(), FILE_ALL_ACCESS,
- FILE_SHARE_DELETE, NULL, OPEN_EXISTING, 0,
- NULL);
- if (file != INVALID_HANDLE_VALUE) {
- VLOG(1) << "Acquired exclusive lock for key file: " << key_file.value();
- opened_key_files.push_back(file);
- } else {
- if (::GetLastError() != ERROR_FILE_NOT_FOUND)
- abort = true;
- PLOG(INFO) << "Failed to open " << key_file.value();
- }
- }
- }
-
- if (!abort) {
- // We now hold exclusive locks with "share delete" permissions for each
- // of the key files and also have created backups of those files.
- // We can safely delete the key files now.
- for (ptrdiff_t i = 0; !abort && i != num_key_files_; ++i) {
- base::FilePath& key_file = key_paths_[i];
- if (!base::DeleteFile(key_file, true)) {
- // This should not really be possible because of the above.
- PLOG(DFATAL) << "Unexpectedly could not delete " << key_file.value();
- abort = true;
- }
- }
- }
-
- std::for_each(opened_key_files.begin(), opened_key_files.end(), CloseHandle);
- opened_key_files.clear();
+DeleteTreeWorkItem::DeleteTreeWorkItem(const base::FilePath& root_path,
+ const base::FilePath& temp_path)
+ : root_path_(root_path), temp_path_(temp_path) {}
- if (abort) {
- LOG(ERROR) << "Could not exclusively hold all key files.";
- return ignore_failure_;
- }
+DeleteTreeWorkItem::~DeleteTreeWorkItem() = default;
- // Now that we've taken care of the key files, take care of the rest.
+bool DeleteTreeWorkItem::DoImpl() {
if (root_path_.empty() || !base::PathExists(root_path_))
return true;
- if (ignore_failure_) {
- if (DeleteRoot())
- return true;
- // The file cannot be removed, but perhaps it can be moved into
- // the temporary backup path. Consumers are responsible for making
- // a best-effort attempt to remove the backup path. SelfCleaningTempDir
- // is generally used for the backup path, so in the
- // worst case the file(s) will be removed after the next reboot.
- MoveRootToBackup();
+ // If rollback is not enabled, try to delete the root without making a backup.
+ // When that fails, fall back to moving the root to the temporary backup path.
+ // Consumers are responsible for making a best-effort attempt to remove the
+ // backup path. SelfCleaningTempDir is generally used for the backup path, so
+ // in the worst case the file(s) will be removed after the next reboot.
+ if (!rollback_enabled() && DeleteRoot())
return true;
- }
// Attempt to move the root to the backup.
return MoveRootToBackup();
}
-// If there are files in backup paths move them back.
-void DeleteTreeWorkItem::Rollback() {
- if (ignore_failure_)
- return;
-
+void DeleteTreeWorkItem::RollbackImpl() {
if (moved_to_backup_) {
base::FilePath backup = GetBackupPath();
DCHECK(!backup.empty());
if (base::PathExists(backup))
base::Move(backup, root_path_);
}
-
- for (ptrdiff_t i = 0; i != num_key_files_; ++i) {
- base::ScopedTempDir& backup_dir = key_backup_paths_[i];
- if (!backup_dir.path().empty()) {
- base::FilePath& key_file = key_paths_[i];
- base::FilePath backup_file =
- backup_dir.path().Append(key_file.BaseName());
- if (base::PathExists(backup_file) &&
- !base::Move(backup_file, key_file)) {
- // This could happen if we could not delete the key file to begin with.
- PLOG(WARNING) << "Rollback: Failed to move backup file back in place: "
- << backup_file.value() << " to " << key_file.value();
- }
- }
- }
}
base::FilePath DeleteTreeWorkItem::GetBackupPath() {

Powered by Google App Engine
This is Rietveld 408576698