Make FileVersionInfo return a unique_ptr.

Bug: none
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: Id324ad0abc178937ce373e03180d2135fe3d2c97
Reviewed-on: https://chromium-review.googlesource.com/c/1235113
Commit-Queue: David Benjamin <davidben@chromium.org>
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Reviewed-by: Rebekah Potter <rbpotter@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Reviewed-by: Albert J. Wong <ajwong@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626899}

base/file_version_info.h

@@ -5,6 +5,7 @@
 #ifndef BASE_FILE_VERSION_INFO_H_
 #define BASE_FILE_VERSION_INFO_H_
 
+#include <memory>
 #include <string>
 
 #include "build/build_config.h"
@@ -33,21 +34,23 @@ class BASE_EXPORT FileVersionInfo {
  public:
   virtual ~FileVersionInfo() {}
 #if defined(OS_WIN) || defined(OS_MACOSX)
-  // Creates a FileVersionInfo for the specified path. Returns NULL if something
-  // goes wrong (typically the file does not exit or cannot be opened). The
-  // returned object should be deleted when you are done with it.
-  static FileVersionInfo* CreateFileVersionInfo(
+  // Creates a FileVersionInfo for the specified path. Returns nullptr if
+  // something goes wrong (typically the file does not exit or cannot be
+  // opened).
+  static std::unique_ptr<FileVersionInfo> CreateFileVersionInfo(
       const base::FilePath& file_path);
 #endif  // OS_WIN || OS_MACOSX
 
 #if defined(OS_WIN)
-  // Creates a FileVersionInfo for the specified module. Returns NULL in case
-  // of error. The returned object should be deleted when you are done with it.
-  static FileVersionInfo* CreateFileVersionInfoForModule(HMODULE module);
+  // Creates a FileVersionInfo for the specified module. Returns nullptr in
+  // case of error.
+  static std::unique_ptr<FileVersionInfo> CreateFileVersionInfoForModule(
+      HMODULE module);
 #else
-  // Creates a FileVersionInfo for the current module. Returns NULL in case
-  // of error. The returned object should be deleted when you are done with it.
-  static FileVersionInfo* CreateFileVersionInfoForCurrentModule();
+  // Creates a FileVersionInfo for the current module. Returns nullptr in case
+  // of error.
+  static std::unique_ptr<FileVersionInfo>
+  CreateFileVersionInfoForCurrentModule();
 #endif  // OS_WIN
 
   // Accessors to the different version properties.

base/file_version_info_mac.mm

@@ -20,16 +20,17 @@ FileVersionInfoMac::FileVersionInfoMac(NSBundle *bundle)
 FileVersionInfoMac::~FileVersionInfoMac() {}
 
 // static
-FileVersionInfo* FileVersionInfo::CreateFileVersionInfoForCurrentModule() {
+std::unique_ptr<FileVersionInfo>
+FileVersionInfo::CreateFileVersionInfoForCurrentModule() {
   return CreateFileVersionInfo(base::mac::FrameworkBundlePath());
 }
 
 // static
-FileVersionInfo* FileVersionInfo::CreateFileVersionInfo(
+std::unique_ptr<FileVersionInfo> FileVersionInfo::CreateFileVersionInfo(
     const base::FilePath& file_path) {
   NSString* path = base::SysUTF8ToNSString(file_path.value());
   NSBundle* bundle = [NSBundle bundleWithPath:path];
-  return new FileVersionInfoMac(bundle);
+  return std::make_unique<FileVersionInfoMac>(bundle);
 }
 
 base::string16 FileVersionInfoMac::company_name() {

base/file_version_info_win.cc

@@ -9,6 +9,7 @@
 
 #include "base/files/file_path.h"
 #include "base/logging.h"
+#include "base/memory/ptr_util.h"
 #include "base/stl_util.h"
 #include "base/threading/scoped_blocking_call.h"
 #include "base/win/resource_util.h"
@@ -49,8 +50,8 @@ VS_FIXEDFILEINFO* GetVsFixedFileInfo(const void* data) {
 FileVersionInfoWin::~FileVersionInfoWin() = default;
 
 // static
-FileVersionInfo* FileVersionInfo::CreateFileVersionInfoForModule(
-    HMODULE module) {
+std::unique_ptr<FileVersionInfo>
+FileVersionInfo::CreateFileVersionInfoForModule(HMODULE module) {
   void* data;
   size_t version_info_length;
   const bool has_version_resource = base::win::GetResourceFromModule(
@@ -62,13 +63,19 @@ FileVersionInfo* FileVersionInfo::CreateFileVersionInfoForModule(
   if (!translate)
     return nullptr;
 
-  return new FileVersionInfoWin(data, translate->language,
-                                translate->code_page);
+  return base::WrapUnique(
+      new FileVersionInfoWin(data, translate->language, translate->code_page));
 }
 
 // static
-FileVersionInfo* FileVersionInfo::CreateFileVersionInfo(
+std::unique_ptr<FileVersionInfo> FileVersionInfo::CreateFileVersionInfo(
     const FilePath& file_path) {
+  return FileVersionInfoWin::CreateFileVersionInfoWin(file_path);
+}
+
+// static
+std::unique_ptr<FileVersionInfoWin>
+FileVersionInfoWin::CreateFileVersionInfoWin(const FilePath& file_path) {
   base::ScopedBlockingCall scoped_blocking_call(base::BlockingType::MAY_BLOCK);
 
   DWORD dummy;
@@ -86,8 +93,8 @@ FileVersionInfo* FileVersionInfo::CreateFileVersionInfo(
   if (!translate)
     return nullptr;
 
-  return new FileVersionInfoWin(std::move(data), translate->language,
-                                translate->code_page);
+  return base::WrapUnique(new FileVersionInfoWin(
+      std::move(data), translate->language, translate->code_page));
 }
 
 base::string16 FileVersionInfoWin::company_name() {

base/file_version_info_win.h

@@ -53,6 +53,10 @@ class BASE_EXPORT FileVersionInfoWin : public FileVersionInfo {
   // Get the fixed file info if it exists. Otherwise NULL
   const VS_FIXEDFILEINFO* fixed_file_info() const { return fixed_file_info_; }
 
+  // Behaves like CreateFileVersionInfo, but returns a FileVersionInfoWin.
+  static std::unique_ptr<FileVersionInfoWin> CreateFileVersionInfoWin(
+      const base::FilePath& file_path);
+
  private:
   friend FileVersionInfo;
 

base/file_version_info_win_unittest.cc

@@ -13,7 +13,6 @@
 #include "base/file_version_info.h"
 #include "base/files/file_path.h"
 #include "base/macros.h"
-#include "base/memory/ptr_util.h"
 #include "base/path_service.h"
 #include "base/scoped_native_library.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -37,7 +36,7 @@ class FileVersionInfoFactory {
   explicit FileVersionInfoFactory(const FilePath& path) : path_(path) {}
 
   std::unique_ptr<FileVersionInfo> Create() const {
-    return base::WrapUnique(FileVersionInfo::CreateFileVersionInfo(path_));
+    return FileVersionInfo::CreateFileVersionInfo(path_);
   }
 
  private:
@@ -58,8 +57,7 @@ class FileVersionInfoForModuleFactory {
   }
 
   std::unique_ptr<FileVersionInfo> Create() const {
-    return base::WrapUnique(
-        FileVersionInfo::CreateFileVersionInfoForModule(library_.get()));
+    return FileVersionInfo::CreateFileVersionInfoForModule(library_.get());
   }
 
  private:

base/win/windows_version.cc

@@ -194,17 +194,15 @@ Version OSInfo::Kernel32Version() const {
 // kernel32 will still be the "real" version.
 base::Version OSInfo::Kernel32BaseVersion() const {
   static const base::NoDestructor<base::Version> version([] {
-    std::unique_ptr<FileVersionInfoWin> file_version_info(
-        static_cast<FileVersionInfoWin*>(
-            FileVersionInfoWin::CreateFileVersionInfo(
-                base::FilePath(FILE_PATH_LITERAL("kernel32.dll")))));
+    std::unique_ptr<FileVersionInfoWin> file_version_info =
+        FileVersionInfoWin::CreateFileVersionInfoWin(
+            base::FilePath(FILE_PATH_LITERAL("kernel32.dll")));
     if (!file_version_info) {
       // crbug.com/912061: on some systems it seems kernel32.dll might be
       // corrupted or not in a state to get version info. In this case try
       // kernelbase.dll as a fallback.
-      file_version_info.reset(static_cast<FileVersionInfoWin*>(
-          FileVersionInfoWin::CreateFileVersionInfo(
-              base::FilePath(FILE_PATH_LITERAL("kernelbase.dll")))));
+      file_version_info = FileVersionInfoWin::CreateFileVersionInfoWin(
+          base::FilePath(FILE_PATH_LITERAL("kernelbase.dll")));
     }
     CHECK(file_version_info);
     const int major =

cloud_print/common/win/install_utils.cc

@@ -7,7 +7,7 @@
 #include <windows.h>
 
 #include "base/command_line.h"
-#include "base/file_version_info_win.h"
+#include "base/file_version_info.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/path_service.h"
@@ -60,12 +60,10 @@ void SetGoogleUpdateKeys(const base::string16& product_id,
 
   // Get the version from the resource file.
   base::string16 version_string;
-  std::unique_ptr<FileVersionInfo> version_info(
-      FileVersionInfo::CreateFileVersionInfoForModule(CURRENT_MODULE()));
-  if (version_info.get()) {
-    FileVersionInfoWin* version_info_win =
-        static_cast<FileVersionInfoWin*>(version_info.get());
-    version_string = version_info_win->product_version();
+  std::unique_ptr<FileVersionInfo> version_info =
+      FileVersionInfo::CreateFileVersionInfoForModule(CURRENT_MODULE());
+  if (version_info) {
+    version_string = version_info->product_version();
   } else {
     LOG(ERROR) << "Unable to get version string";
     // Use a random version string so that Google Update has something to go by.
@@ -149,14 +147,12 @@ void CreateUninstallKey(const base::string16& uninstall_id,
   key.WriteValue(kInstallLocation, unstall_binary.DirName().value().c_str());
 
   // Get the version resource.
-  std::unique_ptr<FileVersionInfo> version_info(
-      FileVersionInfo::CreateFileVersionInfoForModule(CURRENT_MODULE()));
+  std::unique_ptr<FileVersionInfo> version_info =
+      FileVersionInfo::CreateFileVersionInfoForModule(CURRENT_MODULE());
 
-  if (version_info.get()) {
-    FileVersionInfoWin* version_info_win =
-        static_cast<FileVersionInfoWin*>(version_info.get());
-    key.WriteValue(kDisplayVersion, version_info_win->file_version().c_str());
-    key.WriteValue(kPublisher, version_info_win->company_name().c_str());
+  if (version_info) {
+    key.WriteValue(kDisplayVersion, version_info->file_version().c_str());
+    key.WriteValue(kPublisher, version_info->company_name().c_str());
   } else {
     LOG(ERROR) << "Unable to get version string";
   }

gpu/config/gpu_info_collector_win.cc

@@ -320,18 +320,17 @@ bool BadAMDVulkanDriverVersion() {
   // 32-bit dll will be used to detect the AMD Vulkan driver.
   const base::FilePath kAmdDriver64(FILE_PATH_LITERAL("amdvlk64.dll"));
   const base::FilePath kAmdDriver32(FILE_PATH_LITERAL("amdvlk32.dll"));
-  auto file_version_info =
-      base::WrapUnique(FileVersionInfoWin::CreateFileVersionInfo(kAmdDriver64));
+  std::unique_ptr<FileVersionInfoWin> file_version_info =
+      FileVersionInfoWin::CreateFileVersionInfoWin(kAmdDriver64);
   if (!file_version_info) {
-    file_version_info.reset(
-        FileVersionInfoWin::CreateFileVersionInfo(kAmdDriver32));
+    file_version_info =
+        FileVersionInfoWin::CreateFileVersionInfoWin(kAmdDriver32);
     if (!file_version_info)
       return false;
   }
 
   const VS_FIXEDFILEINFO* fixed_file_info =
-      static_cast<FileVersionInfoWin*>(file_version_info.get())
-          ->fixed_file_info();
+      file_version_info->fixed_file_info();
   const int major = HIWORD(fixed_file_info->dwFileVersionMS);
   const int minor = LOWORD(fixed_file_info->dwFileVersionMS);
   const int minor_1 = HIWORD(fixed_file_info->dwFileVersionLS);
@@ -348,17 +347,14 @@ bool BadAMDVulkanDriverVersion() {
 }
 
 bool BadVulkanDllVersion() {
-  std::unique_ptr<FileVersionInfoWin> file_version_info(
-      static_cast<FileVersionInfoWin*>(
-          FileVersionInfoWin::CreateFileVersionInfo(
-              base::FilePath(FILE_PATH_LITERAL("vulkan-1.dll")))));
-
+  std::unique_ptr<FileVersionInfoWin> file_version_info =
+      FileVersionInfoWin::CreateFileVersionInfoWin(
+          base::FilePath(FILE_PATH_LITERAL("vulkan-1.dll")));
   if (!file_version_info)
     return false;
 
   const VS_FIXEDFILEINFO* fixed_file_info =
-      static_cast<FileVersionInfoWin*>(file_version_info.get())
-          ->fixed_file_info();
+      file_version_info->fixed_file_info();
   const int major = HIWORD(fixed_file_info->dwFileVersionMS);
   const int minor = LOWORD(fixed_file_info->dwFileVersionMS);
   const int build_1 = HIWORD(fixed_file_info->dwFileVersionLS);

gpu/config/nvml_info.cc

@@ -43,10 +43,8 @@ bool GetNvmlDeviceInfo(uint32_t pci_device_id,
   }
   dll_path = dll_path.Append(L"NVIDIA Corporation\\NVSMI\\nvml.dll");
 
-  std::unique_ptr<FileVersionInfoWin> file_version_info(
-      static_cast<FileVersionInfoWin*>(
-          FileVersionInfoWin::CreateFileVersionInfo(dll_path)));
-
+  std::unique_ptr<FileVersionInfoWin> file_version_info =
+      FileVersionInfoWin::CreateFileVersionInfoWin(dll_path);
   if (!file_version_info) {
     return false;
   }