Use GUID to generate unique temp file names and retire GetTempFileName

base/files/file_util_win.cc

Index: base/files/file_util_win.cc
diff --git a/base/files/file_util_win.cc b/base/files/file_util_win.cc
index 65dc5ce1e2fe7495624cc6d8c16b954395eb11ac..2c6c5304d91d549fc6e2584dc9232a81f77487ab 100644
--- a/base/files/file_util_win.cc
+++ b/base/files/file_util_win.cc
@@ -40,6 +40,32 @@ 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,
@@ -342,7 +368,13 @@ bool CreateTemporaryFileInDir(const FilePath& dir, FilePath* temp_file) {
 
   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;