Errors while deleting old version dirs no longer cause them to grow in odd ways.

chrome/installer/util/installer_state.cc

Index: chrome/installer/util/installer_state.cc
diff --git a/chrome/installer/util/installer_state.cc b/chrome/installer/util/installer_state.cc
index ba0740d7f9189156044f4f1c7cae1f35ed728e8a..6c54c512c5ad61659ee6b2b21912f6e87ef808ad 100644
--- a/chrome/installer/util/installer_state.cc
+++ b/chrome/installer/util/installer_state.cc
@@ -448,50 +448,43 @@ void InstallerState::RemoveOldVersionDirectories(
     const Version& new_version,
     Version* existing_version,
     const FilePath& temp_path) const {
-  file_util::FileEnumerator version_enum(target_path(), false,
-      file_util::FileEnumerator::DIRECTORIES);
   scoped_ptr<Version> version;
   std::vector<FilePath> key_files;
+  scoped_ptr<WorkItem> item;
 
-  // We try to delete all directories whose versions are lower than
-  // latest_version.
-  FilePath next_version = version_enum.Next();
-  while (!next_version.empty()) {
-    file_util::FileEnumerator::FindInfo find_data = {0};
-    version_enum.GetFindInfo(&find_data);
-    VLOG(1) << "directory found: " << find_data.cFileName;
-    version.reset(Version::GetVersionFromString(
-                      WideToASCII(find_data.cFileName)));
-    if (version.get()) {
-      // Delete the version folder if it is less than the new version and not
-      // equal to the old version (if we have an old version).
-      if (version->CompareTo(new_version) < 0 &&
-          (existing_version == NULL ||
-           version->CompareTo(*existing_version) != 0)) {
-        key_files.clear();
-        std::for_each(products_.begin(), products_.end(),
-                      std::bind2nd(std::mem_fun(&Product::AddKeyFiles),
-                                   &key_files));
-        const std::vector<FilePath>::iterator end = key_files.end();
-        for (std::vector<FilePath>::iterator scan = key_files.begin();
-             scan != end; ++scan) {
-          *scan = next_version.Append(*scan);
-        }
+  // Try to delete all directories whose versions are lower than latest_version
+  // and not equal to the existing version (opv).
+  file_util::FileEnumerator version_enum(target_path(), false,
+      file_util::FileEnumerator::DIRECTORIES);
+  for (FilePath next_version = version_enum.Next(); !next_version.empty();
+       next_version = version_enum.Next()) {
+    FilePath dir_name(next_version.BaseName());
+    VLOG(1) << "Directory found: " << dir_name.value();

[robertshield, 2011/10/14 21:28:35]

Is this log useful to folk reading the log file? 

[grt (UTC plus 2), 2011/10/14 22:55:25]

Hmm.  Not so much.  Removed.

+    version.reset(Version::GetVersionFromString(WideToASCII(dir_name.value())));
+    // Delete the version folder if it is less than the new version and not
+    // equal to the old version (if we have an old version).
+    if (version.get() &&
+        version->CompareTo(new_version) < 0 &&
+        (existing_version == NULL || !version->Equals(*existing_version))) {
+      // Collect the key files (relative to the version dir) for all products.
+      key_files.clear();
+      std::for_each(products_.begin(), products_.end(),
+                    std::bind2nd(std::mem_fun(&Product::AddKeyFiles),
+                                 &key_files));
+      // Make the key_paths absolute.
+      const std::vector<FilePath>::iterator end = key_files.end();
+      for (std::vector<FilePath>::iterator scan = key_files.begin();
+           scan != end; ++scan) {
+        *scan = next_version.Append(*scan);
+      }
 
-        VLOG(1) << "Deleting directory: " << next_version.value();
+      VLOG(1) << "Deleting directory: " << next_version.value();

[robertshield, 2011/10/14 21:28:35]

"Deleting directory: " -> "Deleting old version directory: "

[grt (UTC plus 2), 2011/10/14 22:55:25]

Done.

 
-        scoped_ptr<WorkItem> item(
-            WorkItem::CreateDeleteTreeWorkItem(next_version, temp_path,
-                                               key_files));
-        if (!item->Do()) {
-          LOG(ERROR) << "Failed to delete old version directory: "
-                     << next_version.value();
-          item->Rollback();
-        }
-      }
+      item.reset(WorkItem::CreateDeleteTreeWorkItem(next_version, temp_path,
+                                                    key_files));
+      item->set_ignore_failure(true);
+      item->Do();
     }
-
-    next_version = version_enum.Next();
   }
 }