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

base/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/file_util.h"
 
 #include <windows.h>
 #include <psapi.h>
 #include <shellapi.h>
 #include <shlobj.h>
 #include <time.h>
 
 #include <algorithm>
 #include <limits>
 #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"
 #include "base/rand_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/time/time.h"
 #include "base/win/scoped_handle.h"
 #include "base/win/windows_version.h"
 
 namespace base {
 
 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) {
   // WinXP SHFileOperation doesn't like trailing separators.
   FilePath stripped_from = from_path.StripTrailingSeparators();
   FilePath stripped_to = to_path.StripTrailingSeparators();
 
   ThreadRestrictions::AssertIOAllowed();
 
   // NOTE: I suspect we could support longer paths, but that would involve
@@ skipping 11 matching lines @@
 #pragma warning(suppress:4996)  // don't complain about wcscpy deprecation
   wcscpy(double_terminated_path_from, stripped_from.value().c_str());
 #pragma warning(suppress:4996)  // don't complain about wcscpy deprecation
   wcscpy(double_terminated_path_to, stripped_to.value().c_str());
 
   SHFILEOPSTRUCT file_operation = {0};
   file_operation.wFunc = FO_COPY;
   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
 
 FilePath MakeAbsoluteFilePath(const FilePath& input) {
   ThreadRestrictions::AssertIOAllowed();
   wchar_t file_path[MAX_PATH];
   if (!_wfullpath(file_path, input.value().c_str(), MAX_PATH))
     return FilePath();
   return FilePath(file_path);
@@ skipping 675 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