Use LoadLibraryExW if applicable in LoadNativeLibrary on Windows

base/native_library_win.cc

 // Copyright (c) 2011 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 "base/native_library.h"
 
 #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"
 #include "base/threading/thread_restrictions.h"
 
 namespace base {
 
-typedef HMODULE (WINAPI* LoadLibraryFunction)(const wchar_t* file_name);
+typedef HMODULE(WINAPI* AddDllDirectory)(PCWSTR NewDirectory);
+typedef HMODULE(WINAPI* LoadLibraryFunction)(const wchar_t* file_name);
+typedef HMODULE(WINAPI* LoadLibraryFunctionEx)(const wchar_t* file_name,
+                                               HANDLE hFile,
+                                               DWORD dwFlags);
 
 namespace {
-
+// This helper method uses LoadLibrary() WinAPI.
 NativeLibrary LoadNativeLibraryHelper(const FilePath& library_path,
                                       LoadLibraryFunction load_library_api,
                                       NativeLibraryLoadError* error) {
   // LoadLibrary() opens the file off disk.
   ThreadRestrictions::AssertIOAllowed();
 
   // Switch the current directory to the library directory as the library
   // may have dependencies on DLLs in this directory.
   bool restore_directory = false;
   FilePath current_directory;
@@ skipping 10 matching lines @@
     // GetLastError() needs to be called immediately after |load_library_api|.
     error->code = GetLastError();
   }
 
   if (restore_directory)
     SetCurrentDirectory(current_directory);
 
   return module;
 }
 
+// This helper method uses LoadLibraryEx() WinAPI.
+NativeLibrary LoadNativeLibraryHelperEx(const FilePath& library_path,
+                                        LoadLibraryFunctionEx load_library_api,
+                                        NativeLibraryLoadError* error) {
+  // LoadLibraryEx() opens the file off disk.
+  ThreadRestrictions::AssertIOAllowed();
+
+  // 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.
+  HMODULE module = (*load_library_api)(
+      library_path.value().c_str(), nullptr,
+      LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR | LOAD_LIBRARY_SEARCH_DEFAULT_DIRS);
+  if (!module && error) {
+    // GetLastError() needs to be called immediately after |load_library_api|.
+    error->code = GetLastError();
+  }
+
+  return module;
+}
+
 }  // namespace
 
 std::string NativeLibraryLoadError::ToString() const {
   return StringPrintf("%u", code);
 }
 
 // static
 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);
+  HMODULE load_library_module = nullptr;
 
-  LoadLibraryFunction load_library = reinterpret_cast<LoadLibraryFunction>(
-      GetProcAddress(GetModuleHandle(L"kernel32.dll"), "LoadLibraryW"));
+  enum LoadLibraryResult {
+    // LoadLibraryExW API/flags are available and the call succeeds.
+    LoadLibExW_Succeed = 0,
+    // LoadLibraryExW API/flags are availabe to use but the call fails, then
+    // LoadLibraryW is used and succeeds.
+    LoadLibExW_Fail_LoadLibW_Succeed,
+    // LoadLibraryExW API/flags are availabe to use but the call fails, then
+    // LoadLibraryW is used but fails as well.
+    LoadLibExW_Fail_LoadLibW_Fail,
+    // LoadLibraryExW API/flags are unavailabe to use, then LoadLibraryW is used
+    // and succeeds.
+    LoadLibExW_Unavailable_LoadLibW_Succeed,
+    // LoadLibraryExW API/flags are unavailabe to use, then LoadLibraryW is used
+    // but fails.
+    LoadLibExW_Unavailable_LoadLibW_Fail,
+    // Add new items before this one, always keep this one at the end.
+    END

[xhwang, 2017/03/14 00:14:32]

nit: We use UPPER_CASE style for enums
https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md...

Also, the names seem a bit verbose to me. Assuming you'll explain what these are
in histograms.xml, how about something like:

SUCCEED,
FAIL_AND_SUCCEED,
FAIL_AND_FAIL,
UNAVAILABLE_AND_SUCCEED,
UNAVAILABLE_AND_FAIL,

[chengx, 2017/03/14 02:44:40]

Done.

+  };
 
-  return LoadNativeLibraryHelper(library_path, load_library, NULL);
+  // This variable records the library loading result.
+  LoadLibraryResult load_library_status = LoadLibraryResult::LoadLibExW_Succeed;
+
+  // 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 LoadNativeLibraryHelperEx
+  // method above.
+  auto add_dll_dir_func = reinterpret_cast<AddDllDirectory>(
+      GetProcAddress(GetModuleHandle(L"kernel32.dll"), "AddDllDirectory"));
+
+  if (add_dll_dir_func) {
+    LoadLibraryFunctionEx load_library =
+        reinterpret_cast<LoadLibraryFunctionEx>(
+            GetProcAddress(GetModuleHandle(L"kernel32.dll"), "LoadLibraryExW"));
+    load_library_module =
+        LoadNativeLibraryHelperEx(library_path, load_library, nullptr);

[xhwang, 2017/03/14 00:14:32]

I guess we are mixing DLL load failure with LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR not
supported failure. But we really are only interested in
LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR failure, not the former...

If LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR is not supported, what kind of error code
will we get?

[chengx, 2017/03/14 02:44:40]

I think you are right. We have LoadLibraryW and LoadLibraryExW defined in the
Winbase.h already, so there is no need to GetProcAddress for them. So there will
be no DLL load failures now.

+  }

[xhwang, 2017/03/14 00:14:32]

nit: We (media) like to return early...

So here you can do something like:

if (load_library_module) {
  ReportResultToUMA(...);
  return load_library_module;
}

So the code below doesn't need to be a big if block...

Feel free to ignore if base/ or your team has different style.

[chengx, 2017/03/14 02:44:40]

Return early SGTM.

+
+  // If LoadLibraryExW API call fails, try LoadLibraryW API.
+  if (!load_library_module) {
+    LoadLibraryFunction load_library = reinterpret_cast<LoadLibraryFunction>(
+        GetProcAddress(GetModuleHandle(L"kernel32.dll"), "LoadLibraryW"));
+    load_library_module =
+        LoadNativeLibraryHelper(library_path, load_library, nullptr);
+    // Record the library loading result.
+    if (add_dll_dir_func) {
+      if (load_library_module)
+        load_library_status =
+            LoadLibraryResult::LoadLibExW_Fail_LoadLibW_Succeed;
+      else
+        load_library_status = LoadLibraryResult::LoadLibExW_Fail_LoadLibW_Fail;
+    } else if (load_library_module)
+      load_library_status =
+          LoadLibraryResult::LoadLibExW_Unavailable_LoadLibW_Succeed;
+    else
+      load_library_status =
+          LoadLibraryResult::LoadLibExW_Unavailable_LoadLibW_Fail;
+  }
+
+  UMA_HISTOGRAM_ENUMERATION("LibraryLoader.NativeLibraryDynamicallyLoad",
+                            load_library_status, LoadLibraryResult::END);
+  return load_library_module;
 }
 
 // static
 void UnloadNativeLibrary(NativeLibrary library) {
   FreeLibrary(library);
 }
 
 // static
 void* GetFunctionPointerFromNativeLibrary(NativeLibrary library,
                                           StringPiece name) {
   return GetProcAddress(library, name.data());
 }
 
 // static
 std::string GetNativeLibraryName(StringPiece name) {
   DCHECK(IsStringASCII(name));
   return name.as_string() + ".dll";
 }
 
 }  // namespace base