Use chrome::kChromeVersion in installer::GetModulePath().

Before this CL, installer::GetModulePath() used the version in the
VS_VERSION_INFO resource of the current module to construct the path
to another module. Unfortunately, reading that resource sometimes
crashed. With this CL, installer::GetModulePath() uses the
chrome::kChromeVersion constant instead. This contains the same
value as the VS_VERSION_INFO resource.

BUG=682987

Review-Url: https://codereview.chromium.org/2656443002
Cr-Commit-Position: refs/heads/master@{#448055}

chrome/app/main_dll_loader_win.cc

@@ -16,12 +16,14 @@
 #include "base/base_switches.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
+#include "base/files/file.h"
 #include "base/files/file_path.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/path_service.h"
 #include "base/strings/string16.h"
+#include "base/strings/string_piece.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/trace_event/trace_event.h"
@@ -40,7 +42,6 @@
 #include "chrome/installer/util/google_update_constants.h"
 #include "chrome/installer/util/google_update_settings.h"
 #include "chrome/installer/util/install_util.h"
-#include "chrome/installer/util/module_util_win.h"
 #include "chrome/installer/util/util_constants.h"
 #include "content/public/app/sandbox_helper_win.h"
 #include "content/public/common/content_switches.h"
@@ -73,6 +74,35 @@ bool ProcessTypeUsesMainDll(const std::string& process_type) {
   return process_type.empty() || process_type == switches::kServiceProcess;
 }
 
+// Indicates whether a file can be opened using the same flags that
+// ::LoadLibrary() uses to open modules.
+bool ModuleCanBeRead(const base::FilePath& file_path) {
+  return base::File(file_path, base::File::FLAG_OPEN | base::File::FLAG_READ)
+      .IsValid();
+}
+
+// Returns the full path to |module_name|. Both dev builds (where |module_name|
+// is in the current executable's directory) and proper installs (where
+// |module_name| is in a versioned sub-directory of the current executable's
+// directory) are suported. The identified file is not guaranteed to exist.
+base::FilePath GetModulePath(base::StringPiece16 module_name) {
+  base::FilePath exe_dir;
+  const bool has_path = base::PathService::Get(base::DIR_EXE, &exe_dir);
+  DCHECK(has_path);
+
+  // Look for the module in the current executable's directory and return the
+  // path if it can be read. This is the expected location of modules for dev
+  // builds.
+  const base::FilePath module_path = exe_dir.Append(module_name);
+  if (ModuleCanBeRead(module_path))
+    return module_path;
+
+  // Othwerwise, return the path to the module in a versioned sub-directory of
+  // the current executable's directory. This is the expected location of
+  // modules for proper installs.
+  return exe_dir.AppendASCII(chrome::kChromeVersion).Append(module_name);
+}
+
 }  // namespace
 
 //=============================================================================
@@ -98,7 +128,7 @@ HMODULE MainDllLoader::Load(base::FilePath* module) {
 #endif
   }
 
-  *module = installer::GetModulePath(dll_name);
+  *module = GetModulePath(dll_name);
   if (module->empty()) {
     PLOG(ERROR) << "Cannot find module " << dll_name;
     return nullptr;

chrome/common/chrome_constants_win_unittest.cc

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/common/chrome_constants.h"
+
+#include <memory>
+
+#include "base/file_version_info.h"
+#include "base/files/file_path.h"
+#include "base/path_service.h"
+#include "base/strings/utf_string_conversions.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace chrome {
+
+// Verify that |kChromeVersion| is equal to the version in the VS_VERSION_INFO
+// resource of chrome.exe.
+TEST(ChromeConstants, ChromeVersion) {
+  base::FilePath current_exe_dir;
+  EXPECT_TRUE(base::PathService::Get(base::DIR_EXE, &current_exe_dir));
+  base::FilePath chrome_exe_path =
+      current_exe_dir.Append(chrome::kBrowserProcessExecutableName);
+
+  std::unique_ptr<FileVersionInfo> file_version_info(
+      FileVersionInfo::CreateFileVersionInfo(chrome_exe_path));
+  ASSERT_TRUE(file_version_info);
+  EXPECT_EQ(base::UTF16ToASCII(file_version_info->file_version()),
+            kChromeVersion);
+}
+
+}  // namespace chrome

chrome/installer/util/BUILD.gn

