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

util/test/scoped_temp_dir_win.cc

 // Copyright 2015 The Crashpad Authors. All rights reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at
 //
 //     http://www.apache.org/licenses/LICENSE-2.0
 //
 // Unless required by applicable law or agreed to in writing, software
 // distributed under the License is distributed on an "AS IS" BASIS,
@@ skipping 36 matching lines @@
     // Try to move to a new temporary directory with a randomly generated name.
     // If the one we try exists, retry with a new name until we reach some
     // limit.
     base::FilePath target_path = GenerateCandidateName();
     if (MoveFileEx(path_.value().c_str(), target_path.value().c_str(), 0)) {
       path_ = target_path;
       return;
     }
   }
 
   CHECK(false) << "Couldn't find temp dir name";

[Peter Kasting, 2015/02/13 00:32:41]

Nit: "Couldn't create a unique temp dir name" might be slightly clearer.  (2
places)

[scottmg, 2015/02/13 06:34:05]

Done.

 }
 
 // static
 base::FilePath ScopedTempDir::CreateTemporaryDirectory() {
   for (int count = 0; count < kRetries; ++count) {
     // Try to create a new temporary directory with random generated name. If
     // the one we generate exists, keep trying another path name until we reach
     // some limit.
     base::FilePath path_to_create = GenerateCandidateName();
     if (CreateDirectory(path_to_create.value().c_str(), NULL))
       return path_to_create;
   }
 
   CHECK(false) << "Couldn't find temp dir name";
   return base::FilePath();
 }
 
 // static
 void ScopedTempDir::RecursivelyDeleteTemporaryDirectory(
     const base::FilePath& path) {
   const std::wstring all_files_mask(L"\\*");
 
   std::wstring 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);

[Peter Kasting, 2015/02/13 00:32:41]

Nit: (expected, actual) for gtest ASSERT/EXPECT_EQ/NE (2 places)

[scottmg, 2015/02/13 06:34:05]

Done.

     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;

[Peter Kasting, 2015/02/13 00:32:40]

Nit: Shorter:

      if (find_data.dwFileAttributes &
          (FILE_ATTRIBUTE_DIRECTORY | FILE_ATTRIBUTE_REPARSE_POINT)) {

[scottmg, 2015/02/13 06:34:06]

Done.

       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)) {

[Peter Kasting, 2015/02/13 00:32:41]

Nit: This can be used as a loop condition, and the EXPECT moved below the loop. 
That in turn would allow you to "continue" in the case of "." and ".." like you
did in the Linux code, unindenting the rest of the loop one level:

  do {
    if (...)
      continue;
    ...
  } while (FindNextFile(search_handle, &find_data));
  EXPECT_EQ(ERROR_NO_MORE_FILES, GetLastError());

[scottmg, 2015/02/13 06:34:06]

Done.

       EXPECT_EQ(GetLastError(), ERROR_NO_MORE_FILES);
       break;
     }
   }
 
   EXPECT_TRUE(FindClose(search_handle));
   EXPECT_TRUE(RemoveDirectory(path.value().c_str()));
 }
 
 }  // namespace test
 }  // namespace crashpad
+// EOF comment added for readability review.