summaryrefslogtreecommitdiffstats
path: root/base/file_util_win.cc
diff options
context:
space:
mode:
authorgrt@chromium.org <grt@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-03-13 04:47:27 +0000
committergrt@chromium.org <grt@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-03-13 04:47:27 +0000
commit8aad6d3f1d940536528fd92ffa390ed2ff39bae7 (patch)
treeb90b03e3ff3517e53558c336f70b2990ff3b907d /base/file_util_win.cc
parent2b5ce54ecefe55b03c40f3d9e143a988ea994d65 (diff)
downloadchromium_src-8aad6d3f1d940536528fd92ffa390ed2ff39bae7.zip
chromium_src-8aad6d3f1d940536528fd92ffa390ed2ff39bae7.tar.gz
chromium_src-8aad6d3f1d940536528fd92ffa390ed2ff39bae7.tar.bz2
Fix race in file_util::ReplaceFile on Windows.
The previous implementation left a window where an empty destination file had been created yet the replace hadn't yet happened. This was causing flakes in the Chrome Frame integration tests since chrome.exe is killed shortly after startup over and over again. When the stars aligned (more often than you may think), ReplaceFile would have time to create an empty Preferences file, but didn't have time to complete the ReplaceFile. Subsequent tests would then fail because an empty prefs file is an invalid prefs file. BUG=81479 TEST=none Review URL: http://codereview.chromium.org/9689013 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@126334 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'base/file_util_win.cc')
-rw-r--r--base/file_util_win.cc30
1 files changed, 13 insertions, 17 deletions
diff --git a/base/file_util_win.cc b/base/file_util_win.cc
index db21d32..7ea1aa3 100644
--- a/base/file_util_win.cc
+++ b/base/file_util_win.cc
@@ -177,23 +177,19 @@ bool Move(const FilePath& from_path, const FilePath& to_path) {
bool ReplaceFile(const FilePath& from_path, const FilePath& to_path) {
base::ThreadRestrictions::AssertIOAllowed();
-
- // Make sure that the target file exists.
- HANDLE target_file = ::CreateFile(
- to_path.value().c_str(),
- 0,
- FILE_SHARE_READ | FILE_SHARE_WRITE,
- NULL,
- CREATE_NEW,
- FILE_ATTRIBUTE_NORMAL,
- NULL);
- if (target_file != INVALID_HANDLE_VALUE)
- ::CloseHandle(target_file);
- // When writing to a network share, we may not be able to change the ACLs.
- // Ignore ACL errors then (REPLACEFILE_IGNORE_MERGE_ERRORS).
- return ::ReplaceFile(to_path.value().c_str(),
- from_path.value().c_str(), NULL,
- REPLACEFILE_IGNORE_MERGE_ERRORS, NULL, NULL) ? true : false;
+ // Try a simple move first. It will only succeed when |to_path| doesn't
+ // already exist.
+ if (::MoveFile(from_path.value().c_str(), to_path.value().c_str()))
+ return true;
+ // Try the full-blown replace if the move fails, as ReplaceFile will only
+ // succeed when |to_path| does exist. When writing to a network share, we may
+ // not be able to change the ACLs. Ignore ACL errors then
+ // (REPLACEFILE_IGNORE_MERGE_ERRORS).
+ if (::ReplaceFile(to_path.value().c_str(), from_path.value().c_str(), NULL,
+ REPLACEFILE_IGNORE_MERGE_ERRORS, NULL, NULL)) {
+ return true;
+ }
+ return false;
}
bool CopyFile(const FilePath& from_path, const FilePath& to_path) {