Revert of [GN] Don't rewrite files with the same contents (patchset #5 id:80001 of https://codereview.chromium.org/1656253003/ )

Reason for revert:
Need to revert this patch according https://sites.google.com/a/chromium.org/dev/developers/tree-sheriffs/sheriffing-bug-queues. This patch added a flaky test FilesystemUtils.WriteFileIfChanged.

-----

It's a test flake:
1) Try to find the patch that caused the flake. It should be recent (e.g. last day or two) in all likelihood.
2) If successful with finding that patch, revert the patch. This is especially true if the flake is from a new test introduced in that patch.
3) Close the bug.

-----

The test has failed in the following builds:

http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/176911
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/176911
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/176735
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/176715
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/176715
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/176561
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/176462
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/176375

-----

Example failure:

[ RUN      ] FilesystemUtils.WriteFileIfChanged
../../tools/gn/filesystem_utils_unittest.cc:610: Failure
Expected: (last_modified) != (file_info.last_modified), actual: 2016-02-04 18:06:36.920 UTC vs 2016-02-04 18:06:36.920 UTC
[  FAILED  ] FilesystemUtils.WriteFileIfChanged (3 ms)
[315/315] FilesystemUtils.WriteFileIfChanged (3 ms)
Retrying 1 test (retry #2)
[ RUN      ] FilesystemUtils.WriteFileIfChanged
../../tools/gn/filesystem_utils_unittest.cc:610: Failure
Expected: (last_modified) != (file_info.last_modified), actual: 2016-02-04 18:06:36.936 UTC vs 2016-02-04 18:06:36.936 UTC
[  FAILED  ] FilesystemUtils.WriteFileIfChanged (2 ms)
[316/316] FilesystemUtils.WriteFileIfChanged (2 ms)
Retrying 1 test (retry #3)
[ RUN      ] FilesystemUtils.WriteFileIfChanged
../../tools/gn/filesystem_utils_unittest.cc:610: Failure
Expected: (last_modified) != (file_info.last_modified), actual: 2016-02-04 18:06:36.952 UTC vs 2016-02-04 18:06:36.952 UTC
[  FAILED  ] FilesystemUtils.WriteFileIfChanged (2 ms)
[317/317] FilesystemUtils.WriteFileIfChanged (2 ms)
1 test failed:
    FilesystemUtils.WriteFileIfChanged (../../tools/gn/filesystem_utils_unittest.cc:579)

-----

More details in http://crbug.com/584548.

Original issue's description:
> [GN] Don't rewrite files with the same contents
>
> BUG=
>
> Committed: https://crrev.com/f8ea5cceefcedd4a01935d5ac4d2ba71e23ac13e
> Cr-Commit-Position: refs/heads/master@{#373544}

TBR=brettw@chromium.org,tmoniuszko@opera.com
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=

Review URL: https://codereview.chromium.org/1672783002

Cr-Commit-Position: refs/heads/master@{#373882}

6 files changed, 125 insertions, 156 deletions

diff --git a/tools/gn/filesystem_utils.cc b/tools/gn/filesystem_utils.cc
index 2ceca81..5905cdb 100644
--- a/tools/gn/filesystem_utils.cc
+++ b/tools/gn/filesystem_utils.cc
@@ -687,86 +687,6 @@ std::string GetOutputSubdirName(const Label& toolchain_label, bool is_default) {
   return toolchain_label.name() + "/";
 }
 
-bool ContentsEqual(const base::FilePath& file_path, const std::string& data) {
-  // Compare file and stream sizes first. Quick and will save us some time if
-  // they are different sizes.
-  int64_t file_size;
-  if (!base::GetFileSize(file_path, &file_size) ||
-      static_cast<size_t>(file_size) != data.size()) {
-    return false;
-  }
-
-  std::string file_data;
-  file_data.resize(file_size);
-  if (!base::ReadFileToString(file_path, &file_data))
-    return false;
-
-  return file_data == data;
-}
-
-bool WriteFileIfChanged(const base::FilePath& file_path,
-                        const std::string& data,
-                        Err* err) {
-  if (ContentsEqual(file_path, data))
-    return true;
-
-  // Create the directory if necessary.
-  if (!base::CreateDirectory(file_path.DirName())) {
-    if (err) {
-      *err =
-          Err(Location(), "Unable to create directory.",
-              "I was using \"" + FilePathToUTF8(file_path.DirName()) + "\".");
-    }
-    return false;
-  }
-
-  int size = static_cast<int>(data.size());
-  bool write_success = false;
-
-#if defined(OS_WIN)
-  // On Windows, provide a custom implementation of base::WriteFile. Sometimes
-  // the base version fails, especially on the bots. The guess is that Windows
-  // Defender or other antivirus programs still have the file open (after
-  // checking for the read) when the write happens immediately after. This
-  // version opens with FILE_SHARE_READ (normally not what you want when
-  // replacing the entire contents of the file) which lets us continue even if
-  // another program has the file open for reading. See http://crbug.com/468437
-  base::win::ScopedHandle file(::CreateFile(file_path.value().c_str(),
-                                            GENERIC_WRITE, FILE_SHARE_READ,
-                                            NULL, CREATE_ALWAYS, 0, NULL));
-  if (file.IsValid()) {
-    DWORD written;
-    BOOL result = ::WriteFile(file.Get(), data.c_str(), size, &written, NULL);
-    if (result) {
-      if (static_cast<int>(written) == size) {
-        write_success = true;
-      } else {
-        // Didn't write all the bytes.
-        LOG(ERROR) << "wrote" << written << " bytes to "
-                   << base::UTF16ToUTF8(file_path.value()) << " expected "
-                   << size;
-      }
-    } else {
-      // WriteFile failed.
-      PLOG(ERROR) << "writing file " << base::UTF16ToUTF8(file_path.value())
-                  << " failed";
-    }
-  } else {
-    PLOG(ERROR) << "CreateFile failed for path "
-                << base::UTF16ToUTF8(file_path.value());
-  }
-#else
-  write_success = base::WriteFile(file_path, data.c_str(), size) == size;
-#endif
-
-  if (!write_success && err) {
-    *err = Err(Location(), "Unable to write file.",
-               "I was writing \"" + FilePathToUTF8(file_path) + "\".");
-  }
-
-  return write_success;
-}
-
 SourceDir GetToolchainOutputDir(const Settings* settings) {
   return settings->toolchain_output_subdir().AsSourceDir(
       settings->build_settings());
diff --git a/tools/gn/filesystem_utils.h b/tools/gn/filesystem_utils.h
index aaa08ba..f64c00d 100644
--- a/tools/gn/filesystem_utils.h
+++ b/tools/gn/filesystem_utils.h
@@ -163,18 +163,6 @@ SourceDir SourceDirForCurrentDirectory(const base::FilePath& source_root);
 // go in the root build directory. Otherwise, the result will end in a slash.
 std::string GetOutputSubdirName(const Label& toolchain_label, bool is_default);
 
-// Returns true if the contents of the file and stream given are equal, false
-// otherwise.
-bool ContentsEqual(const base::FilePath& file_path, const std::string& data);
-
-// Writes given stream contents to the given file if it differs from existing
-// file contents. Returns true if new contents was successfully written or
-// existing file contents doesn't need updating, false on write error. |err| is
-// set on write error if not nullptr.
-bool WriteFileIfChanged(const base::FilePath& file_path,
-                        const std::string& data,
-                        Err* err);
-
 // -----------------------------------------------------------------------------
 
 // These functions return the various flavors of output and gen directories.
diff --git a/tools/gn/filesystem_utils_unittest.cc b/tools/gn/filesystem_utils_unittest.cc
index e44d337..27ccab2 100644
--- a/tools/gn/filesystem_utils_unittest.cc
+++ b/tools/gn/filesystem_utils_unittest.cc
@@ -3,11 +3,8 @@
 // found in the LICENSE file.
 
 #include "base/files/file_path.h"
-#include "base/files/file_util.h"
-#include "base/files/scoped_temp_dir.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
-#include "base/threading/platform_thread.h"
 #include "build/build_config.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "tools/gn/filesystem_utils.h"
@@ -557,59 +554,6 @@ TEST(FilesystemUtils, SourceDirForPath) {
 #endif
 }
 
-TEST(FilesystemUtils, ContentsEqual) {
-  base::ScopedTempDir temp_dir;
-  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
-
-  std::string data = "foo";
-
-  base::FilePath file_path = temp_dir.path().AppendASCII("foo.txt");
-  base::WriteFile(file_path, data.c_str(), static_cast<int>(data.size()));
-
-  EXPECT_TRUE(ContentsEqual(file_path, data));
-
-  // Different length and contents.
-  data += "bar";
-  EXPECT_FALSE(ContentsEqual(file_path, data));
-
-  // The same length, different contents.
-  EXPECT_FALSE(ContentsEqual(file_path, "bar"));
-}
-
-TEST(FilesystemUtils, WriteFileIfChanged) {
-  base::ScopedTempDir temp_dir;
-  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
-
-  std::string data = "foo";
-
-  // Write if file doesn't exist. Create also directory.
-  base::FilePath file_path =
-      temp_dir.path().AppendASCII("bar").AppendASCII("foo.txt");
-  EXPECT_TRUE(WriteFileIfChanged(file_path, data, nullptr));
-
-  base::File::Info file_info;
-  ASSERT_TRUE(base::GetFileInfo(file_path, &file_info));
-  base::Time last_modified = file_info.last_modified;
-
-#if defined(OS_MACOSX)
-  // Modification times are in seconds in HFS on Mac.
-  base::TimeDelta sleep_time = base::TimeDelta::FromSeconds(1);
-#else
-  base::TimeDelta sleep_time = base::TimeDelta::FromMilliseconds(1);
-#endif
-  base::PlatformThread::Sleep(sleep_time);
-
-  // Don't write if contents is the same.
-  EXPECT_TRUE(WriteFileIfChanged(file_path, data, nullptr));
-  ASSERT_TRUE(base::GetFileInfo(file_path, &file_info));
-  EXPECT_EQ(last_modified, file_info.last_modified);
-
-  // Write if contents changed.
-  EXPECT_TRUE(WriteFileIfChanged(file_path, "bar", nullptr));
-  ASSERT_TRUE(base::GetFileInfo(file_path, &file_info));
-  EXPECT_NE(last_modified, file_info.last_modified);
-}
-
 TEST(FilesystemUtils, GetToolchainDirs) {
   BuildSettings build_settings;
   build_settings.SetBuildDir(SourceDir("//out/Debug/"));
diff --git a/tools/gn/function_write_file.cc b/tools/gn/function_write_file.cc
index feb1004..9159975 100644
--- a/tools/gn/function_write_file.cc
+++ b/tools/gn/function_write_file.cc
@@ -19,6 +19,55 @@
 
 namespace functions {
 
+namespace {
+
+// On Windows, provide a custom implementation of base::WriteFile. Sometimes
+// the base version fails, especially on the bots. The guess is that Windows
+// Defender or other antivirus programs still have the file open (after
+// checking for the read) when the write happens immediately after. This
+// version opens with FILE_SHARE_READ (normally not what you want when
+// replacing the entire contents of the file) which lets us continue even if
+// another program has the file open for reading. See http://crbug.com/468437
+#if defined(OS_WIN)
+int DoWriteFile(const base::FilePath& filename, const char* data, int size) {
+  base::win::ScopedHandle file(::CreateFile(
+      filename.value().c_str(),
+      GENERIC_WRITE,
+      FILE_SHARE_READ,
+      NULL,
+      CREATE_ALWAYS,
+      0,
+      NULL));
+  if (!file.IsValid()) {
+    PLOG(ERROR) << "CreateFile failed for path "
+                  << base::UTF16ToUTF8(filename.value());
+    return -1;
+  }
+
+  DWORD written;
+  BOOL result = ::WriteFile(file.Get(), data, size, &written, NULL);
+  if (result && static_cast<int>(written) == size)
+    return written;
+
+  if (!result) {
+    // WriteFile failed.
+    PLOG(ERROR) << "writing file " << base::UTF16ToUTF8(filename.value())
+                << " failed";
+  } else {
+    // Didn't write all the bytes.
+    LOG(ERROR) << "wrote" << written << " bytes to "
+               << base::UTF16ToUTF8(filename.value()) << " expected " << size;
+  }
+  return -1;
+}
+#else
+int DoWriteFile(const base::FilePath& filename, const char* data, int size) {
+  return base::WriteFile(filename, data, size);
+}
+#endif
+
+}  // namespace
+
 const char kWriteFile[] = "write_file";
 const char kWriteFile_HelpShort[] =
     "write_file: Write a file to disk.";
@@ -89,14 +138,30 @@ Value RunWriteFile(Scope* scope,
   } else {
     contents << args[1].ToString(false);
   }
-
+  const std::string& new_contents = contents.str();
   base::FilePath file_path =
       scope->settings()->build_settings()->GetFullPath(source_file);
 
   // Make sure we're not replacing the same contents.
-  if (!WriteFileIfChanged(file_path, contents.str(), err))
-    *err = Err(function->function(), err->message(), err->help_text());
+  std::string existing_contents;
+  if (base::ReadFileToString(file_path, &existing_contents) &&
+      existing_contents == new_contents)
+    return Value();  // Nothing to do.
+
+  // Write file, creating the directory if necessary.
+  if (!base::CreateDirectory(file_path.DirName())) {
+    *err = Err(function->function(), "Unable to create directory.",
+               "I was using \"" + FilePathToUTF8(file_path.DirName()) + "\".");
+    return Value();
+  }
 
+  int int_size = static_cast<int>(new_contents.size());
+  if (DoWriteFile(file_path, new_contents.c_str(), int_size)
+      != int_size) {
+    *err = Err(function->function(), "Unable to write file.",
+               "I was writing \"" + FilePathToUTF8(file_path) + "\".");
+    return Value();
+  }
   return Value();
 }
 
diff --git a/tools/gn/ninja_target_writer.cc b/tools/gn/ninja_target_writer.cc
index 9f19b2b..7d9c08a 100644
--- a/tools/gn/ninja_target_writer.cc
+++ b/tools/gn/ninja_target_writer.cc
@@ -74,7 +74,9 @@ void NinjaTargetWriter::RunAndWriteFile(const Target* target) {
     CHECK(0) << "Output type of target not handled.";
   }
 
-  WriteFileIfChanged(ninja_file, file.str(), nullptr);
+  std::string contents = file.str();
+  base::WriteFile(ninja_file, contents.c_str(),
+                  static_cast<int>(contents.size()));
 }
 
 void NinjaTargetWriter::WriteEscapedSubstitution(SubstitutionType type) {
diff --git a/tools/gn/visual_studio_writer.cc b/tools/gn/visual_studio_writer.cc
index dec0837..44a3f11 100644
--- a/tools/gn/visual_studio_writer.cc
+++ b/tools/gn/visual_studio_writer.cc
@@ -9,6 +9,7 @@
 #include <set>
 #include <string>
 
+#include "base/files/file_util.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/strings/string_util.h"
@@ -152,6 +153,26 @@ base::StringPiece FindParentDir(const std::string* path) {
   return base::StringPiece();
 }
 
+bool HasSameContent(std::stringstream& data_1, const base::FilePath& data_2) {
+  // Compare file sizes first. Quick and will save us some time if they are
+  // different sizes.
+  int64_t data_1_len = data_1.tellp();
+
+  int64_t data_2_len;
+  if (!base::GetFileSize(data_2, &data_2_len) || data_1_len != data_2_len)
+    return false;
+
+  std::string data_2_data;
+  data_2_data.resize(data_2_len);
+  if (!base::ReadFileToString(data_2, &data_2_data))
+    return false;
+
+  std::string data_1_data;
+  data_1_data = data_1.str();
+
+  return data_1_data == data_2_data;
+}
+
 }  // namespace
 
 VisualStudioWriter::SolutionEntry::SolutionEntry(const std::string& _name,
@@ -268,13 +289,32 @@ bool VisualStudioWriter::WriteProjectFiles(const Target* target, Err* err) {
   // Only write the content to the file if it's different. That is
   // both a performance optimization and more importantly, prevents
   // Visual Studio from reloading the projects.
-  if (!WriteFileIfChanged(vcxproj_path, vcxproj_string_out.str(), err))
-    return false;
+  if (!HasSameContent(vcxproj_string_out, vcxproj_path)) {
+    std::string content = vcxproj_string_out.str();
+    int size = static_cast<int>(content.size());
+    if (base::WriteFile(vcxproj_path, content.c_str(), size) != size) {
+      *err = Err(Location(), "Couldn't open " + target->label().name() +
+                                 ".vcxproj for writing");
+      return false;
+    }
+  }
 
   base::FilePath filters_path = UTF8ToFilePath(vcxproj_path_str + ".filters");
+
   std::stringstream filters_string_out;
   WriteFiltersFileContents(filters_string_out, target);
-  return WriteFileIfChanged(filters_path, filters_string_out.str(), err);
+
+  if (!HasSameContent(filters_string_out, filters_path)) {
+    std::string content = filters_string_out.str();
+    int size = static_cast<int>(content.size());
+    if (base::WriteFile(filters_path, content.c_str(), size) != size) {
+      *err = Err(Location(), "Couldn't open " + target->label().name() +
+                                 ".vcxproj.filters for writing");
+      return false;
+    }
+  }
+
+  return true;
 }
 
 bool VisualStudioWriter::WriteProjectFileContents(
@@ -572,7 +612,17 @@ bool VisualStudioWriter::WriteSolutionFile(Err* err) {
   // Only write the content to the file if it's different. That is
   // both a performance optimization and more importantly, prevents
   // Visual Studio from reloading the projects.
-  return WriteFileIfChanged(sln_path, string_out.str(), err);
+  if (HasSameContent(string_out, sln_path))
+    return true;
+
+  std::string content = string_out.str();
+  int size = static_cast<int>(content.size());
+  if (base::WriteFile(sln_path, content.c_str(), size) != size) {
+    *err = Err(Location(), "Couldn't open all.sln for writing");
+    return false;
+  }
+
+  return true;
 }
 
 void VisualStudioWriter::WriteSolutionFileContents(