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

Unified Diff: chrome/installer/setup/uninstall.cc

Issue 10832210: Delete the installation folder after deleting its contents. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Review comments. Created 8 years, 4 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
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/installer/setup/uninstall.cc
diff --git a/chrome/installer/setup/uninstall.cc b/chrome/installer/setup/uninstall.cc
index f4c4d70a253314a8fbfd68c38f7f9964a60ae4f5..c1d3633cd83222f48fb569c3baba8857d3721611 100644
--- a/chrome/installer/setup/uninstall.cc
+++ b/chrome/installer/setup/uninstall.cc
@@ -318,25 +318,25 @@ bool ScheduleParentAndGrandparentForDeletion(const FilePath& path) {
return ret;
}
-// Deletes empty parent & empty grandparent dir of given path.
-bool DeleteEmptyParentDir(const FilePath& path) {
- bool ret = true;
- FilePath parent_dir = path.DirName();
- if (!parent_dir.empty() && file_util::IsDirectoryEmpty(parent_dir)) {
- if (!file_util::Delete(parent_dir, true)) {
- ret = false;
- LOG(ERROR) << "Failed to delete folder: " << parent_dir.value();
- }
+enum DeleteResult {
+ DELETE_SUCCEEDED,
+ DELETE_NOT_EMPTY,
+ DELETE_FAILED,
+ DELETE_REQUIRES_REBOOT,
+};
- parent_dir = parent_dir.DirName();
- if (!parent_dir.empty() && file_util::IsDirectoryEmpty(parent_dir)) {
- if (!file_util::Delete(parent_dir, true)) {
- ret = false;
- LOG(ERROR) << "Failed to delete folder: " << parent_dir.value();
- }
- }
- }
- return ret;
+// Deletes the given directory if it is empty. Returns DELETE_SUCCEEDED if the
+// directory is deleted, DELETE_NOT_EMPTY if it is not empty, and DELETE_FAILED
+// otherwise.
+DeleteResult DeleteEmptyDir(const FilePath& path) {
+ if (!file_util::IsDirectoryEmpty(path))
+ return DELETE_NOT_EMPTY;
+
+ if (file_util::Delete(path, true))
+ return DELETE_SUCCEEDED;
+
+ LOG(ERROR) << "Failed to delete folder: " << path.value();
+ return DELETE_FAILED;
}
void GetLocalStateFolders(const Product& product,
@@ -367,12 +367,6 @@ FilePath BackupLocalStateFile(
return backup;
}
-enum DeleteResult {
- DELETE_SUCCEEDED,
- DELETE_FAILED,
- DELETE_REQUIRES_REBOOT,
-};
-
// Deletes all user data directories for a product.
DeleteResult DeleteLocalState(const std::vector<FilePath>& local_state_folders,
bool schedule_on_failure) {
@@ -398,7 +392,13 @@ DeleteResult DeleteLocalState(const std::vector<FilePath>& local_state_folders,
if (result == DELETE_REQUIRES_REBOOT) {
ScheduleParentAndGrandparentForDeletion(local_state_folders[0]);
} else {
- DeleteEmptyParentDir(local_state_folders[0]);
+ const FilePath user_data_dir(local_state_folders[0].DirName());
+ if (!user_data_dir.empty() &&
+ DeleteEmptyDir(user_data_dir) == DELETE_SUCCEEDED) {
+ const FilePath product_dir(user_data_dir.DirName());
+ if (!product_dir.empty())
+ DeleteEmptyDir(product_dir);
+ }
}
return result;
@@ -428,6 +428,27 @@ bool MoveSetupOutOfInstallFolder(const InstallerState& installer_state,
return ret;
}
+DeleteResult DeleteApplicationProductAndVendorDirectories(
+ const FilePath& application_directory) {
+ DeleteResult result(DeleteEmptyDir(application_directory));
gab 2012/08/13 19:33:54 nit: DeleteResult is a POD (i.e. enum), I think th
erikwright (departed) 2012/08/13 19:37:37 Found no mention of this for PODS or non-PODS. The
gab 2012/08/13 19:46:16 Ah interesting, maybe we should define "grt-style"
+ if (result == DELETE_SUCCEEDED) {
+ // Now check and delete if the parent directories are empty
+ // For example Google\Chrome or Chromium
+ const FilePath product_directory(application_directory.DirName());
+ if (!product_directory.empty()) {
+ result = DeleteEmptyDir(product_directory);
+ if (result == DELETE_SUCCEEDED) {
+ const FilePath vendor_directory(product_directory.DirName());
+ if (!vendor_directory.empty())
+ result = DeleteEmptyDir(vendor_directory);
+ }
+ }
+ }
+ if (result == DELETE_NOT_EMPTY)
+ result = DELETE_SUCCEEDED;
+ return result;
+}
+
DeleteResult DeleteAppHostFilesAndFolders(const InstallerState& installer_state,
const Version& installed_version) {
const FilePath& target_path = installer_state.target_path();
@@ -446,7 +467,7 @@ DeleteResult DeleteAppHostFilesAndFolders(const InstallerState& installer_state,
result = DELETE_FAILED;
LOG(ERROR) << "Failed to delete path: " << app_host_exe.value();
} else {
- DeleteEmptyParentDir(target_path);
+ result = DeleteApplicationProductAndVendorDirectories(target_path);
}
return result;
@@ -478,7 +499,7 @@ DeleteResult DeleteChromeFilesAndFolders(const InstallerState& installer_state,
if (to_delete.BaseName().value() == installer::kChromeAppHostExe)
continue;
- VLOG(1) << "Deleting install path " << target_path.value();
+ VLOG(1) << "Deleting install path " << to_delete.value();
if (!file_util::Delete(to_delete, true)) {
LOG(ERROR) << "Failed to delete path (1st try): " << to_delete.value();
if (installer_state.FindProduct(BrowserDistribution::CHROME_FRAME)) {
@@ -507,14 +528,15 @@ DeleteResult DeleteChromeFilesAndFolders(const InstallerState& installer_state,
}
if (result == DELETE_REQUIRES_REBOOT) {
+ // Delete the Application directory at reboot if empty.
+ ScheduleFileSystemEntityForDeletion(target_path.value().c_str());
+
// If we need a reboot to continue, schedule the parent directories for
// deletion unconditionally. If they are not empty, the session manager
// will not delete them on reboot.
ScheduleParentAndGrandparentForDeletion(target_path);
} else {
- // Now check and delete if the parent directories are empty
- // For example Google\Chrome or Chromium
- DeleteEmptyParentDir(target_path);
+ result = DeleteApplicationProductAndVendorDirectories(target_path);
}
return result;
}
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698