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

chrome/installer/util/move_tree_work_item.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/move_tree_work_item.h"
 
 #include <shlwapi.h>
 
 #include "base/files/file_util.h"
 #include "base/logging.h"
@@ skipping 25 matching lines @@
   // If dest_path_ exists, we can do one of two things:
   // 1) If the contents of src_path_are already fully contained in dest_path_
   //    then do nothing and return success. Fully contained means the full
   //    file structure with identical files is contained in dest_path_. For
   //    Chrome, if dest_path_ exists, this is expected to be the common case.
   // 2) If the contents of src_path_ are NOT fully contained in dest_path_, we
   //    attempt to backup dest_path_ and replace it with src_path_. This will
   //    fail if files in dest_path_ are in use.
   if (base::PathExists(dest_path_)) {
     // Generate a backup path that can keep the original files under dest_path_.
-    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;
     }
-    base::FilePath backup = backup_path_.path().Append(dest_path_.BaseName());
+    backup_path_ = backup_dir_.GetPath();

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

Just like copy_tree_work_item.cc, looks like the only use case in this class
would be happy with GetPath()

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

Looking at the test failures in the first patch set of this CL, it seems that
MoveTreeWorkItem::RollbackImpl can be called when the backup directory has not
been created (in MoveTreeWorkItemTest.MoveAFile and
MoveTreeWorkItemTest.MoveDirectory). Calling GetPath() would trigger a DCHECK
then, on purpose. And actually, thinking about it now, RollbackImpl trying to
call base::Move on a path which comes from a directory which has not been
created seems like an error. What do you think?

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

Right, I think obtaining the variable should be inside an

if (moved_to_backup_ || source_moved_to_backup_) {}

block.

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

Good idea, I will do that.

+
+    base::FilePath backup = backup_path_.Append(dest_path_.BaseName());
 
     if (duplicate_option_ == CHECK_DUPLICATES) {
       if (installer::IsIdenticalFileHierarchy(source_path_, dest_path_)) {
         // The files we are moving are already present in the destination path.
         // We most likely don't need to do anything. As such, just move the
         // source files to the temp folder as backup.
         if (base::Move(source_path_, backup)) {
           source_moved_to_backup_ = true;
           VLOG(1) << "Moved source " << source_path_.value()
                   << " to backup path " << backup.value();
@@ skipping 36 matching lines @@
 
   return true;
 }
 
 void MoveTreeWorkItem::RollbackImpl() {
   if (moved_to_dest_path_ && !base::Move(dest_path_, source_path_)) {
     PLOG(ERROR) << "Can not move " << dest_path_.value()
                 << " to " << source_path_.value();
   }
 
-  base::FilePath backup = backup_path_.path().Append(dest_path_.BaseName());
+  base::FilePath backup = backup_path_.Append(dest_path_.BaseName());
   if (moved_to_backup_ && !base::Move(backup, dest_path_)) {
     PLOG(ERROR) << "failed move " << backup.value()
                 << " to " << dest_path_.value();
   }
 
   if (source_moved_to_backup_ && !base::Move(backup, source_path_)) {
     PLOG(ERROR) << "Can not restore " << backup.value()
                 << " to " << source_path_.value();
   }
 }