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

chrome/installer/util/installer_state.cc

 // Copyright (c) 2011 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.
 
 #include "chrome/installer/util/installer_state.h"
 
 #include <algorithm>
 #include <functional>
 #include <utility>
 
@@ skipping 430 matching lines @@
   // with ERROR_SHARING_VIOLATION if the file exists and is in-use.
   return !base::win::ScopedHandle(CreateFile(file.value().c_str(),
                                              GENERIC_WRITE, 0, NULL,
                                              OPEN_EXISTING, 0, 0)).IsValid();
 }
 
 void InstallerState::RemoveOldVersionDirectories(
     const Version& new_version,
     Version* existing_version,
     const FilePath& temp_path) const {
+  scoped_ptr<Version> version;
+  std::vector<FilePath> key_files;
+  scoped_ptr<WorkItem> item;
+
+  // 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);
-  scoped_ptr<Version> version;
-  std::vector<FilePath> key_files;
+  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);
+      }
 
-  // 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);
-        }
+      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.

 
-        VLOG(1) << "Deleting directory: " << next_version.value();
-
-        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();
   }
 }
 
 void InstallerState::AddComDllList(std::vector<FilePath>* com_dll_list) const {
   std::for_each(products_.begin(), products_.end(),
                 std::bind2nd(std::mem_fun(&Product::AddComDllList),
                              com_dll_list));
 }
 
 bool InstallerState::SetChannelFlags(bool set,
@@ skipping 101 matching lines @@
   if (is_multi_install()) {
     InstallUtil::AddInstallerResultItems(
         system_install, multi_package_binaries_distribution()->GetStateKey(),
         status, string_resource_id, launch_cmd, install_list.get());
   }
   if (!install_list->Do())
     LOG(ERROR) << "Failed to record installer error information in registry.";
 }
 
 }  // namespace installer