Use GUID to generate unique temp file names and retire GetTempFileName

base/files/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/files/file_util.h"
 
 #include <windows.h>
 #include <io.h>
 #include <psapi.h>
 #include <shellapi.h>
@@ skipping 22 matching lines @@
 #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;
 
+// An array consists of digits and alphabet. It is used to generate random
+// prefix of a temp file name which is passed to GetTempFileName Windows API.
+constexpr base::FilePath::CharType kAlphaNumberCharSet[] =
+    FILE_PATH_LITERAL("0123456789abcdefghijklmnopqrstuvwxyz");
+
+// The length of the temp file name prefix, which is used by GetTempFileName
+// Windows API.
+// GetTempFileName uses up to the first three characters of the input
+// prefix string as the prefix of the file name. Therefore, this constant is
+// set to three.
+// https://msdn.microsoft.com/en-us/library/windows/desktop/aa364991(v=vs.85).aspx
+constexpr int kPrefixLength = 3;
+
+// Get a random prefix with length kPrefixLength of the temp file name.
+// The characters are obtained from kAlphaNumberCharSet.
+FilePath::StringType GetRandomPrefix() {
+  FilePath::StringType temp_file_name_prefix;

[grt (UTC plus 2), 2017/04/03 21:11:15]

always prefer constructing to the desired size over constructing then resizing:
  FilePath::StringType temp_file_name_prefix(kPrefixLength, 0);

[chengx, 2017/04/04 01:25:54]

Done.

+  temp_file_name_prefix.resize(kPrefixLength);
+
+  for (size_t i = 0; i < kPrefixLength; i++)

[grt (UTC plus 2), 2017/04/03 21:11:15]

nit: add braces. alternatively, use std::generate to fill temp_file_name_prefix

[chengx, 2017/04/04 01:25:54]

Done.

+    temp_file_name_prefix[i] = kAlphaNumberCharSet[base::RandGenerator(
+        arraysize(kAlphaNumberCharSet))];

[grt (UTC plus 2), 2017/04/03 21:11:15]

arraysize - 1 to omit the trailing string terminator

[chengx, 2017/04/04 01:25:54]

Done.

+
+  return temp_file_name_prefix;
+}
+
 // Deletes all files and directories in a path.
 // Returns false on the first failure it encounters.
 bool DeleteFileRecursive(const FilePath& path,
                          const FilePath::StringType& pattern,
                          bool recursive) {
   FileEnumerator traversal(path, false,
                            FileEnumerator::FILES | FileEnumerator::DIRECTORIES,
                            pattern);
   for (FilePath current = traversal.Next(); !current.empty();
        current = traversal.Next()) {
@@ skipping 282 matching lines @@
   // it replaces \n's with \r\n's, which may surprise you.
   // Reference: http://msdn.microsoft.com/en-us/library/h9t88zwz(VS.71).aspx
   return OpenFile(*path, "wb+");
 }
 
 bool CreateTemporaryFileInDir(const FilePath& dir, FilePath* temp_file) {
   ThreadRestrictions::AssertIOAllowed();
 
   wchar_t temp_name[MAX_PATH + 1];
 
-  if (!GetTempFileName(dir.value().c_str(), L"", 0, temp_name)) {
+  // Due to the algorithm used to generate file names, GetTempFileName Windows
+  // API can perform poorly when creating a large number of files with the same
+  // prefix. Therefore, making the prefix random should boost its performance.

[grt (UTC plus 2), 2017/04/03 21:11:15]

re: "should": have you verified that filling a directory with a bajillion FOO*
files and then creating a temp file with the prefix FOO is slower than creating
a file with the prefix BAR? in other words, is the problem that there are many
files with the same prefix as the desired temp file, or is it that there are
many files period?

[chengx, 2017/04/04 01:25:54]

I should remove "should" probably. The main issue is that there are simply many
files there in the target Downloads folder. In this case, checking the
uniqueness of a newly generated name is expensive, because it needs to loop over
all the files. There is nothing we can do here. The users need to delete them
all. 

The problem of GetTempFileName is that it can only generate 16^4=65536 files,
and this limit is not that hard to hit. When the temp files in the folder
accumulates, the chance of failing to generate an unique name increases and the
uniqueness check needs to be done for more than once, maybe a few times. This
makes things worse. I think this CL can fix this issue, as now the file limit
has been increased to 36^3 times larger. Using GUID is another option to fix
this issue.  

+  // https://msdn.microsoft.com/en-us/library/windows/desktop/aa364991(v=vs.85).aspx
+
+  if (!GetTempFileName(dir.value().c_str(), GetRandomPrefix().c_str(), 0,
+                       temp_name)) {
     DPLOG(WARNING) << "Failed to get temporary file name in "
                    << UTF16ToUTF8(dir.value());
     return false;
   }
 
   wchar_t long_temp_name[MAX_PATH + 1];
   DWORD long_name_len = GetLongPathName(temp_name, long_temp_name, MAX_PATH);
   if (long_name_len > MAX_PATH || long_name_len == 0) {
     // GetLongPathName() failed, but we still have a temporary file.
     *temp_file = FilePath(temp_name);
@@ skipping 486 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