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..df67a4fb75a31b4d9d4b2fc6917ccbb51d52cb0f 100644
--- a/base/files/file_util_unittest.cc
+++ b/base/files/file_util_unittest.cc
@@ -1379,24 +1379,34 @@ 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

[palmer, 2014/10/24 18:26:46]

Nit: Move this comment to just above line 1401.

[ripp, 2014/10/27 08:57:06]

Done.

+  // 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));
   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);

[palmer, 2014/10/24 18:26:46]

mode_t, not int

[ripp, 2014/10/27 08:57:06]

Done.

+  EXPECT_TRUE(SetPosixFilePermissions(
+      path, DirectoryExists(path) ? (mode | S_IXUSR) : mode));
 #endif
 }
 
@@ -1413,23 +1423,34 @@ 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"));
   CreateTextFile(src_file, L"Gooooooooooooooooooooogle");
-  SetReadOnly(src_file);
+  SetReadOnly(src_file, true);
   ASSERT_TRUE(IsReadOnly(src_file));
 
+  // Make directory read-only.
+  SetReadOnly(src_subdir, true);
+  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) {
@@ -1480,7 +1501,7 @@ TEST_F(FileUtilTest, CopyFileACL) {
 
   // Set the source file to read-only.
   ASSERT_FALSE(IsReadOnly(src));
-  SetReadOnly(src);
+  SetReadOnly(src, true);
   ASSERT_TRUE(IsReadOnly(src));
 
   // Copy the file.