Bug 180642 – [WinCairo] DLLLauncherMain should use SetDllDirectory

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Fujii Hironori]

[Win] DLLLauncherMain should use SetDllDirectory

Windows have icuuc.dll in the system directory. WebKit should
find one in WebKitLibraries directory instead of one in the
system directory.

Dynamic-Link Library Search Order (Windows)
https://msdn.microsoft.com/library/windows/desktop/ms682586%28v=vs.85%29.aspx

SetDllDirectory function (Windows)
https://msdn.microsoft.com/library/windows/desktop/ms686203(v=vs.85).aspx

[Fujii Hironori]

Created attachment 328960 [details]
Patch

[Alex Christensen]

Comment on attachment 328960 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=328960&action=review

> Tools/ChangeLog:12
> +        * win/DLLLauncher/DLLLauncherMain.cpp:

We should keep this the same as Source/JavaScriptCore/shell/DLLLauncherMain.cpp

> Tools/win/DLLLauncher/DLLLauncherMain.cpp:128
>      wstring pathGStreamer = copyEnvironmentVariable(L"GSTREAMER_1_0_ROOT_X86") + L"bin";

We should probably remove this.  Nobody uses gstreamer with wincairo any more.

> Tools/win/DLLLauncher/DLLLauncherMain.cpp:132
>      wstring pathWinCairo = copyEnvironmentVariable(L"WEBKIT_LIBRARIES") + L"\\bin32";
>  #endif
> -    prependPath(pathWinCairo);
> +    if (!SetDllDirectory(pathWinCairo.c_str()))
> +        fatalError(programName, L"Failed to SetDllDirectory");

What does this do if there is no WEBKIT_LIBRARIES environment variable?

[Fujii Hironori]

Comment on attachment 328960 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=328960&action=review

Thank you for the review.

>> Tools/ChangeLog:12
>> +        * win/DLLLauncher/DLLLauncherMain.cpp:
> 
> We should keep this the same as Source/JavaScriptCore/shell/DLLLauncherMain.cpp

Good point. I'll fix it.

>> Tools/win/DLLLauncher/DLLLauncherMain.cpp:128
>>      wstring pathGStreamer = copyEnvironmentVariable(L"GSTREAMER_1_0_ROOT_X86") + L"bin";
> 
> We should probably remove this.  Nobody uses gstreamer with wincairo any more.

Agreed.

>> Tools/win/DLLLauncher/DLLLauncherMain.cpp:132
>> +        fatalError(programName, L"Failed to SetDllDirectory");
> 
> What does this do if there is no WEBKIT_LIBRARIES environment variable?

Good point. It should work without WEBKIT_LIBRARIES. I'll fix it.

[Fujii Hironori]

Created attachment 329071 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 329071 [details]
Patch

Clearing flags on attachment: 329071

Committed r225767: <https://trac.webkit.org/changeset/225767>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/35985025>