Move ChromeDllPrefetchResult histogram to chrome.dll

This change moves the code to emit histogram
"Windows.ChromeDllPrefetchResult" histogram into ChromeMain(). This
fixes an issue where the histogram was created in chrome.exe and
thus failed to be emitted by chrome.dll on non-components builds.

Currently, the histogram is created before chrome.dll is entered, in
chrome.exe. On non-component (release) builds, chrome.exe and
chrome.dll each have separate copies of base, because base is
statically linked ('baked in') into each portable executable file.
Creating the histogram in chrome.exe's base means that chrome.dll's
base, the one that actually emits the histograms to UMA, does not see
the histogram.

This change addresses this by passing the histogram value into
ChromeMain() and emitting the histogram there.

Bug: 1096609
Change-Id: I918144dff4fc0d5189d5882462ffadd88f27b9b2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2280242
Commit-Queue: Jesse McKenna <jessemckenna@google.com>
Reviewed-by: Greg Thompson <grt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#786465}

chrome/app/chrome_main.cc

@@ -28,6 +28,8 @@
 
 #if defined(OS_WIN)
 #include "base/debug/dump_without_crashing.h"
+#include "base/files/file_util.h"
+#include "base/metrics/histogram_functions.h"
 #include "base/win/win_util.h"
 #include "chrome/chrome_elf/chrome_elf_main.h"
 #include "chrome/common/chrome_constants.h"
