Bug 180642

Summary:

[WinCairo] DLLLauncherMain should use SetDllDirectory

Product:

WebKit

Reporter:

Fujii Hironori <fujii.hironori>

Component:

Tools / Tests

Assignee:

Fujii Hironori <fujii.hironori>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

achristensen, bfulgham, commit-queue, ews-watchlist, keith_miller, lforschler, mark.lam, msaboff, pvollan, saam, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none

Fujii Hironori

Reported 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

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
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Fujii Hironori

Comment 1 2017-12-11 02:48:47 PST

Created attachment 328960 [details] Patch

Alex Christensen

Comment 2 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?

Fujii Hironori

Comment 3 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.

Fujii Hironori

Comment 4 2017-12-11 18:22:56 PST

Created attachment 329071 [details] Patch

WebKit Commit Bot

Comment 5 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>

WebKit Commit Bot

Comment 6 2017-12-11 18:45:18 PST

All reviewed patches have been landed. Closing bug.

Radar WebKit Bug Importer

Comment 7 2017-12-11 18:48:32 PST

<rdar://problem/35985025>

Note You need to log in before you can comment on or make changes to this bug.