Bug 180642 - [WinCairo] DLLLauncherMain should use SetDllDirectory
Summary: [WinCairo] DLLLauncherMain should use SetDllDirectory
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Fujii Hironori
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-12-11 02:33 PST by Fujii Hironori
Modified: 2017-12-11 18:48 PST (History)
11 users (show)

See Also:


Attachments
Patch (1.64 KB, patch)
2017-12-11 02:48 PST, Fujii Hironori
no flags Details | Formatted Diff | Diff
Patch (4.02 KB, patch)
2017-12-11 18:22 PST, Fujii Hironori
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Fujii Hironori 2017-12-11 02:33:22 PST
[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
Comment 1 Fujii Hironori 2017-12-11 02:48:47 PST
Created attachment 328960 [details]
Patch
Comment 2 Alex Christensen 2017-12-11 09:14:16 PST
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?
Comment 3 Fujii Hironori 2017-12-11 18:16:17 PST
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.
Comment 4 Fujii Hironori 2017-12-11 18:22:56 PST
Created attachment 329071 [details]
Patch
Comment 5 WebKit Commit Bot 2017-12-11 18:45:16 PST
Comment on attachment 329071 [details]
Patch

Clearing flags on attachment: 329071

Committed r225767: <https://trac.webkit.org/changeset/225767>
Comment 6 WebKit Commit Bot 2017-12-11 18:45:18 PST
All reviewed patches have been landed.  Closing bug.
Comment 7 Radar WebKit Bug Importer 2017-12-11 18:48:32 PST
<rdar://problem/35985025>