Re-try r29078: Remove some deprecated file_util wstring functions.

With the previous patch, the try bots failed with mysterious messages, so I ignored them, patched it into my windows box and tested it there manually, and found no problems. As it turns out, the try failures were real :(. But nsylvain and I found the problem: the behavior of file_util::GetDirectoryFromPath() differs from DirName() when the path is empty (officially, GetDirectoryFromPath is not supposed to support non-absolute paths, but that is not enforced).

Here is a green win try result: http://build.chromium.org/buildbot/try-server/builders/win/builds/3705
mac: http://build.chromium.org/buildbot/try-server/builders/mac/builds/3491
linux: http://build.chromium.org/buildbot/try-server/builders/linux/builds/3466

I also applied this patch locally in Windows to test that it doesn't break the chrome frame compile or tests, since that's not covered by the trybots yet.
Review URL: http://codereview.chromium.org/271099

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

base/file_util.cc

@@ -317,10 +317,6 @@ bool ContentsEqual(const std::wstring& filename1,
   return ContentsEqual(FilePath::FromWStringHack(filename1),
                        FilePath::FromWStringHack(filename2));
 }
-bool CopyFile(const std::wstring& from_path, const std::wstring& to_path) {
-  return CopyFile(FilePath::FromWStringHack(from_path),
-                  FilePath::FromWStringHack(to_path));
-}
 bool CreateDirectory(const std::wstring& full_path) {
   return CreateDirectory(FilePath::FromWStringHack(full_path));
 }
@@ -340,9 +336,6 @@ bool CreateNewTempDirectory(const std::wstring& prefix,
 bool Delete(const std::wstring& path, bool recursive) {
   return Delete(FilePath::FromWStringHack(path), recursive);
 }
-bool DirectoryExists(const std::wstring& path) {
-  return DirectoryExists(FilePath::FromWStringHack(path));
-}
 bool EndsWithSeparator(std::wstring* path) {
   return EndsWithSeparator(FilePath::FromWStringHack(*path));
 }
@@ -384,16 +377,9 @@ bool GetTempDir(std::wstring* path_str) {
   *path_str = path.ToWStringHack();
   return true;
 }
-bool Move(const std::wstring& from_path, const std::wstring& to_path) {
-  return Move(FilePath::FromWStringHack(from_path),
-              FilePath::FromWStringHack(to_path));
-}
 FILE* OpenFile(const std::wstring& filename, const char* mode) {
   return OpenFile(FilePath::FromWStringHack(filename), mode);
 }
-bool PathExists(const std::wstring& path) {
-  return PathExists(FilePath::FromWStringHack(path));
-}
 int ReadFile(const std::wstring& filename, char* data, int size) {
   return ReadFile(FilePath::FromWStringHack(filename), data, size);
 }

base/file_util.h

@@ -128,8 +128,6 @@ bool Delete(const std::wstring& path, bool recursive);
 // on different volumes, this will attempt to copy and delete. Returns
 // true for success.
 bool Move(const FilePath& from_path, const FilePath& to_path);
-// Deprecated temporary compatibility function.
-bool Move(const std::wstring& from_path, const std::wstring& to_path);
 
 // Renames file |from_path| to |to_path|. Both paths must be on the same
 // volume, or the function will fail. Destination file will be created
@@ -140,8 +138,6 @@ bool ReplaceFile(const FilePath& from_path, const FilePath& to_path);
 
 // Copies a single file. Use CopyDirectory to copy directories.
 bool CopyFile(const FilePath& from_path, const FilePath& to_path);
-// Deprecated temporary compatibility function.
-bool CopyFile(const std::wstring& from_path, const std::wstring& to_path);
 
 // Copies the given path, and optionally all subdirectories and their contents
 // as well.
@@ -159,16 +155,12 @@ bool CopyDirectory(const std::wstring& from_path, const std::wstring& to_path,
 // Returns true if the given path exists on the local filesystem,
 // false otherwise.
 bool PathExists(const FilePath& path);
-// Deprecated temporary compatibility function.
-bool PathExists(const std::wstring& path);
 
 // Returns true if the given path is writable by the user, false otherwise.
 bool PathIsWritable(const FilePath& path);
 
 // Returns true if the given path exists and is a directory, false otherwise.
 bool DirectoryExists(const FilePath& path);
-// Deprecated temporary compatibility function.
-bool DirectoryExists(const std::wstring& path);
 
 #if defined(OS_WIN)
 // Gets the creation time of the given file (expressed in the local timezone),

chrome/browser/first_run_win.cc

@@ -311,7 +311,7 @@ bool Upgrade::SwapNewChromeExeIfPresent() {
   std::wstring new_chrome_exe;
   if (!GetNewerChromeFile(&new_chrome_exe))
     return false;
-  if (!file_util::PathExists(new_chrome_exe))
+  if (!file_util::PathExists(FilePath::FromWStringHack(new_chrome_exe)))
     return false;
   std::wstring curr_chrome_exe;
   if (!PathService::Get(base::FILE_EXE, &curr_chrome_exe))

chrome/browser/importer/firefox2_importer.cc

@@ -291,7 +291,7 @@ void Firefox2Importer::ImportPasswords() {
   // exist, we try to find its older version.
   std::wstring file = source_path_;
   file_util::AppendToPath(&file, L"signons2.txt");
-  if (!file_util::PathExists(file)) {
+  if (!file_util::PathExists(FilePath::FromWStringHack(file))) {
     file = source_path_;
     file_util::AppendToPath(&file, L"signons.txt");
   }

chrome/browser/importer/firefox3_importer.cc

@@ -68,7 +68,7 @@ void Firefox3Importer::StartImport(ProfileInfo profile_info,
 void Firefox3Importer::ImportHistory() {
   std::wstring file = source_path_;
   file_util::AppendToPath(&file, L"places.sqlite");
-  if (!file_util::PathExists(file))
+  if (!file_util::PathExists(FilePath::FromWStringHack(file)))
     return;
 
   sqlite3* sqlite;
@@ -116,7 +116,7 @@ void Firefox3Importer::ImportHistory() {
 void Firefox3Importer::ImportBookmarks() {
   std::wstring file = source_path_;
   file_util::AppendToPath(&file, L"places.sqlite");
-  if (!file_util::PathExists(file))
+  if (!file_util::PathExists(FilePath::FromWStringHack(file)))
     return;
 
   sqlite3* sqlite;
@@ -314,7 +314,7 @@ void Firefox3Importer::GetSearchEnginesXMLFiles(
     std::vector<std::wstring>* files) {
   std::wstring file = source_path_;
   file_util::AppendToPath(&file, L"search.sqlite");
-  if (!file_util::PathExists(file))
+  if (!file_util::PathExists(FilePath::FromWStringHack(file)))
     return;
 
   sqlite3* sqlite;

chrome/browser/importer/importer_unittest.cc

@@ -49,7 +49,7 @@ class ImporterTest : public testing::Test {
   virtual void TearDown() {
     // Deletes the profile and cleans up the profile directory.
     ASSERT_TRUE(file_util::Delete(test_path_, true));
-    ASSERT_FALSE(file_util::PathExists(test_path_));
+    ASSERT_FALSE(file_util::PathExists(FilePath::FromWStringHack(test_path_)));
   }
 
   void Firefox3xImporterTest(std::wstring profile_dir,
@@ -70,7 +70,7 @@ class ImporterTest : public testing::Test {
     if (import_search_plugins) {
       ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &data_path));
       file_util::AppendToPath(&data_path, L"firefox3_searchplugins");
-      if (!file_util::PathExists(data_path)) {
+      if (!file_util::PathExists(FilePath::FromWStringHack(data_path))) {
         // TODO(maruel):  Create search test data that we can open source!
         LOG(ERROR) << L"Missing internal test data";
         return;
@@ -646,7 +646,7 @@ TEST_F(ImporterTest, Firefox2Importer) {
   CreateDirectory(search_engine_path.c_str(), NULL);
   ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &data_path));
   file_util::AppendToPath(&data_path, L"firefox2_searchplugins");
-  if (!file_util::PathExists(data_path)) {
+  if (!file_util::PathExists(FilePath::FromWStringHack(data_path))) {
     // TODO(maruel):  Create test data that we can open source!
     LOG(ERROR) << L"Missing internal test data";
     return;

chrome/browser/jumplist.cc

@@ -486,9 +486,10 @@ void JumpListUpdateTask::Run() {
   // icon directory, and create a new directory which contains new JumpList
   // icon files.
   std::wstring icon_dir_old(icon_dir_ + L"Old");
-  if (file_util::PathExists(icon_dir_old))
+  if (file_util::PathExists(FilePath::FromWStringHack(icon_dir_old)))
     file_util::Delete(icon_dir_old, true);
-  file_util::Move(icon_dir_, icon_dir_old);
+  file_util::Move(FilePath::FromWStringHack(icon_dir_),
+                  FilePath::FromWStringHack(icon_dir_old));
   file_util::CreateDirectory(icon_dir_);
 
   // Create temporary icon files for shortcuts in the "Most Visited" category.

chrome/browser/printing/printing_layout_uitest.cc

@@ -73,7 +73,8 @@ class PrintingLayoutTest : public PrintingTest<UITest> {
     FilePath png(verification_file + L".png");
 
     // Looks for Cleartype override.
-    if (file_util::PathExists(verification_file + L"_cleartype.png") &&
+    if (file_util::PathExists(
+            FilePath::FromWStringHack(verification_file + L"_cleartype.png")) &&
         IsClearTypeEnabled()) {
       png = FilePath(verification_file + L"_cleartype.png");
     }

chrome/browser/tab_contents/navigation_controller_unittest.cc

@@ -88,7 +88,7 @@ class NavigationControllerHistoryTest : public NavigationControllerTest {
     NavigationControllerTest::TearDown();
 
     ASSERT_TRUE(file_util::Delete(test_dir_, true));
-    ASSERT_FALSE(file_util::PathExists(test_dir_));
+    ASSERT_FALSE(file_util::PathExists(FilePath::FromWStringHack(test_dir_)));
   }
 
   // Deletes the current profile manager and creates a new one. Indirectly this

chrome/common/chrome_plugin_util.cc

@@ -131,7 +131,7 @@ CPError CPB_GetCommandLineArgumentsCommon(const char* url,
   if (!user_data_dir.empty()) {
     // Make sure user_data_dir is an absolute path.
     if (file_util::AbsolutePath(&user_data_dir) &&
-        file_util::PathExists(user_data_dir)) {
+        file_util::PathExists(FilePath::FromWStringHack(user_data_dir))) {
       // TODO(evanm): use CommandLine APIs instead of this.
       arguments_w += std::wstring(L"--") + ASCIIToWide(switches::kUserDataDir) +
                      L"=\"" + user_data_dir + L"\" ";

chrome/installer/setup/install.cc

@@ -157,12 +157,13 @@ void AddUninstallShortcutWorkItems(HKEY reg_root,
 // only on the first install of Chrome.
 void CopyPreferenceFileForFirstRun(bool system_level,
                                    const std::wstring& prefs_source_path) {
-  std::wstring prefs_dest_path(
+  FilePath prefs_dest_path = FilePath::FromWStringHack(
       installer::GetChromeInstallPath(system_level));
-  file_util::AppendToPath(&prefs_dest_path,
-                          installer_util::kDefaultMasterPrefs);
-  if (!file_util::CopyFile(prefs_source_path, prefs_dest_path))
+  prefs_dest_path = prefs_dest_path.Append(installer_util::kDefaultMasterPrefs);
+  if (!file_util::CopyFile(FilePath::FromWStringHack(prefs_source_path),
+                           prefs_dest_path)) {
     LOG(INFO) << "Failed to copy master preferences.";
+  }
 }
 
 // This method creates Chrome shortcuts in Start->Programs for all users or
@@ -304,7 +305,7 @@ bool DoPostInstallTasks(HKEY reg_root,
   BrowserDistribution* dist = BrowserDistribution::GetDistribution();
   std::wstring version_key = dist->GetVersionKey();
 
-  if (file_util::PathExists(new_chrome_exe)) {
+  if (file_util::PathExists(FilePath::FromWStringHack(new_chrome_exe))) {
     // Looks like this was in use update. So make sure we update the 'opv' key
     // with the current version that is active and 'cmd' key with the rename
     // command to run.
@@ -492,7 +493,7 @@ bool InstallNewVersion(const std::wstring& exe_path,
                                            installer_util::kChromeNewExe);
   BrowserDistribution* dist = BrowserDistribution::GetDistribution();
   RegKey chrome_key(reg_root, dist->GetVersionKey().c_str(), KEY_READ);
-  if (file_util::PathExists(new_chrome_exe))
+  if (file_util::PathExists(FilePath::FromWStringHack(new_chrome_exe)))
     chrome_key.ReadValue(google_update::kRegOldVersionField, current_version);
   if (current_version->empty())
     chrome_key.ReadValue(google_update::kRegVersionField, current_version);

chrome/installer/setup/setup_main.cc

@@ -67,7 +67,7 @@ DWORD UnPackArchive(const std::wstring& archive, bool system_install,
   // Check if this is differential update and if it is, patch it to the
   // installer archive that should already be on the machine. We assume
   // it is a differential installer if chrome.7z is not found.
-  if (!file_util::PathExists(uncompressed_archive)) {
+  if (!file_util::PathExists(FilePath::FromWStringHack(uncompressed_archive))) {
     incremental_install = true;
     LOG(INFO) << "Differential patch found. Applying to existing archive.";
     if (!installed_version) {
@@ -163,10 +163,11 @@ bool CheckPreInstallConditions(const installer::Version* installed_version,
   // either does not exist or can be deleted (i.e. is not locked by some other
   // process).
   if (!installed_version) {
-    std::wstring install_path(installer::GetChromeInstallPath(system_install));
+    FilePath install_path = FilePath::FromWStringHack(
+        installer::GetChromeInstallPath(system_install));
     if (file_util::PathExists(install_path) &&
         !file_util::Delete(install_path, true)) {
-      LOG(ERROR) << "Installation directory " << install_path
+      LOG(ERROR) << "Installation directory " << install_path.value()
                  << " exists and can not be deleted.";
       status = installer_util::INSTALL_DIR_IN_USE;
       int str_id = IDS_INSTALL_DIR_IN_USE_BASE;

chrome/installer/util/copy_tree_work_item.cc

@@ -31,7 +31,7 @@ CopyTreeWorkItem::CopyTreeWorkItem(const std::wstring& source_path,
 
 bool CopyTreeWorkItem::Do() {
   if (!file_util::PathExists(source_path_)) {
-    LOG(ERROR) << source_path_ << " does not exist";
+    LOG(ERROR) << source_path_.value() << " does not exist";
     return false;
   }
 
@@ -39,29 +39,29 @@ bool CopyTreeWorkItem::Do() {
   // handle overwrite_option_ = IF_DIFFERENT case.
   if ((dest_exist) &&
       (overwrite_option_ == WorkItem::IF_DIFFERENT) && // only for single file
-      (!PathIsDirectory(source_path_.c_str())) &&
-      (!PathIsDirectory(dest_path_.c_str())) &&
+      (!file_util::DirectoryExists(source_path_)) &&
+      (!file_util::DirectoryExists(dest_path_)) &&
       (file_util::ContentsEqual(source_path_, dest_path_))) {
-    LOG(INFO) << "Source file " << source_path_
-              << " and destination file " << dest_path_
+    LOG(INFO) << "Source file " << source_path_.value()
+              << " and destination file " << dest_path_.value()
               << " are exactly same. Returning true.";
     return true;
   } else if ((dest_exist) &&
              (overwrite_option_ == WorkItem::NEW_NAME_IF_IN_USE) &&
-             (!PathIsDirectory(source_path_.c_str())) &&
-             (!PathIsDirectory(dest_path_.c_str())) &&
+             (!file_util::DirectoryExists(source_path_)) &&
+             (!file_util::DirectoryExists(dest_path_)) &&
              (IsFileInUse(dest_path_))) {
     // handle overwrite_option_ = NEW_NAME_IF_IN_USE case.
     if (alternative_path_.empty() ||
         file_util::PathExists(alternative_path_) ||
         !file_util::CopyFile(source_path_, alternative_path_)) {
-      LOG(ERROR) << "failed to copy " << source_path_ <<
-                    " to " << alternative_path_;
+      LOG(ERROR) << "failed to copy " << source_path_.value() <<
+                    " to " << alternative_path_.value();
       return false;
     } else {
       copied_to_alternate_path_ = true;
-      LOG(INFO) << "Copied source file " << source_path_
-                << " to alternative path " << alternative_path_;
+      LOG(INFO) << "Copied source file " << source_path_.value()
+                << " to alternative path " << alternative_path_.value();
       return true;
     }
   } else if ((dest_exist) &&
@@ -77,10 +77,11 @@ bool CopyTreeWorkItem::Do() {
 
     if (file_util::Move(dest_path_, backup_path_)) {
       moved_to_backup_ = true;
-      LOG(INFO) << "Moved destination " << dest_path_
-                << " to backup path " << backup_path_;
+      LOG(INFO) << "Moved destination " << dest_path_.value() <<
+                   " to backup path " << backup_path_.value();
     } else {
-      LOG(ERROR) << "failed moving " << dest_path_ << " to " << backup_path_;
+      LOG(ERROR) << "failed moving " << dest_path_.value() << " to " <<
+                     backup_path_.value();
       return false;
     }
   }
@@ -88,10 +89,11 @@ bool CopyTreeWorkItem::Do() {
   // In all cases that reach here, copy source to destination.
   if (file_util::CopyDirectory(source_path_, dest_path_, true)) {
     copied_to_dest_path_ = true;
-    LOG(INFO) << "Copied source " << source_path_
-              << " to destination " << dest_path_;
+    LOG(INFO) << "Copied source " << source_path_.value()
+              << " to destination " << dest_path_.value();
   } else {
-    LOG(ERROR) << "failed copy " << source_path_ << " to " << dest_path_;
+    LOG(ERROR) << "failed copy " << source_path_.value() <<
+                  " to " << dest_path_.value();
     return false;
   }
 
@@ -105,22 +107,23 @@ void CopyTreeWorkItem::Rollback() {
   // Delete here. For now we just log the error and continue with the
   // rest of rollback operation.
   if (copied_to_dest_path_ && !file_util::Delete(dest_path_, true)) {
-    LOG(ERROR) << "Can not delete " << dest_path_;
+    LOG(ERROR) << "Can not delete " << dest_path_.value();
   }
   if (moved_to_backup_ && !file_util::Move(backup_path_, dest_path_)) {
-    LOG(ERROR) << "failed move " << backup_path_ << " to " << dest_path_;
+    LOG(ERROR) << "failed move " << backup_path_.value() << " to " <<
+                  dest_path_.value();
   }
   if (copied_to_alternate_path_ &&
       !file_util::Delete(alternative_path_, true)) {
-    LOG(ERROR) << "Can not delete " << alternative_path_;
+    LOG(ERROR) << "Can not delete " << alternative_path_.value();
   }
 }
 
-bool CopyTreeWorkItem::IsFileInUse(const std::wstring& path) {
+bool CopyTreeWorkItem::IsFileInUse(const FilePath& path) {
   if (!file_util::PathExists(path))
     return false;
 
-  HANDLE handle = ::CreateFile(path.c_str(), FILE_ALL_ACCESS,
+  HANDLE handle = ::CreateFile(path.value().c_str(), FILE_ALL_ACCESS,
                                NULL, NULL, OPEN_EXISTING, NULL, NULL);
   if (handle  == INVALID_HANDLE_VALUE)
     return true;
@@ -130,16 +133,14 @@ bool CopyTreeWorkItem::IsFileInUse(const std::wstring& path) {
 }
 
 bool CopyTreeWorkItem::GetBackupPath() {
-  std::wstring file_name = file_util::GetFilenameFromPath(dest_path_);
-  backup_path_.assign(temp_dir_);
-  file_util::AppendToPath(&backup_path_, file_name);
+  backup_path_ = temp_dir_.Append(dest_path_.BaseName());
 
   if (file_util::PathExists(backup_path_)) {
     // Ideally we should not fail immediately. Instead we could try some
     // random paths under temp_dir_ until we reach certain limit.
     // For now our caller always provides a good temporary directory so
     // we don't bother.
-    LOG(ERROR) << "backup path " << backup_path_ << " already exists";
+    LOG(ERROR) << "backup path " << backup_path_.value() << " already exists";
     return false;
   }
 

chrome/installer/util/copy_tree_work_item.h

@@ -7,6 +7,8 @@
 
 #include <string>
 #include <windows.h>
+
+#include "base/file_path.h"
 #include "chrome/installer/util/work_item.h"
 
 // A WorkItem subclass that recursively copies a file system hierarchy from
@@ -40,20 +42,20 @@ class CopyTreeWorkItem : public WorkItem {
                    const std::wstring& alternative_path);
 
   // Checks if the path specified is in use (and hence can not be deleted)
-  bool IsFileInUse(const std::wstring& path);
+  bool IsFileInUse(const FilePath& path);
 
   // Get a backup path that can keep the original files under dest_path_,
   // and set backup_path_ with the result.
   bool GetBackupPath();
 
   // Source path to copy files from.
-  std::wstring source_path_;
+  FilePath source_path_;
 
   // Destination path to copy files to.
-  std::wstring dest_path_;
+  FilePath dest_path_;
 
   // Temporary directory that can be used.
-  std::wstring temp_dir_;
+  FilePath temp_dir_;
 
   // Controls the behavior for overwriting.
   CopyOverWriteOption overwrite_option_;
@@ -61,7 +63,7 @@ class CopyTreeWorkItem : public WorkItem {
   // If overwrite_option_ = NEW_NAME_IF_IN_USE, this variables stores the path
   // to be used if the file is in use and hence we want to copy it to a
   // different path.
-  std::wstring alternative_path_;
+  FilePath alternative_path_;
 
   // Whether the source was copied to dest_path_
   bool copied_to_dest_path_;
@@ -76,7 +78,7 @@ class CopyTreeWorkItem : public WorkItem {
 
   // The full path in temporary directory that the original dest_path_ has
   // been moved to.
-  std::wstring backup_path_;
+  FilePath backup_path_;
 };
 
 #endif  // CHROME_INSTALLER_UTIL_COPY_TREE_WORK_ITEM_H_

chrome/installer/util/copy_tree_work_item_unittest.cc

@@ -61,11 +61,11 @@ namespace {
     file.close();
   }
 
-  bool IsFileInUse(const std::wstring& path) {
+  bool IsFileInUse(const FilePath& path) {
     if (!file_util::PathExists(path))
       return false;
 
-    HANDLE handle = ::CreateFile(path.c_str(), FILE_ALL_ACCESS,
+    HANDLE handle = ::CreateFile(path.value().c_str(), FILE_ALL_ACCESS,
                                  NULL, NULL, OPEN_EXISTING, NULL, NULL);
     if (handle  == INVALID_HANDLE_VALUE)
       return true;
@@ -478,10 +478,10 @@ TEST_F(CopyTreeWorkItemTest, NewNameAndCopyTest) {
       file_name_from.ToWStringHack(), file_name_to.ToWStringHack(),
       temp_dir_.ToWStringHack(), WorkItem::NEW_NAME_IF_IN_USE,
       alternate_to.ToWStringHack()));
-  if (IsFileInUse(file_name_to.value()))
+  if (IsFileInUse(file_name_to))
     PlatformThread::Sleep(2000);
   // If file is still in use, the rest of the test will fail.
-  ASSERT_FALSE(IsFileInUse(file_name_to.value()));
+  ASSERT_FALSE(IsFileInUse(file_name_to));
   EXPECT_TRUE(work_item->Do());
 
   EXPECT_TRUE(file_util::PathExists(file_name_from));

chrome/installer/util/delete_after_reboot_helper.cc

@@ -64,7 +64,7 @@ bool ScheduleFileSystemEntityForDeletion(const wchar_t* path) {
   }
 
   DWORD flags = MOVEFILE_DELAY_UNTIL_REBOOT;
-  if (!file_util::DirectoryExists(path)) {
+  if (!file_util::DirectoryExists(FilePath::FromWStringHack(path))) {
     // This flag valid only for files
     flags |= MOVEFILE_REPLACE_EXISTING;
   }

chrome/installer/util/delete_tree_work_item.cc

@@ -7,13 +7,17 @@
 #include "chrome/installer/util/delete_tree_work_item.h"
 
 DeleteTreeWorkItem::~DeleteTreeWorkItem() {
-  std::wstring tmp_dir = file_util::GetDirectoryFromPath(backup_path_);
-  if (file_util::PathExists(tmp_dir)) {
-    file_util::Delete(tmp_dir, true);
+  if (!backup_path_.empty()) {
+    FilePath tmp_dir = backup_path_.DirName();
+    if (file_util::PathExists(tmp_dir)) {
+      file_util::Delete(tmp_dir, true);
+    }
   }
-  tmp_dir = file_util::GetDirectoryFromPath(key_backup_path_);
-  if (file_util::PathExists(tmp_dir)) {
-    file_util::Delete(tmp_dir, true);
+  if (!key_backup_path_.empty()) {
+    FilePath tmp_dir = key_backup_path_.DirName();
+    if (file_util::PathExists(tmp_dir)) {
+      file_util::Delete(tmp_dir, true);
+    }
   }
 }
 
@@ -30,8 +34,8 @@ bool DeleteTreeWorkItem::Do() {
     if (!GetBackupPath(key_path_, &key_backup_path_) ||
         !file_util::CopyDirectory(key_path_, key_backup_path_, true) ||
         !file_util::Delete(key_path_, true)) {
-      LOG(ERROR) << "can not delete " << key_path_
-                 << " OR copy it to backup path " << key_backup_path_;
+      LOG(ERROR) << "can not delete " << key_path_.value()
+                 << " OR copy it to backup path " << key_backup_path_.value();
       return false;
     }
   }
@@ -40,8 +44,8 @@ bool DeleteTreeWorkItem::Do() {
     if (!GetBackupPath(root_path_, &backup_path_) ||
         !file_util::CopyDirectory(root_path_, backup_path_, true) ||
         !file_util::Delete(root_path_, true)) {
-      LOG(ERROR) << "can not delete " << root_path_
-                 << " OR copy it to backup path " << backup_path_;
+      LOG(ERROR) << "can not delete " << root_path_.value()
+                 << " OR copy it to backup path " << backup_path_.value();
       return false;
     }
   }
@@ -58,15 +62,14 @@ void DeleteTreeWorkItem::Rollback() {
   }
 }
 
-bool DeleteTreeWorkItem::GetBackupPath(const std::wstring& for_path,
-                                       std::wstring* backup_path) {
+bool DeleteTreeWorkItem::GetBackupPath(const FilePath& for_path,
+                                       FilePath* backup_path) {
   if (!file_util::CreateNewTempDirectory(L"", backup_path)) {
     // We assume that CreateNewTempDirectory() is doing its job well.
     LOG(ERROR) << "Couldn't get backup path for delete.";
     return false;
   }
-  std::wstring file_name = file_util::GetFilenameFromPath(for_path);
-  file_util::AppendToPath(backup_path, file_name);
 
+  *backup_path = backup_path->Append(for_path.BaseName());
   return true;
 }

chrome/installer/util/delete_tree_work_item.h

@@ -7,6 +7,8 @@
 
 #include <string>
 #include <windows.h>
+
+#include "base/file_path.h"
 #include "chrome/installer/util/work_item.h"
 
 // A WorkItem subclass that recursively deletes a file system hierarchy at the
@@ -26,25 +28,25 @@ class DeleteTreeWorkItem : public WorkItem {
   friend class WorkItem;
 
   // Get a backup path that can keep root_path_ or key_path_
-  bool GetBackupPath(const std::wstring& for_path, std::wstring* backup_path);
+  bool GetBackupPath(const FilePath& for_path, FilePath* backup_path);
 
   DeleteTreeWorkItem(const std::wstring& root_path,
                      const std::wstring& key_path);
 
   // Root path to delete.
-  std::wstring root_path_;
+  FilePath root_path_;
 
   // Path to the key file. If the key file is specified, deletion will be
   // performed only if the key file is not in use.
-  std::wstring key_path_;
+  FilePath key_path_;
 
   // The full path in temporary directory that the original root_path_ has
   // been moved to.
-  std::wstring backup_path_;
+  FilePath backup_path_;
 
   // The full path in temporary directory that the original key_path_ has
   // been moved to.
-  std::wstring key_backup_path_;
+  FilePath key_backup_path_;
 };
 
 #endif  // CHROME_INSTALLER_UTIL_DELETE_TREE_WORK_ITEM_H_

chrome/installer/util/move_tree_work_item.cc

@@ -26,30 +26,26 @@ MoveTreeWorkItem::MoveTreeWorkItem(const std::wstring& source_path,
 
 bool MoveTreeWorkItem::Do() {
   if (!file_util::PathExists(source_path_)) {
-    LOG(ERROR) << source_path_ << " does not exist";
+    LOG(ERROR) << source_path_.value() << " does not exist";
     return false;
   }
 
-  FilePath backup_path;
-
   // If dest_path_ exists, move destination to a backup path.
   if (file_util::PathExists(dest_path_)) {
     // Generate a backup path that can keep the original files under dest_path_.
     if (!file_util::CreateTemporaryFileInDir(FilePath(temp_dir_),
-                                             &backup_path)) {
-      LOG(ERROR) << "Failed to get backup path in folder " << temp_dir_;
+                                             &backup_path_)) {
+      LOG(ERROR) << "Failed to get backup path in folder " << temp_dir_.value();
       return false;
     }
 
-    backup_path_ = backup_path.value();
-
     if (file_util::Move(dest_path_, backup_path_)) {
       moved_to_backup_ = true;
-      LOG(INFO) << "Moved destination " << dest_path_
-                << " to backup path " << backup_path_;
+      LOG(INFO) << "Moved destination " << dest_path_.value()
+                << " to backup path " << backup_path_.value();
     } else {
-      LOG(ERROR) << "failed moving " << dest_path_
-                 << " to " << backup_path_;
+      LOG(ERROR) << "failed moving " << dest_path_.value()
+                 << " to " << backup_path_.value();
       return false;
     }
   }
@@ -57,10 +53,11 @@ bool MoveTreeWorkItem::Do() {
   // Now move source to destination.
   if (file_util::Move(source_path_, dest_path_)) {
     moved_to_dest_path_ = true;
-    LOG(INFO) << "Moved source " << source_path_
-              << " to destination " << dest_path_;
+    LOG(INFO) << "Moved source " << source_path_.value()
+              << " to destination " << dest_path_.value();
   } else {
-    LOG(ERROR) << "failed move " << source_path_ << " to " << dest_path_;
+    LOG(ERROR) << "failed move " << source_path_.value() << " to " <<
+                  dest_path_.value();
     return false;
   }
 
@@ -69,8 +66,10 @@ bool MoveTreeWorkItem::Do() {
 
 void MoveTreeWorkItem::Rollback() {
   if (moved_to_dest_path_ && !file_util::Move(dest_path_, source_path_))
-    LOG(ERROR) << "Can not move " << dest_path_ << " to " << source_path_;
+    LOG(ERROR) << "Can not move " << dest_path_.value() <<
+                   " to " << source_path_.value();
 
   if (moved_to_backup_ && !file_util::Move(backup_path_, dest_path_))
-    LOG(ERROR) << "failed move " << backup_path_ << " to " << dest_path_;
+    LOG(ERROR) << "failed move " << backup_path_.value() <<
+                  " to " << dest_path_.value();
 }

chrome/installer/util/move_tree_work_item.h

@@ -38,17 +38,17 @@ class MoveTreeWorkItem : public WorkItem {
                    const std::wstring& temp_dir);
 
   // Source path to move files from.
-  std::wstring source_path_;
+  FilePath source_path_;
 
   // Destination path to move files to.
-  std::wstring dest_path_;
+  FilePath dest_path_;
 
   // Temporary directory to backup dest_path_ (if it already exists).
-  std::wstring temp_dir_;
+  FilePath temp_dir_;
 
   // The full path in temp_dir_ where the original dest_path_ has
   // been moved to.
-  std::wstring backup_path_;
+  FilePath backup_path_;
 
   // Whether the source was moved to dest_path_
   bool moved_to_dest_path_;