Delete the installation folder after deleting its contents.

chrome/installer/setup/uninstall.cc

 // Copyright (c) 2012 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.
 //
 // This file defines the methods useful for uninstalling Chrome.
 
 #include "chrome/installer/setup/uninstall.h"
 
 #include <windows.h>
 
@@ skipping 300 matching lines @@
     FilePath grandparent_dir(parent_dir.DirName());
     ret = ScheduleFileSystemEntityForDeletion(grandparent_dir.value().c_str());
     if (!ret) {
       LOG(ERROR) << "Failed to schedule grandparent dir for deletion: "
                  << grandparent_dir.value();
     }
   }
   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 (path.empty())
+    return DELETE_FAILED;
+
+  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,
                           std::vector<FilePath>* paths) {
   // Obtain the location of the user profile data.
   product.GetUserDataPaths(paths);
   LOG_IF(ERROR, paths->empty())
       << "Could not retrieve user's profile directory.";
 }
 
@@ skipping 10 matching lines @@
       continue;
     if (!file_util::CreateTemporaryFile(&backup))
       LOG(ERROR) << "Failed to create temporary file for Local State.";
     else
       file_util::CopyFile(state_file, backup);
     break;
   }
   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) {
   if (local_state_folders.empty())
     return DELETE_SUCCEEDED;
 
   DeleteResult result = DELETE_SUCCEEDED;
   for (size_t i = 0; i < local_state_folders.size(); ++i) {
     const FilePath& user_local_state = local_state_folders[i];
     VLOG(1) << "Deleting user profile " << user_local_state.value();
     if (!file_util::Delete(user_local_state, true)) {
       LOG(ERROR) << "Failed to delete user profile dir: "
                  << user_local_state.value();
       if (schedule_on_failure) {
         ScheduleDirectoryForDeletion(user_local_state.value().c_str());
         result = DELETE_REQUIRES_REBOOT;
       } else {
         result = DELETE_FAILED;
       }
     }
   }
 
   if (result == DELETE_REQUIRES_REBOOT) {
     ScheduleParentAndGrandparentForDeletion(local_state_folders[0]);
   } else {
-    DeleteEmptyParentDir(local_state_folders[0]);
+    FilePath user_data_dir = local_state_folders[0].DirName();

[gab, 2012/08/13 18:20:15]

nit: As per Chromium style, initialize non-POD types using '()' rather than '='.

(and make it 'const')

[erikwright (departed), 2012/08/13 19:27:36]

Done.

+    if (!user_data_dir.empty() &&

[gab, 2012/08/13 18:20:15]

why do you need to first check if it isn't empty? (and even if you do I feel the
current conditional is wrong and should be the opposite, i.e. check that it IS
empty... no?)

[erikwright (departed), 2012/08/13 19:27:36]

Note that this is querying the path string and not the filesystem.

+        DELETE_SUCCEEDED == DeleteEmptyDir(user_data_dir)) {

[gab, 2012/08/13 18:20:15]

nit: put constant on RHS of ==

[erikwright (departed), 2012/08/13 19:27:36]

Done.

+      FilePath product_dir = user_data_dir.DirName();

[gab, 2012/08/13 18:20:15]

nit: initialization style as above

optional: I would personally prefer to have this inlined below.

[erikwright (departed), 2012/08/13 19:27:36]

Done.
I think labeling it has merit. Also, now it's used twice.

+      DeleteEmptyDir(product_dir);
+    }
   }
 
   return result;
 }
 
 bool MoveSetupOutOfInstallFolder(const InstallerState& installer_state,
                                  const FilePath& setup_path,
                                  const Version& installed_version) {
   bool ret = false;
   FilePath setup_exe(installer_state.GetInstallerDirectory(installed_version)
       .Append(setup_path.BaseName()));
   FilePath temp_file;
   if (!file_util::CreateTemporaryFile(&temp_file)) {
     LOG(ERROR) << "Failed to create temporary file for setup.exe.";
   } else {
     VLOG(1) << "Attempting to move setup to: " << temp_file.value();
     ret = file_util::Move(setup_exe, temp_file);
     PLOG_IF(ERROR, !ret) << "Failed to move setup to " << temp_file.value();
 
     // We cannot delete the file right away, but try to delete it some other
     // way. Either with the help of a different process or the system.
     if (ret && !file_util::DeleteAfterReboot(temp_file)) {
       static const uint32 kDeleteAfterMs = 10 * 1000;
       installer::DeleteFileFromTempProcess(temp_file, kDeleteAfterMs);
     }
   }
   return ret;
 }
 