@@ -40,7 +42,8 @@
 extern "C" {
 DLLEXPORT int __cdecl ChromeMain(HINSTANCE instance,
                                  sandbox::SandboxInterfaceInfo* sandbox_info,
-                                 int64_t exe_entry_point_ticks);
+                                 int64_t exe_entry_point_ticks,
+                                 base::PrefetchResultCode prefetch_result_code);
 }
 #elif defined(OS_POSIX)
 extern "C" {
@@ -50,15 +53,19 @@ int ChromeMain(int argc, const char** argv);
 #endif
 
 #if defined(OS_WIN)
-DLLEXPORT int __cdecl ChromeMain(HINSTANCE instance,
-                                 sandbox::SandboxInterfaceInfo* sandbox_info,
-                                 int64_t exe_entry_point_ticks) {
+DLLEXPORT int __cdecl ChromeMain(
+    HINSTANCE instance,
+    sandbox::SandboxInterfaceInfo* sandbox_info,
+    int64_t exe_entry_point_ticks,
+    base::PrefetchResultCode prefetch_result_code) {
 #elif defined(OS_POSIX)
 int ChromeMain(int argc, const char** argv) {
   int64_t exe_entry_point_ticks = 0;
 #endif
 
 #if defined(OS_WIN)
+  base::UmaHistogramEnumeration("Windows.ChromeDllPrefetchResult",
+                                prefetch_result_code);
   install_static::InitializeFromPrimaryModule();
 #endif
 

chrome/app/main_dll_loader_win.cc

@@ -22,7 +22,6 @@
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/macros.h"
-#include "base/metrics/histogram_functions.h"
 #include "base/path_service.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_piece.h"
@@ -47,22 +46,13 @@
 
 namespace {
 // The entry point signature of chrome.dll.
-typedef int (*DLL_MAIN)(HINSTANCE, sandbox::SandboxInterfaceInfo*, int64_t);
+typedef int (*DLL_MAIN)(HINSTANCE,
+                        sandbox::SandboxInterfaceInfo*,
+                        int64_t,
+                        base::PrefetchResultCode);
 
 typedef void (*RelaunchChromeBrowserWithNewCommandLineIfNeededFunc)();
 
-// Loads |module| after setting the CWD to |module|'s directory. Returns a
-// reference to the loaded module on success, or null on error.
-HMODULE LoadModuleWithDirectory(const base::FilePath& module) {
-  ::SetCurrentDirectoryW(module.DirName().value().c_str());
-  const base::PrefetchResult prefetch_result =
-      base::PreReadFile(module, /*is_executable=*/true);
-  base::UmaHistogramEnumeration("Windows.ChromeDllPrefetchResult",
-                                prefetch_result.code_);
-  return ::LoadLibraryExW(module.value().c_str(), nullptr,
-                          LOAD_WITH_ALTERED_SEARCH_PATH);
-}
-
 void RecordDidRun(const base::FilePath& dll_path) {
   installer::UpdateDidRunState(true);
 }
@@ -107,20 +97,28 @@ MainDllLoader::MainDllLoader()
 MainDllLoader::~MainDllLoader() {
 }
 
-HMODULE MainDllLoader::Load(base::FilePath* module) {
+// static
+MainDllLoader::LoadResult MainDllLoader::Load(base::FilePath* module) {
   *module = GetModulePath(installer::kChromeDll);
   if (module->empty()) {
     PLOG(ERROR) << "Cannot find module " << installer::kChromeDll;
-    return nullptr;
+    return {nullptr, base::PrefetchResultCode::kInvalidFile};
   }
-  HMODULE dll = LoadModuleWithDirectory(*module);
-  if (!dll) {
+  LoadResult load_result = LoadModuleWithDirectory(*module);
+  if (!load_result.handle)
     PLOG(ERROR) << "Failed to load Chrome DLL from " << module->value();
-    return nullptr;
-  }
+  return load_result;
+}
 
-  DCHECK(dll);
-  return dll;
+// static
+MainDllLoader::LoadResult MainDllLoader::LoadModuleWithDirectory(
+    const base::FilePath& module) {
+  ::SetCurrentDirectoryW(module.DirName().value().c_str());
+  base::PrefetchResultCode prefetch_result_code =
+      base::PreReadFile(module, /*is_executable=*/true).code_;
+  HMODULE handle = ::LoadLibraryExW(module.value().c_str(), nullptr,
+                                    LOAD_WITH_ALTERED_SEARCH_PATH);
+  return {handle, prefetch_result_code};
 }
 
 const int kNonBrowserShutdownPriority = 0x280;
@@ -148,7 +146,8 @@ int MainDllLoader::Launch(HINSTANCE instance,
   }
 
   base::FilePath file;
-  dll_ = Load(&file);
+  LoadResult load_result = Load(&file);
+  dll_ = load_result.handle;
   if (!dll_)
     return chrome::RESULT_CODE_MISSING_DATA;
 
@@ -166,7 +165,8 @@ int MainDllLoader::Launch(HINSTANCE instance,
   DLL_MAIN chrome_main =
       reinterpret_cast<DLL_MAIN>(::GetProcAddress(dll_, "ChromeMain"));
   int rc = chrome_main(instance, &sandbox_info,
-                       exe_entry_point_ticks.ToInternalValue());
+                       exe_entry_point_ticks.ToInternalValue(),
+                       load_result.prefetch_result_code);
   return rc;
 }
 

chrome/app/main_dll_loader_win.h

@@ -17,7 +17,8 @@
 namespace base {
 class CommandLine;
 class FilePath;
-}
+enum class PrefetchResultCode;
+}  // namespace base
 
 // Implements the common aspects of loading the main dll for both chrome and
 // chromium scenarios, which are in charge of implementing one abstract
@@ -49,12 +50,22 @@ class MainDllLoader {
                               const base::FilePath& dll_path) = 0;
 
  private:
-  // Loads the appropriate DLL for the process type |process_type_|. Populates
-  // |module| with the path of the loaded DLL. Returns a reference to the
-  // module, or null on failure.
-  HMODULE Load(base::FilePath* module);
+  struct LoadResult {
+    HMODULE handle;
+    base::PrefetchResultCode prefetch_result_code;
+  };
+
+  // Prefetches and loads the appropriate DLL for the process type
+  // |process_type_|. Populates |module| with the path of the loaded DLL, and
+  // returns a struct containing a handle to the loaded DLL (or nullptr on
+  // failure), and a prefetch result code.
+  static LoadResult Load(base::FilePath* module);
+
+  // Prefetches and loads |module| after setting the CWD to |module|'s
+  // directory. Returns a struct containing a handle to the loaded module on
+  // success (or nullptr on error) and a prefetch result code.
+  static LoadResult LoadModuleWithDirectory(const base::FilePath& module);
 
- private:
   HMODULE dll_;
   std::string process_type_;
 };