Improve logging when the installer fails to create a directory.

base::CreateDirectoryAndGetError now ensures that the last error code on
Windows is relevant so that callers may meaningfully use the PLOG family
of macros to get readable error messages.

BUG=874037
R=gab@chromium.org

Change-Id: I6d426c57e0a0ae101e13ebdbe34815ec40801b1e
Reviewed-on: https://chromium-review.googlesource.com/1175782
Commit-Queue: Greg Thompson <grt@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583831}

base/files/file_util_win.cc

@@ -598,8 +598,8 @@ bool CreateDirectoryAndGetError(const FilePath& full_path,
   AssertBlockingAllowed();
 
   // If the path exists, we've succeeded if it's a directory, failed otherwise.
-  const wchar_t* full_path_str = full_path.value().c_str();
-  DWORD fileattr = ::GetFileAttributes(full_path_str);
+  const wchar_t* const full_path_str = full_path.value().c_str();
+  const DWORD fileattr = ::GetFileAttributes(full_path_str);
   if (fileattr != INVALID_FILE_ATTRIBUTES) {
     if ((fileattr & FILE_ATTRIBUTE_DIRECTORY) != 0) {
       DVLOG(1) << "CreateDirectory(" << full_path_str << "), "
@@ -608,9 +608,9 @@ bool CreateDirectoryAndGetError(const FilePath& full_path,
     }
     DLOG(WARNING) << "CreateDirectory(" << full_path_str << "), "
                   << "conflicts with existing file.";
-    if (error) {
+    if (error)
       *error = File::FILE_ERROR_NOT_A_DIRECTORY;
-    }
+    ::SetLastError(ERROR_FILE_EXISTS);
     return false;
   }
 
@@ -621,37 +621,33 @@ bool CreateDirectoryAndGetError(const FilePath& full_path,
   // directories starting with the highest-level missing parent.
   FilePath parent_path(full_path.DirName());
   if (parent_path.value() == full_path.value()) {
-    if (error) {
+    if (error)
       *error = File::FILE_ERROR_NOT_FOUND;
-    }
+    ::SetLastError(ERROR_FILE_NOT_FOUND);
     return false;
   }
   if (!CreateDirectoryAndGetError(parent_path, error)) {
     DLOG(WARNING) << "Failed to create one of the parent directories.";
-    if (error) {
-      DCHECK(*error != File::FILE_OK);
-    }
+    DCHECK(!error || *error != File::FILE_OK);
     return false;
   }
 
-  if (!::CreateDirectory(full_path_str, NULL)) {
-    DWORD error_code = ::GetLastError();
+  if (::CreateDirectory(full_path_str, NULL))
+    return true;
+
+  const DWORD error_code = ::GetLastError();
   if (error_code == ERROR_ALREADY_EXISTS && DirectoryExists(full_path)) {
-      // This error code ERROR_ALREADY_EXISTS doesn't indicate whether we
-      // were racing with someone creating the same directory, or a file
-      // with the same path.  If DirectoryExists() returns true, we lost the
-      // race to create the same directory.
+    // This error code ERROR_ALREADY_EXISTS doesn't indicate whether we were
+    // racing with someone creating the same directory, or a file with the same
+    // path.  If DirectoryExists() returns true, we lost the race to create the
+    // same directory.
     return true;
-    } else {
+  }
   if (error)
     *error = File::OSErrorToFileError(error_code);
-      DLOG(WARNING) << "Failed to create directory " << full_path_str
-                    << ", last error is " << error_code << ".";
+  ::SetLastError(error_code);
+  DPLOG(WARNING) << "Failed to create directory " << full_path_str;
   return false;
-    }
-  } else {
-    return true;
-  }
 }
 
 bool NormalizeFilePath(const FilePath& path, FilePath* real_path) {

chrome/installer/util/create_dir_work_item.cc

@@ -35,9 +35,12 @@ bool CreateDirWorkItem::DoImpl() {
   if (top_path_.empty())
     return true;
 
-  VLOG(1) << "top directory that needs to be created: " << top_path_.value();
+  VLOG(1) << "Top directory that needs to be created: " << top_path_.value();
   bool result = base::CreateDirectory(path_);
-  VLOG(1) << "directory creation result: " << result;
+  if (result)
+    VLOG(1) << "Created directory";
+  else
+    PLOG(ERROR) << "Failed to create directory " << top_path_.value();
 
   rollback_needed_ = true;