WebKit2WebProcess should live with WebKit.dll, which means we should pass a full path to CreateProcess, instead of just assuming that WebKit.dll (and WebKitWebProcess.exe) are in the same folder as Safari.exe
Created attachment 68277 [details] [PATCH] Fix
Comment on attachment 68277 [details] [PATCH] Fix View in context: https://bugs.webkit.org/attachment.cgi?id=68277&action=review > WebKit2/UIProcess/Launcher/win/ProcessLauncherWin.cpp:57 > + HMODULE webkitModule = GetModuleHandle(webkitDLLName); We've been using :: to namespace quality Win32 calls lately. > WebKit2/UIProcess/Launcher/win/ProcessLauncherWin.cpp:63 > + if (!GetModuleFileName(webkitModule, pathStr, MAX_PATH) || GetLastError() != ERROR_SUCCESS) You shouldn't need to check GetLastError and the result of the function. Just checking the result for 0 should be enough to check for failure. Same comment about the ::. > WebKit2/UIProcess/Launcher/win/ProcessLauncherWin.cpp:66 > + PathRemoveFileSpec(pathStr); Why not error check this one since you did with the others? Same comment about :: > WebKit2/UIProcess/Launcher/win/ProcessLauncherWin.cpp:67 > + if (!PathAppend(pathStr, webProcessName)) And again,
Comment on attachment 68277 [details] [PATCH] Fix View in context: https://bugs.webkit.org/attachment.cgi?id=68277&action=review > WebKit2/UIProcess/Launcher/win/ProcessLauncherWin.cpp:35 > +const LPCWSTR webkitDLLName = L"WebKit_debug.dll"; Should be webKitDLLName (with a capital "K"). >> WebKit2/UIProcess/Launcher/win/ProcessLauncherWin.cpp:57 >> + HMODULE webkitModule = GetModuleHandle(webkitDLLName); > > We've been using :: to namespace quality Win32 calls lately. We've also been using the W versions of the APIs, so: ::GetModuleHandleW(webKitDLLName); >> WebKit2/UIProcess/Launcher/win/ProcessLauncherWin.cpp:63 >> + if (!GetModuleFileName(webkitModule, pathStr, MAX_PATH) || GetLastError() != ERROR_SUCCESS) > > You shouldn't need to check GetLastError and the result of the function. Just checking the result for 0 should be enough to check for failure. > Same comment about the ::. _countof(pathStr) would be better than repeating MAX_PATH.
(In reply to comment #2) > (From update of attachment 68277 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=68277&action=review > > > WebKit2/UIProcess/Launcher/win/ProcessLauncherWin.cpp:57 > > + HMODULE webkitModule = GetModuleHandle(webkitDLLName); > > We've been using :: to namespace quality Win32 calls lately. Fixed. > > > WebKit2/UIProcess/Launcher/win/ProcessLauncherWin.cpp:63 > > + if (!GetModuleFileName(webkitModule, pathStr, MAX_PATH) || GetLastError() != ERROR_SUCCESS) > > You shouldn't need to check GetLastError and the result of the function. Just checking the result for 0 should be enough to check for failure. Removed the GetLastError call. > Same comment about the ::. Fixed. > > > WebKit2/UIProcess/Launcher/win/ProcessLauncherWin.cpp:66 > > + PathRemoveFileSpec(pathStr); > > Why not error check this one since you did with the others? This didn't seem to have as clearly defined a failure case and success case as the others (the function returns nonzero if something was removed, or zero otherwise), so I omitted the check for now. > Same comment about :: Fixed. > > > WebKit2/UIProcess/Launcher/win/ProcessLauncherWin.cpp:67 > > + if (!PathAppend(pathStr, webProcessName)) > > And again, Fixed.
(In reply to comment #3) > (From update of attachment 68277 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=68277&action=review > > > WebKit2/UIProcess/Launcher/win/ProcessLauncherWin.cpp:35 > > +const LPCWSTR webkitDLLName = L"WebKit_debug.dll"; > > Should be webKitDLLName (with a capital "K"). Yes, it should. Fixed. > > >> WebKit2/UIProcess/Launcher/win/ProcessLauncherWin.cpp:57 > >> + HMODULE webkitModule = GetModuleHandle(webkitDLLName); > > > > We've been using :: to namespace quality Win32 calls lately. > > We've also been using the W versions of the APIs, so: ::GetModuleHandleW(webKitDLLName); Fixed. > > >> WebKit2/UIProcess/Launcher/win/ProcessLauncherWin.cpp:63 > >> + if (!GetModuleFileName(webkitModule, pathStr, MAX_PATH) || GetLastError() != ERROR_SUCCESS) > > > > You shouldn't need to check GetLastError and the result of the function. Just checking the result for 0 should be enough to check for failure. > > Same comment about the ::. > > _countof(pathStr) would be better than repeating MAX_PATH. Fixed.
Landed in r67979.
http://trac.webkit.org/changeset/67979 might have broken Qt Windows 32-bit Debug The following changes are on the blame list: http://trac.webkit.org/changeset/67973 http://trac.webkit.org/changeset/67974 http://trac.webkit.org/changeset/67975 http://trac.webkit.org/changeset/67976 http://trac.webkit.org/changeset/67977 http://trac.webkit.org/changeset/67978 http://trac.webkit.org/changeset/67979