Report more relevant error code for windows base::ReplaceFile impl

base/files/file_util_win.cc

 // Copyright (c) 2012 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 "base/files/file_util.h"
 
 #include <windows.h>
 #include <io.h>
 #include <psapi.h>
 #include <shellapi.h>
@@ skipping 116 matching lines @@
 }
 
 bool ReplaceFile(const FilePath& from_path,
                  const FilePath& to_path,
                  File::Error* error) {
   ThreadRestrictions::AssertIOAllowed();
   // Try a simple move first.  It will only succeed when |to_path| doesn't
   // already exist.
   if (::MoveFile(from_path.value().c_str(), to_path.value().c_str()))
     return true;
+  File::Error move_error = File::OSErrorToFileError(GetLastError());

[dcheng, 2017/02/07 23:24:50]

DWORD error_code = ::GetLastError();

+
   // Try the full-blown replace if the move fails, as ReplaceFile will only
   // succeed when |to_path| does exist. When writing to a network share, we may
   // not be able to change the ACLs. Ignore ACL errors then
   // (REPLACEFILE_IGNORE_MERGE_ERRORS).
   if (::ReplaceFile(to_path.value().c_str(), from_path.value().c_str(), NULL,
                     REPLACEFILE_IGNORE_MERGE_ERRORS, NULL, NULL)) {
     return true;
   }
-  if (error)
-    *error = File::OSErrorToFileError(GetLastError());
+  // In the case of FILE_ERROR_NOT_FOUND from ReplaceFile, it is likely that
+  // |to_path| does not exist. In this case, the more relevant error comes
+  // from the call to MoveFile.
+  if (error) {
+    File::Error replace_error = File::OSErrorToFileError(GetLastError());
+    *error = replace_error == File::FILE_ERROR_NOT_FOUND ? move_error

[dcheng, 2017/02/07 23:24:50]

if ((replace_error = ::GetLastError()) != ERROR_FILE_NOT_FOUND)
  error_code = replace_error;
if (error)
  *error = File::OSErrorToFileError(error_code);

[Charlie Harrison, 2017/02/07 23:31:23]

Going to push back on this, only because I think if we're comparing the raw
error we'll have to check PATH_NOT_FOUND as well. OSErrorToFileError combines
them into a single NOT_FOUND.

[dcheng, 2017/02/07 23:32:39]

Makes sense, I'm OK with that.

+                                                         : replace_error;
+  }
   return false;
 }
 
 bool CopyDirectory(const FilePath& from_path, const FilePath& to_path,
                    bool recursive) {
   // NOTE(maruel): Previous version of this function used to call
   // SHFileOperation().  This used to copy the file attributes and extended
   // attributes, OLE structured storage, NTFS file system alternate data
   // streams, SECURITY_DESCRIPTOR. In practice, this is not what we want, we
   // want the containing directory to propagate its SECURITY_DESCRIPTOR.
@@ skipping 663 matching lines @@
     // Like Move, this function is not transactional, so we just
     // leave the copied bits behind if deleting from_path fails.
     // If to_path exists previously then we have already overwritten
     // it by now, we don't get better off by deleting the new bits.
   }
   return false;
 }
 
 }  // namespace internal
 }  // namespace base