Use LoadLibraryExW if applicable in LoadNativeLibrary on Windows

base/native_library_win.cc

Index: base/native_library_win.cc
diff --git a/base/native_library_win.cc b/base/native_library_win.cc
index 64c7380f173247c28b78e1f339b963d7374cf83f..2e36fc1bc43e3645553e574e645fecf901ac5ada 100644
--- a/base/native_library_win.cc
+++ b/base/native_library_win.cc
@@ -7,6 +7,7 @@
 #include <windows.h>
 
 #include "base/files/file_util.h"
+#include "base/metrics/histogram_macros.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
@@ -14,16 +15,102 @@
 
 namespace base {
 
-typedef HMODULE (WINAPI* LoadLibraryFunction)(const wchar_t* file_name);
+typedef HMODULE(WINAPI* AddDllDirectory)(PCWSTR NewDirectory);

[dcheng, 2017/03/15 09:33:49]

Nit: since we're changing this anyway, please switch it to using AddDllDirectory
= HMODULE(*)(PCWSTR new_directory);

[chengx, 2017/03/15 18:59:25]

Done.

 
 namespace {
+// This enum is used to back an UMA histogram, and should therefore be treated
+// as append-only.
+enum LoadLibraryResult {
+  // LoadLibraryExW API/flags are available and the call succeeds.
+  SUCCEED = 0,
+  // LoadLibraryExW API/flags are availabe to use but the call fails, then
+  // LoadLibraryW is used and succeeds.
+  FAIL_AND_SUCCEED,
+  // LoadLibraryExW API/flags are availabe to use but the call fails, then
+  // LoadLibraryW is used but fails as well.
+  FAIL_AND_FAIL,
+  // LoadLibraryExW API/flags are unavailabe to use, then LoadLibraryW is used
+  // and succeeds.
+  UNAVAILABLE_AND_SUCCEED,
+  // LoadLibraryExW API/flags are unavailabe to use, then LoadLibraryW is used
+  // but fails.
+  UNAVAILABLE_AND_FAIL,
+  // Add new items before this one, always keep this one at the end.
+  END
+};
+
+// A helper method to log library loading result to UMA.
+void LogLibrarayLoadResultToUMA(LoadLibraryResult result) {
+  UMA_HISTOGRAM_ENUMERATION("LibraryLoader.LoadNativeLibraryWindows", result,
+                            LoadLibraryResult::END);
+}
+
+// A helper method to check if AddDllDirectory method is available, thus
+// LOAD_LIBRARY_SEARCH_* flags are available on systems.
+bool AreSearchFlagsAvailable() {
+  // The LOAD_LIBRARY_SEARCH_* flags are available on systems that have
+  // KB2533623 installed. To determine whether the flags are available, use
+  // GetProcAddress to get the address of the AddDllDirectory,
+  // RemoveDllDirectory, or SetDefaultDllDirectories function. If GetProcAddress
+  // succeeds, the LOAD_LIBRARY_SEARCH_* flags can be used with LoadLibraryEx.
+  // https://msdn.microsoft.com/en-us/library/windows/desktop/ms684179(v=vs.85).aspx
+  // The LOAD_LIBRARY_SEARCH_* flags are used in the LoadNativeLibraryHelper
+  // method.
+  auto add_dll_dir_func = reinterpret_cast<AddDllDirectory>(
+      GetProcAddress(GetModuleHandle(L"kernel32.dll"), "AddDllDirectory"));
+  return !!add_dll_dir_func;
+}
+
+// A helper method to encode the library loading result to enum
+// LoadLibraryResult.
+LoadLibraryResult GetLoadLibraryResult(bool are_search_flags_available,
+                                       bool is_LoadLibraryW_succeeded) {

[xhwang, 2017/03/14 23:17:41]

nit: we don't use upper cases in variable names...

[chengx, 2017/03/15 18:59:25]

Done.

+  LoadLibraryResult result;
+  if (are_search_flags_available) {
+    if (is_LoadLibraryW_succeeded)
+      result = LoadLibraryResult::FAIL_AND_SUCCEED;
+    else
+      result = LoadLibraryResult::FAIL_AND_FAIL;
+  } else if (is_LoadLibraryW_succeeded) {
+    result = LoadLibraryResult::UNAVAILABLE_AND_SUCCEED;
+  } else {
+    result = LoadLibraryResult::UNAVAILABLE_AND_FAIL;
+  }
+  return result;
+}
 
 NativeLibrary LoadNativeLibraryHelper(const FilePath& library_path,
-                                      LoadLibraryFunction load_library_api,
                                       NativeLibraryLoadError* error) {
   // LoadLibrary() opens the file off disk.
   ThreadRestrictions::AssertIOAllowed();
 
+  HMODULE module = nullptr;
+
+  // This variable records the library loading result.
+  LoadLibraryResult load_library_result = LoadLibraryResult::SUCCEED;
+
+  bool are_search_flags_available = AreSearchFlagsAvailable();
+  if (are_search_flags_available) {
+    // LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR flag is needed to search the library
+    // directory as the library may have dependencies on DLLs in this
+    // directory.
+    module = ::LoadLibraryExW(
+        library_path.value().c_str(), nullptr,
+        LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR | LOAD_LIBRARY_SEARCH_DEFAULT_DIRS);
+    // If LoadLibraryExW succeeds, log this metric and return.
+    if (module) {
+      LogLibrarayLoadResultToUMA(load_library_result);
+      return module;
+    }
+    // GetLastError() needs to be called immediately after
+    // LoadLibraryExW call.
+    if (error)
+      error->code = GetLastError();
+  }
+
+  // If LoadLibraryExW API/flags are unavailable or API call fails, try
+  // LoadLibraryW API.
+
   // Switch the current directory to the library directory as the library
   // may have dependencies on DLLs in this directory.
   bool restore_directory = false;
@@ -36,18 +123,21 @@ NativeLibrary LoadNativeLibraryHelper(const FilePath& library_path,
     }
   }
 
-  HMODULE module = (*load_library_api)(library_path.value().c_str());
-  if (!module && error) {
-    // GetLastError() needs to be called immediately after |load_library_api|.
+  module = ::LoadLibraryW(library_path.value().c_str());
+
+  // GetLastError() needs to be called immediately after LoadLibraryW call.
+  if (!module && error)
     error->code = GetLastError();
-  }
 
   if (restore_directory)
     SetCurrentDirectory(current_directory);
 
+  // Get the library loading result and log it to UMA.
+  LogLibrarayLoadResultToUMA(
+      GetLoadLibraryResult(are_search_flags_available, !!module));
+
   return module;
 }
-
 }  // namespace
 
 std::string NativeLibraryLoadError::ToString() const {
@@ -58,16 +148,7 @@ std::string NativeLibraryLoadError::ToString() const {
 NativeLibrary LoadNativeLibraryWithOptions(const FilePath& library_path,
                                            const NativeLibraryOptions& options,
                                            NativeLibraryLoadError* error) {
-  return LoadNativeLibraryHelper(library_path, LoadLibraryW, error);
-}
-
-NativeLibrary LoadNativeLibraryDynamically(const FilePath& library_path) {
-  typedef HMODULE (WINAPI* LoadLibraryFunction)(const wchar_t* file_name);
-
-  LoadLibraryFunction load_library = reinterpret_cast<LoadLibraryFunction>(
-      GetProcAddress(GetModuleHandle(L"kernel32.dll"), "LoadLibraryW"));
-
-  return LoadNativeLibraryHelper(library_path, load_library, NULL);
+  return LoadNativeLibraryHelper(library_path, error);
 }
 
 // static