Make CopyDirectory() not copy the read only bit on Windows.

base/file_util_win.cc

Index: base/file_util_win.cc
diff --git a/base/file_util_win.cc b/base/file_util_win.cc
index dd6282516e4f0a1888ac56da826476898bd9e758..9205b7e924fff18190cefb45e96ffcedf935d93f 100644
--- a/base/file_util_win.cc
+++ b/base/file_util_win.cc
@@ -15,6 +15,7 @@
 #include <string>
 
 #include "base/files/file_path.h"
+#include "base/files/file_enumerator.h"
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
 #include "base/process/process_handle.h"
@@ -34,6 +35,9 @@ namespace {
 const DWORD kFileShareAll =
     FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE;
 
+// ShellCopy can copy directories, unlike CopyFileUnsafe. Beside that, they are
+// mostly similar but both date backs to ~2008.
+// This function is used exclusively for CopyDirectory().
 bool ShellCopy(const FilePath& from_path,
                const FilePath& to_path,
                bool recursive) {
@@ -65,11 +69,30 @@ bool ShellCopy(const FilePath& from_path,
   file_operation.pFrom = double_terminated_path_from;
   file_operation.pTo = double_terminated_path_to;
   file_operation.fFlags = FOF_NOERRORUI | FOF_SILENT | FOF_NOCONFIRMATION |
-                          FOF_NOCONFIRMMKDIR;
+                          FOF_NOCONFIRMMKDIR | FOF_NOCOPYSECURITYATTRIBS;
   if (!recursive)
     file_operation.fFlags |= FOF_NORECURSION | FOF_FILESONLY;
 
-  return (SHFileOperation(&file_operation) == 0);
+  if (SHFileOperation(&file_operation) != 0) {

[grt (UTC plus 2), 2014/01/22 21:27:45]

nit: omit braces for single-liners like this here and on lines 84, 88, and 91.

+    return false;
+  }
+  // Sadly at this point, all the read-only attributes have to be removed
+  // manually, even if FOF_NOCOPYSECURITYATTRIBS is set.
+  FileEnumerator f(to_path, true, FileEnumerator::FILES);

[grt (UTC plus 2), 2014/01/22 21:27:45]

it looks like ShellCopy is expected to be called in some situations where
|to_path| exists. this code here is going to modify every file in |to_path|, not
only the ones that were copied. this doesn't seem right.

[M-A Ruel, 2014/01/23 01:00:50]

You are right.

What about I rip out calls to CopyFile and SHFileOperation and use the same
implementation that on posix? It would the advantage to not mess with the
security descriptors or the file bits. It has the disadvantage of being
potentially a tad slower but it's really only used in practice at install time
to copy ~100mb of data or so, or in tests. So doing it in user mode would not be
dramatic vs the way CopyFile may do it slightly more optimized. What do you
think?

+  while (true) {
+    FilePath p = f.Next();
+    if (p.empty()) {
+      break;
+    }
+    DWORD attrs = GetFileAttributes(p.value().c_str());
+    if (attrs == INVALID_FILE_ATTRIBUTES) {
+      continue;
+    }
+    if (attrs & FILE_ATTRIBUTE_READONLY) {
+      SetFileAttributes(p.value().c_str(), attrs & ~FILE_ATTRIBUTE_READONLY);
+    }
+  }
+  return true;
 }
 
 }  // namespace