Fix a problem with the delete-after-reboot code. Removing pending renames of files not currently on disk would fail due to calls to GetShortPathName failing for already-deleted files. Now we just attempt to keep using the long path names for the already-deleted-file case. Also test.

BUG=64503
TEST=Uninstall CF while it is in use. Delete the in use files. Reinstall. Ensure that the CF files are no longer scheduled for deletion by looking at HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Control\Session Manager\PendingFileRenameOperations.


Review URL: http://codereview.chromium.org/5354006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@67889 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 71 insertions, 4 deletions

diff --git a/chrome/installer/util/delete_after_reboot_helper.cc b/chrome/installer/util/delete_after_reboot_helper.cc
index 075a6a9..1c44676 100644
--- a/chrome/installer/util/delete_after_reboot_helper.cc
+++ b/chrome/installer/util/delete_after_reboot_helper.cc
@@ -244,6 +244,13 @@ std::wstring GetShortPathName(const wchar_t* path) {
   DWORD length = GetShortPathName(path, WriteInto(&short_path, MAX_PATH),
                                   MAX_PATH);
   DLOG_IF(WARNING, length == 0) << __FUNCTION__ << " gle=" << GetLastError();
+  if (length == 0) {
+    // GetShortPathName fails if the path is no longer present. Instead of
+    // returning an empty string, just return the original string. This will
+    // serve for our purposes.
+    return path;
+  }
+
   short_path.resize(length);
   return short_path;
 }
diff --git a/chrome/installer/util/delete_after_reboot_helper.h b/chrome/installer/util/delete_after_reboot_helper.h
index 10d2f0a..a133900 100644
--- a/chrome/installer/util/delete_after_reboot_helper.h
+++ b/chrome/installer/util/delete_after_reboot_helper.h
@@ -64,7 +64,9 @@ void StringArrayToMultiSZBytes(const std::vector<PendingMove>& strings,
                                std::vector<char>* buffer);
 
 // A helper function for the win32 GetShortPathName that more conveniently
-// returns a correctly sized wstring.
+// returns a correctly sized wstring. Note that if |path| is not present on the
+// file system then GetShortPathName will return |path| unchanged, unlike the
+// win32 GetShortPathName which will return an error.
 std::wstring GetShortPathName(const wchar_t* path);
 
 #endif  // CHROME_INSTALLER_UTIL_DELETE_AFTER_REBOOT_HELPER_H_
diff --git a/chrome/installer/util/delete_after_reboot_helper_unittest.cc b/chrome/installer/util/delete_after_reboot_helper_unittest.cc
index 5d8adaf..d5239a7 100644
--- a/chrome/installer/util/delete_after_reboot_helper_unittest.cc
+++ b/chrome/installer/util/delete_after_reboot_helper_unittest.cc
@@ -138,7 +138,7 @@ TEST_F(DeleteAfterRebootHelperTest, TestStringListToMultiSZConversions) {
 }
 
 
-TEST_F(DeleteAfterRebootHelperTest, TestFileDeletes) {
+TEST_F(DeleteAfterRebootHelperTest, TestFileDeleteScheduleAndUnschedule) {
   if (!IsUserAnAdmin()) {
     return;
   }
@@ -165,13 +165,13 @@ TEST_F(DeleteAfterRebootHelperTest, TestFileDeletes) {
   // Check that each of the deletes we expect are there in order.
   FilePath expected_paths[] =
       { temp_file_, temp_subdir_file_, temp_subdir_, temp_dir_ };
-  for (int i = 0; i < arraysize(expected_paths); i++) {
+  for (int i = 0; i < arraysize(expected_paths); ++i) {
     EXPECT_FALSE(iter == pending_moves.end());
     if (iter != pending_moves.end()) {
       std::wstring short_path_name(
           GetShortPathName(expected_paths[i].value().c_str()));
       EXPECT_TRUE(MatchPendingDeletePath(short_path_name, iter->first));
-      iter++;
+      ++iter;
     }
   }
 
@@ -185,3 +185,61 @@ TEST_F(DeleteAfterRebootHelperTest, TestFileDeletes) {
     EXPECT_FALSE(MatchPendingDeletePath(short_temp_file, check_iter->first));
   }
 }
+
+TEST_F(DeleteAfterRebootHelperTest, TestFileDeleteSchedulingWithActualDeletes) {
+  if (!IsUserAnAdmin()) {
+    return;
+  }
+
+  std::vector<PendingMove> initial_pending_moves;
+  GetPendingMovesValue(&initial_pending_moves);
+  size_t initial_pending_moves_size = initial_pending_moves.size();
+
+  EXPECT_TRUE(ScheduleDirectoryForDeletion(temp_dir_.value().c_str()));
+
+  std::vector<PendingMove> pending_moves;
+  EXPECT_TRUE(SUCCEEDED(GetPendingMovesValue(&pending_moves)));
+
+  // We should see, somewhere in this key, deletion writs for
+  // temp_file_, temp_subdir_file_, temp_subdir_ and temp_dir_ in that order.
+  EXPECT_TRUE(pending_moves.size() > 3);
+
+  // Get the short form of temp_file_ and use that to match.
+  std::wstring short_temp_file(GetShortPathName(temp_file_.value().c_str()));
+
+  // Scan for the first expected delete.
+  std::vector<PendingMove>::const_iterator iter(pending_moves.begin());
+  for (; iter != pending_moves.end(); iter++) {
+    if (MatchPendingDeletePath(short_temp_file, iter->first))
+      break;
+  }
+
+  // Check that each of the deletes we expect are there in order.
+  FilePath expected_paths[] =
+      { temp_file_, temp_subdir_file_, temp_subdir_, temp_dir_ };
+  for (int i = 0; i < arraysize(expected_paths); ++i) {
+    EXPECT_FALSE(iter == pending_moves.end());
+    if (iter != pending_moves.end()) {
+      std::wstring short_path_name(
+          GetShortPathName(expected_paths[i].value().c_str()));
+      EXPECT_TRUE(MatchPendingDeletePath(short_path_name, iter->first));
+      ++iter;
+    }
+  }
+
+  // Delete the temporary directory.
+  file_util::Delete(temp_dir_, true);
+
+  // Test that we can remove the pending deletes.
+  EXPECT_TRUE(RemoveFromMovesPendingReboot(temp_dir_.value().c_str()));
+  HRESULT hr = GetPendingMovesValue(&pending_moves);
+  EXPECT_TRUE(hr == S_OK || hr == HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND));
+
+  EXPECT_EQ(initial_pending_moves_size, pending_moves.size());
+
+  std::vector<PendingMove>::const_iterator check_iter(pending_moves.begin());
+  for (; check_iter != pending_moves.end(); ++check_iter) {
+    EXPECT_FALSE(MatchPendingDeletePath(short_temp_file, check_iter->first));
+  }
+}
+