+DeleteResult DeleteApplicationProductAndVendorDirectories(

[gab, 2012/08/13 18:20:15]

Wouldn't it make more sense to simply delete all parents in the hierarchy
iteratively until one of them is not empty?

Or take a number of parent directories to delete and expect to be able to delete
that amount iteratively (or return the number deleted and make that check caller
side)?

[erikwright (departed), 2012/08/13 19:27:36]

No. It seems wrong to attempt to delete AppData or AppData/Local even if we do
first check that they're empty.
Seems better to me to explicitly state in code what directories we are
attempting to delete, rather than having mysterious literals like '2' and '3'.

+    const FilePath& application_directory) {
+  DeleteResult result = DeleteEmptyDir(application_directory);
+  if (result == DELETE_SUCCEEDED) {
+    // Now check and delete if the parent directories are empty
+    // For example Google\Chrome or Chromium
+    FilePath product_directory = application_directory.DirName();

[gab, 2012/08/13 18:20:15]

nit: initialization style as above

[erikwright (departed), 2012/08/13 19:27:36]

Done.

+    if (!product_directory.empty()) {

[gab, 2012/08/13 18:20:15]

As above I don't see the need for this check and feel it is backwards if
anything...

[erikwright (departed), 2012/08/13 19:27:36]

::empty() is querying the path string, not the filesystem.

+        result = DeleteEmptyDir(product_directory);
+        if (result == DELETE_SUCCEEDED) {
+          FilePath vendor_directory = product_directory.DirName();
+          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();
   if (target_path.empty()) {
     LOG(ERROR) << "DeleteAppHostFilesAndFolders: no installation destination "
                << "path.";
     return DELETE_FAILED;  // Nothing else we can do to uninstall, so we return.
   }
 
   DeleteInstallTempDir(target_path);
 
   DeleteResult result = DELETE_SUCCEEDED;
 
   FilePath app_host_exe(target_path.Append(installer::kChromeAppHostExe));
   if (!file_util::Delete(app_host_exe, false)) {
     result = DELETE_FAILED;
     LOG(ERROR) << "Failed to delete path: " << app_host_exe.value();
   } else {
-    DeleteEmptyParentDir(target_path);
+    result = DeleteApplicationProductAndVendorDirectories(target_path);
   }
 
   return result;
 }
 
 DeleteResult DeleteChromeFilesAndFolders(const InstallerState& installer_state,
                                          const Version& installed_version) {
   const FilePath& target_path = installer_state.target_path();
   if (target_path.empty()) {
     LOG(ERROR) << "DeleteChromeFilesAndFolders: no installation destination "
@@ skipping 11 matching lines @@
       false,
       static_cast<FileEnumerator::FileType>(FileEnumerator::FILES |
                                             FileEnumerator::DIRECTORIES));
   while (true) {
     FilePath to_delete(file_enumerator.Next());
     if (to_delete.empty())
       break;
     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)) {
         // We don't try killing Chrome processes for Chrome Frame builds since
         // that is unlikely to help. Instead, schedule files for deletion and
         // return a value that will trigger a reboot prompt.
         FileEnumerator::FindInfo find_info;
         file_enumerator.GetFindInfo(&find_info);
         if (FileEnumerator::IsDirectory(find_info))
           ScheduleDirectoryForDeletion(to_delete.value().c_str());
         else
           ScheduleFileSystemEntityForDeletion(to_delete.value().c_str());
         result = DELETE_REQUIRES_REBOOT;
       } else {
         // Try closing any running Chrome processes and deleting files once
         // again.
         CloseAllChromeProcesses();
         if (!file_util::Delete(to_delete, true)) {
           LOG(ERROR) << "Failed to delete path (2nd try): "
                      << to_delete.value();
           result = DELETE_FAILED;
           break;
         }
       }
     }
   }
 
   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;
 }
 
 // This method checks if Chrome is currently running or if the user has
 // cancelled the uninstall operation by clicking Cancel on the confirmation
 // box that Chrome pops up.
 InstallStatus IsChromeActiveOrUserCancelled(
     const InstallerState& installer_state,
     const Product& product) {
@@ skipping 633 matching lines @@
 
   // Try and delete the preserved local state once the post-install
   // operations are complete.
   if (!backup_state_file.empty())
     file_util::Delete(backup_state_file, false);
 
   return ret;
 }
 
 }  // namespace installer