Use PathService in installer::GetModulePath().

chrome/installer/util/module_util_win.cc

Index: chrome/installer/util/module_util_win.cc
diff --git a/chrome/installer/util/module_util_win.cc b/chrome/installer/util/module_util_win.cc
index c589263690a9c127ca795d8b75e3b449a2fdca6f..8832b9c183c5935dfc016ab215cc7bb661297020 100644
--- a/chrome/installer/util/module_util_win.cc
+++ b/chrome/installer/util/module_util_win.cc
@@ -2,14 +2,14 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include <windows.h>
-
 #include "chrome/installer/util/module_util_win.h"
 
+#include "base/base_paths.h"
 #include "base/file_version_info.h"
 #include "base/files/file.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/path_service.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/version.h"
 
@@ -17,24 +17,15 @@ namespace installer {
 
 namespace {
 
-// Returns the directory in which the currently running executable resides.
-base::FilePath GetExecutableDir() {
-  base::char16 path[MAX_PATH];
-  ::GetModuleFileNameW(nullptr, path, MAX_PATH);
-  return base::FilePath(path).DirName();
-}
-
-// Returns the version in the current module's version resource or the empty
+// Returns the version in the current executable's version resource or the empty
 // string if none found.

[gab, 2015/11/18 23:01:40]

Is this comment still correct?

[fdoray, 2015/11/18 23:26:36]

Done. Removed.

-base::string16 GetCurrentModuleVersion() {
+base::string16 GetCurrentExecutableVersion() {
   scoped_ptr<FileVersionInfo> file_version_info(
       CREATE_FILE_VERSION_INFO_FOR_CURRENT_MODULE());
-  if (file_version_info.get()) {
-    base::string16 version_string(file_version_info->file_version());
-    if (Version(base::UTF16ToASCII(version_string)).IsValid())
-      return version_string;
-  }
-  return base::string16();
+  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
@@ -50,18 +41,25 @@ base::FilePath GetModulePath(base::StringPiece16 module_name,
                              base::string16* version) {
   DCHECK(version);
 
-  base::FilePath module_dir = GetExecutableDir();
-  base::FilePath module = module_dir.Append(module_name);
-  if (ModuleCanBeRead(module))
-    return module;
+  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.
+  base::FilePath module_path = exe_dir.Append(module_name);

[gab, 2015/11/18 23:01:40]

const

[fdoray, 2015/11/18 23:26:36]

Done.

+  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.
+  base::string16 version_string(GetCurrentExecutableVersion());
+  DCHECK(!version_string.empty());

[grt (UTC plus 2), 2015/11/17 23:22:58]

This will be the failure mode if we ever botch the build and leave out the
version resource, which happens from time to time. What exactly will happen if
the string is empty? It would be nice if this were easy to diagnose.

[fdoray, 2015/11/18 15:37:03]

Before this CL, when the version string was empty or invalid, the startup failed
in MainDllLoader::Launch, line 219 [1]. 

Now, for a debug build, it fails when a DCHECK is hit at line 25 or 27 of this
file. For a release build, it can fail when |file_version_info| is dereferenced
at line 26 of this file, or in MainDllLoader::Launch like before [1].

Should I remove the DCHECK at line 25 and replace it with if
(file_version_info.get()) {}, to avoid crashes when the version string is
missing (even though that should never happen for a correct build)?  

IMO, problems are easier to diagnose in debug builds with this CL. For release
builds, nothing changed. What do you think?

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/app/main_dl...

 
-  base::string16 version_string(GetCurrentModuleVersion());
-  if (version_string.empty()) {
-    LOG(ERROR) << "No valid Chrome version found";
-    return base::FilePath();
-  }
   *version = version_string;
-  return module_dir.Append(version_string).Append(module_name);
+  return exe_dir.Append(version_string).Append(module_name);
 }
 
 }  // namespace installer