installer and some misc files: Change ScopedTempDir::path() to GetPath()

chrome/installer/util/copy_tree_work_item.cc

Index: chrome/installer/util/copy_tree_work_item.cc
diff --git a/chrome/installer/util/copy_tree_work_item.cc b/chrome/installer/util/copy_tree_work_item.cc
index 156d7977449af23b14b0de6c166b7a8c635f6133..20e28a78de333d06a3bfd6eeaf362a92d99ce0bb 100644
--- a/chrome/installer/util/copy_tree_work_item.cc
+++ b/chrome/installer/util/copy_tree_work_item.cc
@@ -71,13 +71,14 @@ bool CopyTreeWorkItem::DoImpl() {
 
   // In all cases that reach here, move dest to a backup path.
   if (dest_exist) {
-    if (!backup_path_.CreateUniqueTempDirUnderPath(temp_dir_)) {
+    if (!backup_dir_.CreateUniqueTempDirUnderPath(temp_dir_)) {
       PLOG(ERROR) << "Failed to get backup path in folder "
                   << temp_dir_.value();
       return false;
     }
+    backup_path_ = backup_dir_.GetPath();

[gab, 2016/09/28 16:31:59]

This results in an extra copy though. I did read your comment in the header but
I don't understand why GetPath() doesn't work below, feels like both use cases
would be happy with a const FilePath& to Append() to?

[vabr (Chromium), 2016/09/28 20:06:36]

Here yes, but one of the unit tests [1] uses backup_path_ to verify that the
backup directory does not, in fact, exist. If copying the path is a concern, I
can replace the FilePath with just a bool, which would indicate whether the
backup directory has been created or not.

[1]
https://cs.chromium.org/chromium/src/chrome/installer/util/copy_tree_work_ite...

[gab, 2016/09/28 20:25:25]

So a bool would do? I also haven't looked at the tests in detail but testing for
empty() on a private variable feels like it's reaching into the guts more than a
test should (i.e. it's abusing being a friend).

[vabr (Chromium), 2016/09/28 20:33:10]

Happy to switch to a bool.

 
-    base::FilePath backup = backup_path_.path().Append(dest_path_.BaseName());
+    base::FilePath backup = backup_path_.Append(dest_path_.BaseName());
     if (base::Move(dest_path_, backup)) {
       moved_to_backup_ = true;
       VLOG(1) << "Moved destination " << dest_path_.value() <<
@@ -113,7 +114,7 @@ void CopyTreeWorkItem::RollbackImpl() {
     LOG(ERROR) << "Can not delete " << dest_path_.value();
   }
   if (moved_to_backup_) {
-    base::FilePath backup(backup_path_.path().Append(dest_path_.BaseName()));
+    base::FilePath backup(backup_path_.Append(dest_path_.BaseName()));
     if (!base::Move(backup, dest_path_)) {
       PLOG(ERROR) << "failed move " << backup.value()
                   << " to " << dest_path_.value();