Workaround for Windows-only crash inside delay load helper.

media/base/media_win.cc

Index: media/base/media_win.cc
===================================================================
--- media/base/media_win.cc	(revision 122285)
+++ media/base/media_win.cc	(working copy)
@@ -5,13 +5,18 @@
 #include "media/base/media.h"
 
 #include <windows.h>
+#include <delayimp.h>
 
+#include <string>
+
 #include "base/file_path.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/native_library.h"
 #include "base/path_service.h"
 
+#pragma comment(lib, "delayimp.lib")
+
 namespace media {
 
 enum FFmpegDLLKeys {
@@ -21,23 +26,47 @@
 };
 
 // Retrieves the DLLName for the given key.
-static FilePath::CharType* GetDLLName(FFmpegDLLKeys dll_key) {
+static const char* GetDLLName(FFmpegDLLKeys dll_key) {
   // TODO(ajwong): Do we want to lock to a specific ffmpeg version?
   switch (dll_key) {
     case FILE_LIBAVCODEC:
-      return FILE_PATH_LITERAL("avcodec-53.dll");
+      return "avcodec-53.dll";

[scherkus (not reviewing), 2012/02/23 05:22:55]

dalecurtis: do these need updated?

[DaleCurtis, 2012/02/23 21:03:08]

Not since the FFmpeg roll got reverted. Just make sure to rebase before pushing,
CQ/dcommit will stop you in any case.

     case FILE_LIBAVFORMAT:
-      return FILE_PATH_LITERAL("avformat-53.dll");
+      return "avformat-53.dll";
     case FILE_LIBAVUTIL:
-      return FILE_PATH_LITERAL("avutil-51.dll");
+      return "avutil-51.dll";
     default:
       LOG(DFATAL) << "Invalid DLL key requested: " << dll_key;
-      return FILE_PATH_LITERAL("");
+      return "";
   }
 }
 
 static bool g_media_library_is_initialized = false;
 
+// Handler for SEH when delay loading -- we want to catch everything
+// delayload-related, but crash on access violation and such.
+static int delay_load_exception_filter(unsigned int exception_code) {
+  switch (exception_code) {
+    case VcppException(ERROR_SEVERITY_ERROR, ERROR_INVALID_PARAMETER):
+    case VcppException(ERROR_SEVERITY_ERROR, ERROR_MOD_NOT_FOUND):
+    case VcppException(ERROR_SEVERITY_ERROR, ERROR_PROC_NOT_FOUND):
+      return EXCEPTION_EXECUTE_HANDLER;

[cpu_(ooo_6.6-7.5), 2012/02/23 23:58:25]

I don't understand the purpose of the patch. You swallow 3 exceptions we don't
see and let EXCEPTION_ACCESS_VIOLATION_EXEC  go thru so we still crash?

Or is the associated bug incorrect?

+    default:
+      return EXCEPTION_CONTINUE_SEARCH;
+  }
+}
+
+// Fixes import table for DLL. Returns true if succeed, false otherwise.

[scherkus (not reviewing), 2012/02/23 05:22:55]

so this function doesn't require absolute paths?

+static bool PatchImportTable(const char *dll) {

[scherkus (not reviewing), 2012/02/23 05:22:55]

pointer w/ type: const char* dll

+  __try {
+    if (FAILED(::__HrLoadAllImportsForDll(dll)))
+      return false;
+  } __except(delay_load_exception_filter(GetExceptionCode())) {
+    return false;
+  }
+  return true;
+}
+
 // Attempts to initialize the media library (loading DLLs, DSOs, etc.).
 // Returns true if everything was successfully initialized, false otherwise.
 bool InitializeMediaLibrary(const FilePath& base_path) {
@@ -51,13 +80,30 @@
   };
   HMODULE libs[arraysize(path_keys)] = {NULL};
 
+  // Convert to absolute path, customer can specify relative path by
+  // command-line flag, and LoadLibraryEx(..., LOAD_WITH_ALTERED_SEARCH_PATH)
+  // does not allow relative paths.
+  DWORD full_path_len = ::GetFullPathName(base_path.value().c_str(),

[scherkus (not reviewing), 2012/02/23 05:22:55]

perhaps turn this into a function like GetFullFilePath(...) as it's a little
confusing having this code inlined here

+                                          0,
+                                          NULL,
+                                          NULL);
+  if (full_path_len == 0)
+    return false;
+  std::wstring full_path(full_path_len, 0);
+  ::GetFullPathName(base_path.value().c_str(),
+                    full_path_len,
+                    &full_path[0],
+                    NULL);
+  full_path.resize(full_path_len - 1);  // Get rid of terminating NUL character.
+
   for (size_t i = 0; i < arraysize(path_keys); ++i) {
-    FilePath path = base_path.Append(GetDLLName(path_keys[i]));
+    FilePath path(full_path);
+    path = path.AppendASCII(GetDLLName(path_keys[i]));
 
     // Use alternate DLL search path so we don't load dependencies from the
     // system path.  Refer to http://crbug.com/35857
     const wchar_t* cpath = path.value().c_str();
-    libs[i] = LoadLibraryEx(cpath, NULL, LOAD_WITH_ALTERED_SEARCH_PATH);
+    libs[i] = ::LoadLibraryEx(cpath, NULL, LOAD_WITH_ALTERED_SEARCH_PATH);
     if (!libs[i])
       break;
   }
@@ -65,17 +111,38 @@
   // Check that we loaded all libraries successfully.  We only need to check the
   // last array element because the loop above will break without initializing
   // it on any prior error.
-  g_media_library_is_initialized = (libs[arraysize(libs) - 1] != NULL);
+  bool media_library_is_initialized = (libs[arraysize(libs) - 1] != NULL);
 
-  if (!g_media_library_is_initialized) {
+  if (!media_library_is_initialized) {
     // Free any loaded libraries if we weren't successful.
     for (size_t i = 0; i < arraysize(libs) && libs[i] != NULL; ++i) {
       FreeLibrary(libs[i]);
       libs[i] = NULL;  // Just to be safe.
     }
+    return false;
   }
 
-  return g_media_library_is_initialized;
+  // Workaround for http://crbug.com/110983
+  // LoadLibrary() sometimes AV's when called by delay load helper when we

[scherkus (not reviewing), 2012/02/23 05:22:55]

what's AV?

+  // call function in ffmpeg for the first time, and we don't know why.
+  // Force delay load helper to fix import table here instead.
+  // Theoretically, there is no need to call LoadLibrary() before
+  // __HrLoadAllImportsForDll(), it will call LoadLibrary() itself, but there
+  // is no way to specify LOAD_WITH_ALTERED_SEARCH_PATH when calling
+  // __HrLoadAllImportsForDll(). So we do everything in 2 steps -- first call
+  // LoadLibraryEx(..., LOAD_WITH_ALTERED_SEARCH_PATH), then call
+  // __HrLoadAllImportsForDll(). Overhead is negligible compared to disk
+  // access time.
+  // Note: in case of error we are not unloading DLL because unload requires
+  //       extra resources and should not be necessary; if we ever decide to
+  //       unload by calling __FUnloadDelayLoadedDLL() please add /DELAY:UNLOAD
+  //       to the linker command line.
+  for (size_t i = 0; i < arraysize(path_keys); ++i) {
+    media_library_is_initialized &= PatchImportTable(GetDLLName(path_keys[i]));
+  }
+
+  g_media_library_is_initialized = media_library_is_initialized;
+  return media_library_is_initialized;
 }
 
 void InitializeMediaLibraryForTesting() {