@@ -181,8 +181,6 @@ static_library("with_no_strings") {
       "legacy_firewall_manager_win.h",
       "master_preferences_constants.cc",
       "master_preferences_constants.h",
-      "module_util_win.cc",
-      "module_util_win.h",
       "move_tree_work_item.cc",
       "move_tree_work_item.h",
       "non_updating_app_registration_data.cc",

chrome/installer/util/module_util_win.cc

-// Copyright (c) 2015 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "chrome/installer/util/module_util_win.h"
-
-#include <memory>
-
-#include "base/base_paths.h"
-#include "base/file_version_info.h"
-#include "base/files/file.h"
-#include "base/logging.h"
-#include "base/path_service.h"
-#include "base/strings/string16.h"
-#include "base/strings/utf_string_conversions.h"
-#include "base/version.h"
-#include "base/win/current_module.h"
-
-namespace installer {
-
-namespace {
-
-// Returns the version in the current executable's version resource.
-base::string16 GetCurrentExecutableVersion() {
-  std::unique_ptr<FileVersionInfo> file_version_info(
-      FileVersionInfo::CreateFileVersionInfoForModule(CURRENT_MODULE()));
-  DCHECK(file_version_info.get());
-  base::string16 version_string(file_version_info->file_version());
-  DCHECK(base::Version(base::UTF16ToASCII(version_string)).IsValid());
-  return version_string;
-}
-
-// Indicates whether a file can be opened using the same flags that
-// ::LoadLibrary() uses to open modules.
-bool ModuleCanBeRead(const base::FilePath file_path) {
-  return base::File(file_path, base::File::FLAG_OPEN | base::File::FLAG_READ)
-      .IsValid();
-}
-
-}  // namespace
-
-base::FilePath GetModulePath(base::StringPiece16 module_name) {
-  base::FilePath exe_dir;
-  const bool has_path = base::PathService::Get(base::DIR_EXE, &exe_dir);
-  DCHECK(has_path);
-
-  // Look for the module in the current executable's directory and return the
-  // path if it can be read. This is the expected location of modules for dev
-  // builds.
-  const base::FilePath module_path = exe_dir.Append(module_name);
-  if (ModuleCanBeRead(module_path))
-    return module_path;
-
-  // Othwerwise, return the path to the module in a versioned sub-directory of
-  // the current executable's directory. This is the expected location of
-  // modules for proper installs.
-  const base::string16 version = GetCurrentExecutableVersion();
-  DCHECK(!version.empty());
-  return exe_dir.Append(version).Append(module_name);
-}
-
-}  // namespace installer

chrome/installer/util/module_util_win.h

-// Copyright (c) 2015 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-// This file defines a utility function to get the full path of a module.
-
-#ifndef CHROME_INSTALLER_UTIL_MODULE_UTIL_WIN_H_
-#define CHROME_INSTALLER_UTIL_MODULE_UTIL_WIN_H_
-
-#include "base/files/file_path.h"
-#include "base/strings/string_piece.h"
-
-namespace installer {
-
-// Returns the full path to |module_name|. Both dev builds (where |module_name|
-// is in the current executable's directory) and proper installs (where
-// |module_name| is in a versioned sub-directory of the current executable's
-// directory) are suported. The identified file is not guaranteed to exist.
-base::FilePath GetModulePath(base::StringPiece16 module_name);
-
-}  // namespace installer
-
-#endif  // CHROME_INSTALLER_UTIL_MODULE_UTIL_WIN_H_

chrome/test/BUILD.gn

@@ -3377,6 +3377,7 @@ test("unit_tests") {
     "../browser/webshare/share_target_pref_helper_unittest.cc",
     "../browser/win/chrome_elf_init_unittest.cc",
     "../browser/win/enumerate_modules_model_unittest.cc",
+    "../common/chrome_constants_win_unittest.cc",
     "../common/chrome_content_client_unittest.cc",
     "../common/chrome_paths_unittest.cc",
     "../common/component_flash_hint_file_linux_unittest.cc",
@@ -3454,6 +3455,12 @@ test("unit_tests") {
   if (is_linux || is_win) {
     data += [ "$root_out_dir/chrome_200_percent.pak" ]
   }
+  if (is_win) {
+    data_deps = [
+      "//chrome:chrome_initial",
+    ]
+    data += [ "$root_out_dir/chrome.exe" ]
+  }
 
   defines = []