Fixed copying read-only directories

base/files/file_util_unittest.cc

Index: base/files/file_util_unittest.cc
diff --git a/base/files/file_util_unittest.cc b/base/files/file_util_unittest.cc
index bd4839aad50771e4784594ea526900d857b2ab48..c308656972d40f69b28766e1717a86073ad6afa3 100644
--- a/base/files/file_util_unittest.cc
+++ b/base/files/file_util_unittest.cc
@@ -1379,27 +1379,41 @@ TEST_F(FileUtilTest, CopyDirectoryWithTrailingSeparators) {
 }
 
 // Sets the source file to read-only.
-void SetReadOnly(const FilePath& path) {
+void SetReadOnly(const FilePath& path, bool read_only) {
 #if defined(OS_WIN)
-  // On Windows, it involves setting a bit.
+  // On Windows, it involves setting/removing the 'readonly' bit.
   DWORD attrs = GetFileAttributes(path.value().c_str());
   ASSERT_NE(INVALID_FILE_ATTRIBUTES, attrs);
   ASSERT_TRUE(SetFileAttributes(
-      path.value().c_str(), attrs | FILE_ATTRIBUTE_READONLY));
-  attrs = GetFileAttributes(path.value().c_str());
+      path.value().c_str(),
+      read_only ? (attrs | FILE_ATTRIBUTE_READONLY) :
+          (attrs & ~FILE_ATTRIBUTE_READONLY)));
   // Files in the temporary directory should not be indexed ever. If this
   // assumption change, fix this unit test accordingly.
   // FILE_ATTRIBUTE_NOT_CONTENT_INDEXED doesn't exist on XP.
-  DWORD expected = FILE_ATTRIBUTE_ARCHIVE | FILE_ATTRIBUTE_READONLY;
+  // TODO(ripp@yandex-team.ru): this seems out of place here. If we really think
+  // it is important to verify that temp files are not indexed there should be
+  // a dedicated test for that (create a file, inspect the attributes)
+  DWORD expected = read_only ?
+      ((attrs & (FILE_ATTRIBUTE_ARCHIVE | FILE_ATTRIBUTE_DIRECTORY)) |
+      FILE_ATTRIBUTE_READONLY) :
+          (attrs & (FILE_ATTRIBUTE_ARCHIVE | FILE_ATTRIBUTE_DIRECTORY));

[rvargas (doing something else), 2014/10/23 19:15:34]

nit: should not be extra indentation here.

[ripp, 2014/10/24 05:51:19]

Done.

   if (win::GetVersion() >= win::VERSION_VISTA)
     expected |= FILE_ATTRIBUTE_NOT_CONTENT_INDEXED;
+  attrs = GetFileAttributes(path.value().c_str());
   ASSERT_EQ(expected, attrs);
 #else
-  // On all other platforms, it involves removing the write bit.
-  EXPECT_TRUE(SetPosixFilePermissions(path, S_IRUSR));
+  // On all other platforms, it involves removing/setting the write bit.
+  int mode = read_only ? S_IRUSR : (S_IRUSR | S_IWUSR);
+  EXPECT_TRUE(SetPosixFilePermissions(
+      path,  DirectoryExists(path) ? (mode | S_IXUSR) : mode));
 #endif
 }
 
+void SetReadOnly(const FilePath& path) {

[rvargas (doing something else), 2014/10/23 19:15:34]

I meant we should remove this function.

[ripp, 2014/10/24 05:51:19]

Done.

+  SetReadOnly(path, true);
+}
+
 bool IsReadOnly(const FilePath& path) {
 #if defined(OS_WIN)
   DWORD attrs = GetFileAttributes(path.value().c_str());
@@ -1413,10 +1427,11 @@ bool IsReadOnly(const FilePath& path) {
 }
 
 TEST_F(FileUtilTest, CopyDirectoryACL) {
-  // Create a directory.
+  // Create source directories.
   FilePath src = temp_dir_.path().Append(FILE_PATH_LITERAL("src"));
-  CreateDirectory(src);
-  ASSERT_TRUE(PathExists(src));
+  FilePath src_subdir = src.Append(FILE_PATH_LITERAL("subdir"));
+  CreateDirectory(src_subdir);
+  ASSERT_TRUE(PathExists(src_subdir));
 
   // Create a file under the directory.
   FilePath src_file = src.Append(FILE_PATH_LITERAL("src.txt"));
@@ -1424,12 +1439,22 @@ TEST_F(FileUtilTest, CopyDirectoryACL) {
   SetReadOnly(src_file);
   ASSERT_TRUE(IsReadOnly(src_file));
 
+  // Make directory read-only.
+  SetReadOnly(src_subdir);
+  ASSERT_TRUE(IsReadOnly(src_subdir));
+
   // Copy the directory recursively.
   FilePath dst = temp_dir_.path().Append(FILE_PATH_LITERAL("dst"));
   FilePath dst_file = dst.Append(FILE_PATH_LITERAL("src.txt"));
   EXPECT_TRUE(CopyDirectory(src, dst, true));
 
+  FilePath dst_subdir = dst.Append(FILE_PATH_LITERAL("subdir"));
+  ASSERT_FALSE(IsReadOnly(dst_subdir));
   ASSERT_FALSE(IsReadOnly(dst_file));
+
+  // Give write permissions to allow deletion.
+  SetReadOnly(src_subdir, false);
+  ASSERT_FALSE(IsReadOnly(src_subdir));
 }
 
 TEST_F(FileUtilTest, CopyFile) {