Revert "File::Copy avoids direct copying on Windows"

This reverts commit 391d3bc.

Reason for revert: #42291

Original change's description:
> File::Copy avoids direct copying on Windows
> 
> There is a race condition for copying file on Windows, where CopyFile()
> returns success but data has not been populated into destination file.
> E.g process is killed or died in the middle.
> 
> This cl will change File::Copy as
> 1. Copy file to a temp file in the same directory of destination file.
> 2. Rename the file to the target file.
> 
> Bug: #42119
> Change-Id: I39b6d451f6ace970bc554501148259d33de232c7
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/149667
> Commit-Queue: Zichang Guo <[email protected]>
> Reviewed-by: Zach Anderson <[email protected]>

[email protected],[email protected],[email protected]

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: #42119
Change-Id: I816611107498f41ab4af7c065ac2162af585ae95
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/151028
Reviewed-by: Zichang Guo <[email protected]>
Commit-Queue: Zichang Guo <[email protected]>

Author: zichangg

runtime/bin/file_win.cc

@@ -17,7 +17,6 @@
 #include <sys/utime.h>  // NOLINT
 
 #include "bin/builtin.h"
-#include "bin/crypto.h"
 #include "bin/directory.h"
 #include "bin/namespace.h"
 #include "bin/utils.h"
@@ -504,85 +503,6 @@ bool File::RenameLink(Namespace* namespc,
   return (move_status != 0);
 }
 
-static wchar_t* CopyToDartScopeString(wchar_t* string) {
-  wchar_t* wide_path = reinterpret_cast<wchar_t*>(
-      Dart_ScopeAllocate(MAX_PATH * sizeof(wchar_t) + 1));
-  wcscpy(wide_path, string);
-  return wide_path;
-}
-
-static wchar_t* CopyIntoTempFile(const char* src, const char* dest) {
-  // This function will copy the file to a temp file in the destination
-  // directory and return the path of temp file.
-  // Creating temp file name has the same logic as Directory::CreateTemp(),
-  // which tries with the rng and falls back to a uuid if it failed.
-  const char* last_backslash = strrchr(dest, '\\');
-  if (last_backslash == NULL) {
-    return NULL;
-  }
-  int length_of_parent_dir = last_backslash - dest + 1;
-  if (length_of_parent_dir + 8 > MAX_PATH) {
-    return NULL;
-  }
-  uint32_t suffix_bytes = 0;
-  const int kSuffixSize = sizeof(suffix_bytes);
-  if (Crypto::GetRandomBytes(kSuffixSize,
-                             reinterpret_cast<uint8_t*>(&suffix_bytes))) {
-    PathBuffer buffer;
-    char* dir = reinterpret_cast<char*>(
-        Dart_ScopeAllocate(1 + sizeof(char) * length_of_parent_dir));
-    memmove(dir, dest, length_of_parent_dir);
-    dir[length_of_parent_dir] = '\0';
-    if (!buffer.Add(dir)) {
-      return NULL;
-    }
-
-    char suffix[8 + 1];
-    Utils::SNPrint(suffix, sizeof(suffix), "%x", suffix_bytes);
-    Utf8ToWideScope source_path(src);
-    if (!buffer.Add(suffix)) {
-      return NULL;
-    }
-    if (CopyFileExW(source_path.wide(), buffer.AsStringW(), NULL, NULL, NULL,
-                    0) != 0) {
-      return CopyToDartScopeString(buffer.AsStringW());
-    }
-  }
-  // UUID has a total of 36 characters in the form of
-  // xxxxxxxx-xxxx-Mxxx-Nxxx-xxxxxxxxxxxx.
-  if (length_of_parent_dir + 36 > MAX_PATH) {
-    return NULL;
-  }
-  UUID uuid;
-  RPC_STATUS status = UuidCreateSequential(&uuid);
-  if ((status != RPC_S_OK) && (status != RPC_S_UUID_LOCAL_ONLY)) {
-    return NULL;
-  }
-  RPC_WSTR uuid_string;
-  status = UuidToStringW(&uuid, &uuid_string);
-  if (status != RPC_S_OK) {
-    return NULL;
-  }
-  PathBuffer buffer;
-  char* dir = reinterpret_cast<char*>(
-      Dart_ScopeAllocate(1 + sizeof(char) * length_of_parent_dir));
-  memmove(dir, dest, length_of_parent_dir);
-  dir[length_of_parent_dir] = '\0';
-  Utf8ToWideScope dest_path(dir);
-  if (!buffer.AddW(dest_path.wide()) ||
-      !buffer.AddW(reinterpret_cast<wchar_t*>(uuid_string))) {
-    return NULL;
-  }
-
-  RpcStringFreeW(&uuid_string);
-  Utf8ToWideScope source_path(src);
-  if (CopyFileExW(source_path.wide(), buffer.AsStringW(), NULL, NULL, NULL,
-                  0) != 0) {
-    return CopyToDartScopeString(buffer.AsStringW());
-  }
-  return NULL;
-}
-
 bool File::Copy(Namespace* namespc,
                 const char* old_path,
                 const char* new_path) {
@@ -591,23 +511,11 @@ bool File::Copy(Namespace* namespc,
     SetLastError(ERROR_FILE_NOT_FOUND);
     return false;
   }
-
-  wchar_t* temp_file = CopyIntoTempFile(old_path, new_path);
-  if (temp_file == NULL) {
-    return false;
-  }
-  Utf8ToWideScope system_new_dest(new_path);
-
-  // Remove the existing file. Otherwise, renaming will fail.
-  if (Exists(namespc, new_path)) {
-    DeleteFileW(system_new_dest.wide());
-  }
-
-  if (!MoveFileW(temp_file, system_new_dest.wide())) {
-    DeleteFileW(temp_file);
-    return false;
-  }
-  return true;
+  Utf8ToWideScope system_old_path(old_path);
+  Utf8ToWideScope system_new_path(new_path);
+  bool success = CopyFileExW(system_old_path.wide(), system_new_path.wide(),
+                             NULL, NULL, NULL, 0) != 0;
+  return success;
 }
 
 int64_t File::LengthFromPath(Namespace* namespc, const char* name) {

tests/standalone/io/file_copy_test.dart

@@ -30,14 +30,6 @@ void testCopySync() {
   Expect.equals(FILE_CONTENT2, file1.readAsStringSync());
   Expect.equals(FILE_CONTENT2, file2.readAsStringSync());
 
-  // Check there is no temporary files existing.
-  var list = tmp.listSync();
-  Expect.equals(2, list.length);
-  for (var file in list) {
-    final fileName = file.path.toString();
-    Expect.isTrue(fileName.contains("file1") || fileName.contains("file2"));
-  }
-
   // Fail when coping to directory.
   var dir = new Directory('${tmp.path}/dir')..createSync();
   Expect.throws(() => file1.copySync(dir.path));

tests/standalone_2/io/file_copy_test.dart

@@ -30,14 +30,6 @@ void testCopySync() {
   Expect.equals(FILE_CONTENT2, file1.readAsStringSync());
   Expect.equals(FILE_CONTENT2, file2.readAsStringSync());
 
-  // Check there is no temporary files existing.
-  var list = tmp.listSync();
-  Expect.equals(2, list.length);
-  for (var file in list) {
-    final fileName = file.path.toString();
-    Expect.isTrue(fileName.contains("file1") || fileName.contains("file2"));
-  }
-
   // Fail when coping to directory.
   var dir = new Directory('${tmp.path}/dir')..createSync();
   Expect.throws(() => file1.copySync(dir.path));