win: Implementation of CrashReportDatabase for Windows (for C++ Windows readability review)

util/test/scoped_temp_dir_win.cc

Index: util/test/scoped_temp_dir_win.cc
diff --git a/util/test/scoped_temp_dir_win.cc b/util/test/scoped_temp_dir_win.cc
index f641ec2b750d9adb160692e66e4f2ba5edd99bec..cdadd4f18ed913485f2cf38097f6ce6f39d55c0b 100644
--- a/util/test/scoped_temp_dir_win.cc
+++ b/util/test/scoped_temp_dir_win.cc
@@ -54,7 +54,7 @@ void ScopedTempDir::Rename() {
     }
   }
 
-  CHECK(false) << "Couldn't find temp dir name";
+  CHECK(false) << "Couldn't move to a new unique temp dir";
 }
 
 // static
@@ -68,40 +68,33 @@ base::FilePath ScopedTempDir::CreateTemporaryDirectory() {
       return path_to_create;
   }
 
-  CHECK(false) << "Couldn't find temp dir name";
+  CHECK(false) << "Couldn't create a new unique temp dir";
   return base::FilePath();
 }
 
 // static
 void ScopedTempDir::RecursivelyDeleteTemporaryDirectory(
     const base::FilePath& path) {
-  const std::wstring all_files_mask(L"\\*");
+  const base::string16 all_files_mask(L"\\*");
 
-  std::wstring search_mask = path.value() + all_files_mask;
+  base::string16 search_mask = path.value() + all_files_mask;
   WIN32_FIND_DATA find_data;
   HANDLE search_handle = FindFirstFile(search_mask.c_str(), &find_data);
-  if (search_handle == INVALID_HANDLE_VALUE) {
-    ASSERT_EQ(GetLastError(), ERROR_FILE_NOT_FOUND);
-    return;
-  }
-  for (;;) {
-    if (wcscmp(find_data.cFileName, L".") != 0 &&
-        wcscmp(find_data.cFileName, L"..") != 0) {
-      bool is_dir =
-          (find_data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) != 0 ||
-          (find_data.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) != 0;
-      base::FilePath entry_path = path.Append(find_data.cFileName);
-      if (is_dir)
-        RecursivelyDeleteTemporaryDirectory(entry_path);
-      else
-        EXPECT_TRUE(DeleteFile(entry_path.value().c_str()));
-    }
-
-    if (!FindNextFile(search_handle, &find_data)) {
-      EXPECT_EQ(GetLastError(), ERROR_NO_MORE_FILES);
-      break;
+  if (search_handle == INVALID_HANDLE_VALUE)
+    ASSERT_EQ(ERROR_FILE_NOT_FOUND, GetLastError());
+  do {
+    if (wcscmp(find_data.cFileName, L".") == 0 ||
+        wcscmp(find_data.cFileName, L"..") == 0) {
+      continue;
     }
-  }
+    base::FilePath entry_path = path.Append(find_data.cFileName);
+    ASSERT_FALSE(find_data.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT);

[Mark Mentovai, 2015/02/17 19:40:40]

It’s OK if this was set on a directory, so if you want to move this ASSERT into
the else branch, I’d be fine with that too.

[scottmg, 2015/02/17 19:52:20]

The directory enumeration code in chromium notes that it doesn't follow reparse
directories because they can form a cycle/hang (or stack overflow in this case).
So I think I'll just leave it as the more conservative version.

+    if (find_data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
+      RecursivelyDeleteTemporaryDirectory(entry_path);
+    else
+      EXPECT_TRUE(DeleteFile(entry_path.value().c_str()));
+  } while (FindNextFile(search_handle, &find_data));
+  EXPECT_EQ(ERROR_NO_MORE_FILES, GetLastError());
 
   EXPECT_TRUE(FindClose(search_handle));
   EXPECT_TRUE(RemoveDirectory(path.value().c_str()));