Delete the installation folder after deleting its contents.

chrome/installer/setup/uninstall.cc

Index: chrome/installer/setup/uninstall.cc
diff --git a/chrome/installer/setup/uninstall.cc b/chrome/installer/setup/uninstall.cc
index f4c4d70a253314a8fbfd68c38f7f9964a60ae4f5..bf870ddc345eee9751699bf15d05cf93713300eb 100644
--- a/chrome/installer/setup/uninstall.cc
+++ b/chrome/installer/setup/uninstall.cc
@@ -318,25 +318,23 @@ 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();
-    }
+// Deletes the given directory if it is empty. Returns true if the directory is
+// deleted. *error is set to true iff the attempt to delete the file fails.
+bool DeleteEmptyDir(const FilePath& path, bool* error) {

[gab, 2012/08/09 16:07:22]

I'm not a fan of the bool return and bool* error param.

Could you remove |error| and make the return an enum?

edit: My desire to remove |error| is even stronger as I see its value is not
even used in every scenario below.

Maybe add DELETE_INVALID or something to the DeleteValue enum?

[erikwright (departed), 2012/08/13 14:53:22]

Done.

+  *error = false;
 
-    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;
+  if (path.empty())
+    return false;
+
+  if (!file_util::IsDirectoryEmpty(path))

[gab, 2012/08/09 16:07:22]

Group this condition with path.empty() above since the result (return false) is
the same.

[erikwright (departed), 2012/08/13 14:53:22]

N/A.

+    return false;
+
+  if (file_util::Delete(path, true))
+    return true;
+
+  *error = true;
+  LOG(ERROR) << "Failed to delete folder: " << path.value();
+  return false;
 }
 
 void GetLocalStateFolders(const Product& product,
@@ -398,7 +396,9 @@ 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]);
+    bool error = false;
+    if (DeleteEmptyDir(local_state_folders[0].DirName(), &error))
+      DeleteEmptyDir(local_state_folders[0].DirName().DirName(), &error);
   }
 
   return result;
@@ -446,7 +446,15 @@ DeleteResult DeleteAppHostFilesAndFolders(const InstallerState& installer_state,
     result = DELETE_FAILED;
     LOG(ERROR) << "Failed to delete path: " << app_host_exe.value();
   } else {
-    DeleteEmptyParentDir(target_path);
+    bool error = false;
+    if (DeleteEmptyDir(target_path, &error)) {
+      // Now check and delete if the parent directories are empty
+      // For example Google\Chrome or Chromium
+      if (DeleteEmptyDir(target_path.DirName(), &error))
+        DeleteEmptyDir(target_path.DirName().DirName(), &error);
+    }
+    if (error)
+      result = DELETE_FAILED;
   }
 
   return result;
@@ -478,7 +486,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 +515,23 @@ 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);
+    bool error = false;
+    if (DeleteEmptyDir(target_path, &error)) {
+      // Now check and delete if the parent directories are empty
+      // For example Google\Chrome or Chromium
+      if (DeleteEmptyDir(target_path.DirName(), &error))
+        DeleteEmptyDir(target_path.DirName().DirName(), &error);
+    }
+    if (error)
+      result = DELETE_FAILED;
   }
   return result;
 }