Diff
Modified: trunk/Tools/ChangeLog (178529 => 178530)
--- trunk/Tools/ChangeLog 2015-01-15 22:19:40 UTC (rev 178529)
+++ trunk/Tools/ChangeLog 2015-01-15 22:49:26 UTC (rev 178530)
@@ -1,3 +1,39 @@
+2015-01-15 Brent Fulgham <[email protected]>
+
+ [Win] Miscellaneous DRT fixes
+ https://bugs.webkit.org/show_bug.cgi?id=116562
+
+ Reviewed by Tim Horton.
+
+ While investigating the cause of several Windows crashes, I found:
+ (1) Messy conversions to and from BSTR types
+ (2) Weird mixes of wide-string and narrow string conversions
+ (3) Passing nullptr to some CoreFoundation routines that do not
+ permit null arguments.
+ (4) Commands to link the executable to the VS2005 runtime.
+
+ This patch cleans up these issues to improve DRT reliability on
+ Windows.
+
+ * DumpRenderTree/cg/ImageDiffCG.cpp:
+ (main): Get rid of VS2005 runtime linking.
+ * DumpRenderTree/win/DumpRenderTree.cpp:
+ (urlSuitableForTestResult): Protect against being asked
+ to process an empty URL.
+ (dumpHistoryItem): Do BSTR string building using _bstr_t, rather
+ than converting to/from wchar_t buffers.
+ (runTest): Simplify string and BSTR handling.
+ (createWebViewAndOffscreenWindow): Ditto.
+ (main): Get rid of VS2005 runtime linking.
+ * DumpRenderTree/win/TestRunnerWin.cpp:
+ (jsStringRefToWString): Simplify code.
+ (TestRunner::pathToLocalResource):
+ (TestRunner::setUserStyleSheetLocation):
+ * TestWebKitAPI/win/main.cpp: Get rid of
+ VS2005 runtime linking.
+ * win/DLLLauncher/DLLLauncherMain.cpp:
+ (wWinMain): Ditto.
+
2015-01-15 Alexey Proskuryakov <[email protected]>
Stop hardcoding mac-mountainlion in commit queue
Modified: trunk/Tools/DumpRenderTree/cg/ImageDiffCG.cpp (178529 => 178530)
--- trunk/Tools/DumpRenderTree/cg/ImageDiffCG.cpp 2015-01-15 22:19:40 UTC (rev 178529)
+++ trunk/Tools/DumpRenderTree/cg/ImageDiffCG.cpp 2015-01-15 22:49:26 UTC (rev 178530)
@@ -225,7 +225,7 @@
} else {
if (CGImageGetWidth(actualImage.get()) != CGImageGetWidth(baselineImage.get()) || CGImageGetHeight(actualImage.get()) != CGImageGetHeight(baselineImage.get())) {
#if OS(WINDOWS)
- fprintf(stderr, "Error: test and reference images have different sizes. Test image is %zux%zu, reference image is %Iux%Iu\n",
+ fprintf(stderr, "Error: test and reference images have different sizes. Test image is %Iux%Iu, reference image is %Iux%Iu\n",
#else
fprintf(stderr, "Error: test and reference images have different sizes. Test image is %zux%zu, reference image is %zux%zu\n",
#endif
Modified: trunk/Tools/DumpRenderTree/win/DumpRenderTree.cpp (178529 => 178530)
--- trunk/Tools/DumpRenderTree/win/DumpRenderTree.cpp 2015-01-15 22:19:40 UTC (rev 178529)
+++ trunk/Tools/DumpRenderTree/win/DumpRenderTree.cpp 2015-01-15 22:49:26 UTC (rev 178530)
@@ -43,12 +43,13 @@
#include "WorkQueue.h"
#include <comutil.h>
+#include <cstdio>
+#include <cstring>
#include <fcntl.h>
+#include <fstream>
#include <io.h>
#include <math.h>
#include <shlwapi.h>
-#include <stdio.h>
-#include <string.h>
#include <tchar.h>
#include <wtf/NeverDestroyed.h>
#include <wtf/RetainPtr.h>
@@ -68,9 +69,9 @@
using namespace std;
#ifdef DEBUG_ALL
-const LPWSTR TestPluginDir = L"TestNetscapePlugin_Debug";
+const _bstr_t TestPluginDir = L"TestNetscapePlugin_Debug";
#else
-const LPWSTR TestPluginDir = L"TestNetscapePlugin";
+const _bstr_t TestPluginDir = L"TestNetscapePlugin";
#endif
static LPCWSTR fontsEnvironmentVariable = L"WEBKIT_TESTFONTS";
@@ -143,6 +144,9 @@
wstring urlSuitableForTestResult(const wstring& urlString)
{
+ if (urlString.empty())
+ return urlString;
+
RetainPtr<CFURLRef> url = "" reinterpret_cast<const UInt8*>(urlString.c_str()), urlString.length() * sizeof(wstring::value_type), kCFStringEncodingUTF16, 0));
RetainPtr<CFStringRef> scheme = adoptCF(CFURLCopyScheme(url.get()));
@@ -515,19 +519,18 @@
return;
if (wcsstr(static_cast<wchar_t*>(url), L"file:/") == static_cast<wchar_t*>(url)) {
- static wchar_t* layoutTestsString = L"/LayoutTests/";
- static wchar_t* fileTestString = L"(file test):";
+ static wchar_t* layoutTestsStringUnixPath = L"/LayoutTests/";
+ static wchar_t* layoutTestsStringDOSPath = L"\\LayoutTests\\";
- wchar_t* result = wcsstr(static_cast<wchar_t*>(url), layoutTestsString);
- if (result == NULL)
+ wchar_t* result = wcsstr(static_cast<wchar_t*>(url), layoutTestsStringUnixPath);
+ if (!result)
+ result = wcsstr(static_cast<wchar_t*>(url), layoutTestsStringDOSPath);
+ if (!result)
return;
- wchar_t* start = result + wcslen(layoutTestsString);
- _bstr_t newURL(SysAllocStringLen(0, SysStringLen(url)), false);
- wcscpy(static_cast<wchar_t*>(newURL), fileTestString);
- wcscpy(static_cast<wchar_t*>(newURL) + wcslen(fileTestString), start);
+ wchar_t* start = result + wcslen(layoutTestsStringUnixPath);
- url = ""
+ url = "" test):") + _bstr_t(start);
}
printf("%S", static_cast<wchar_t*>(url));
@@ -1038,25 +1041,22 @@
str = CFURLGetString(url);
CFIndex length = CFStringGetLength(str);
- UniChar* buffer = new UniChar[length + 1];
- CFStringGetCharacters(str, CFRangeMake(0, length), buffer);
- buffer[length] = 0;
+ Vector<UniChar> buffer(length + 1, 0);
+ CFStringGetCharacters(str, CFRangeMake(0, length), buffer.data());
- _bstr_t urlBStr((OLECHAR*)buffer);
+ _bstr_t urlBStr(reinterpret_cast<wchar_t*>(buffer.data()));
ASSERT(urlBStr.length() == length);
- delete[] buffer;
CFIndex maximumURLLengthAsUTF8 = CFStringGetMaximumSizeForEncoding(length, kCFStringEncodingUTF8) + 1;
- char* testURL = new char[maximumURLLengthAsUTF8];
- CFStringGetCString(str, testURL, maximumURLLengthAsUTF8, kCFStringEncodingUTF8);
+ Vector<char> testURL(maximumURLLengthAsUTF8 + 1, 0);
+ CFStringGetCString(str, testURL.data(), maximumURLLengthAsUTF8, kCFStringEncodingUTF8);
CFRelease(url);
resetWebViewToConsistentStateBeforeTesting();
- ::gTestRunner = TestRunner::create(testURL, command.expectedPixelHash);
- delete[] testURL;
+ ::gTestRunner = TestRunner::create(testURL.data(), command.expectedPixelHash);
::gTestRunner->setCustomTimeout(command.timeout);
topLoadingFrame = nullptr;
done = false;
@@ -1220,10 +1220,8 @@
viewPrivate->setShouldApplyMacFontAscentHack(TRUE);
viewPrivate->setAlwaysUsesComplexTextCodePath(forceComplexText);
- _bstr_t pluginPath(SysAllocStringLen(0, exePath().length() + _tcslen(TestPluginDir)), false);
- _tcscpy(static_cast<TCHAR*>(pluginPath), exePath().c_str());
- _tcscat(static_cast<TCHAR*>(pluginPath), TestPluginDir);
- if (FAILED(viewPrivate->addAdditionalPluginDirectory(pluginPath)))
+ _bstr_t pluginPath = _bstr_t(exePath().data()) + TestPluginDir;
+ if (FAILED(viewPrivate->addAdditionalPluginDirectory(pluginPath.GetBSTR())))
return nullptr;
HWND viewWindow;
@@ -1384,11 +1382,6 @@
int main(int argc, const char* argv[])
{
-#ifdef _CRTDBG_MAP_ALLOC
- _CrtSetReportFile(_CRT_WARN, _CRTDBG_FILE_STDERR);
- _CrtSetReportMode(_CRT_WARN, _CRTDBG_MODE_FILE);
-#endif
-
// Cygwin calls ::SetErrorMode(SEM_FAILCRITICALERRORS), which we will inherit. This is bad for
// testing/debugging, as it causes the post-mortem debugger not to be invoked. We reset the
// error mode here to work around Cygwin's behavior. See <http://webkit.org/b/55222>.
Modified: trunk/Tools/DumpRenderTree/win/TestRunnerWin.cpp (178529 => 178530)
--- trunk/Tools/DumpRenderTree/win/TestRunnerWin.cpp 2015-01-15 22:19:40 UTC (rev 178529)
+++ trunk/Tools/DumpRenderTree/win/TestRunnerWin.cpp 2015-01-15 22:49:26 UTC (rev 178530)
@@ -273,9 +273,9 @@
static wstring jsStringRefToWString(JSStringRef jsStr)
{
size_t length = JSStringGetLength(jsStr);
- Vector<WCHAR> buffer(length + 1);
+ Vector<WCHAR> buffer(length + 1, 0);
memcpy(buffer.data(), JSStringGetCharactersPtr(jsStr), length * sizeof(WCHAR));
- buffer[length] = '\0';
+ buffer[length] = 0;
return buffer.data();
}
@@ -287,7 +287,7 @@
wstring localPath;
if (!resolveCygwinPath(input, localPath)) {
printf("ERROR: Failed to resolve Cygwin path %S\n", input.c_str());
- return 0;
+ return nullptr;
}
return JSStringCreateWithCharacters(localPath.c_str(), localPath.length());
@@ -766,9 +766,9 @@
return;
// The path has been resolved, now convert it back to a CFURL.
- int result = WideCharToMultiByte(CP_UTF8, 0, resultPath.c_str(), resultPath.size() + 1, 0, 0, 0, 0);
+ int result = ::WideCharToMultiByte(CP_UTF8, 0, resultPath.c_str(), resultPath.size() + 1, nullptr, 0, nullptr, nullptr);
Vector<char> utf8Vector(result);
- result = WideCharToMultiByte(CP_UTF8, 0, resultPath.c_str(), resultPath.size() + 1, utf8Vector.data(), result, 0, 0);
+ result = ::WideCharToMultiByte(CP_UTF8, 0, resultPath.c_str(), resultPath.size() + 1, utf8Vector.data(), result, nullptr, nullptr);
if (!result)
return;
Modified: trunk/Tools/TestWebKitAPI/win/main.cpp (178529 => 178530)
--- trunk/Tools/TestWebKitAPI/win/main.cpp 2015-01-15 22:19:40 UTC (rev 178529)
+++ trunk/Tools/TestWebKitAPI/win/main.cpp 2015-01-15 22:49:26 UTC (rev 178530)
@@ -38,9 +38,6 @@
#endif
#pragma comment(linker, "/manifestdependency:\"type='win32' name='Microsoft.Windows.Common-Controls' version='6.0.0.0' processorArchitecture='" PROCESSORARCHITECTURE "' publicKeyToken='6595b64144ccf1df' language='*'\"")
-#if defined(_MSC_VER) && (_MSC_VER >= 1600)
-#pragma comment(linker, "/manifestdependency:\"type='win32' name='Microsoft.VC80.CRT' version='8.0.50727.6195' processorArchitecture='" PROCESSORARCHITECTURE "' publicKeyToken='1fc8b3b9a1e18e3b' language='*'\"")
-#endif
int main(int argc, char** argv)
{
Modified: trunk/Tools/win/DLLLauncher/DLLLauncherMain.cpp (178529 => 178530)
--- trunk/Tools/win/DLLLauncher/DLLLauncherMain.cpp 2015-01-15 22:19:40 UTC (rev 178529)
+++ trunk/Tools/win/DLLLauncher/DLLLauncherMain.cpp 2015-01-15 22:49:26 UTC (rev 178530)
@@ -48,9 +48,6 @@
#endif
#pragma comment(linker, "/manifestdependency:\"type='win32' name='Microsoft.Windows.Common-Controls' version='6.0.0.0' processorArchitecture='" PROCESSORARCHITECTURE "' publicKeyToken='6595b64144ccf1df' language='*'\"")
-#if defined(_MSC_VER) && (_MSC_VER >= 1600) && !defined(WIN_CAIRO)
-#pragma comment(linker, "/manifestdependency:\"type='win32' name='Microsoft.VC80.CRT' version='8.0.50727.6195' processorArchitecture='" PROCESSORARCHITECTURE "' publicKeyToken='1fc8b3b9a1e18e3b' language='*'\"")
-#endif
static void enableTerminationOnHeapCorruption()
{
@@ -187,8 +184,13 @@
int WINAPI wWinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPWSTR lpstrCmdLine, int nCmdShow)
#endif
{
- _CrtSetDbgFlag(_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF | _CRTDBG_CHECK_ALWAYS_DF);
+#ifdef _CRTDBG_MAP_ALLOC
+ _CrtSetReportFile(_CRT_WARN, _CRTDBG_FILE_STDERR);
+ _CrtSetReportMode(_CRT_WARN, _CRTDBG_MODE_FILE);
+#endif
+ _CrtSetDbgFlag(_CRTDBG_ALLOC_MEM_DF | _CRTDBG_DELAY_FREE_MEM_DF | _CRTDBG_CHECK_ALWAYS_DF);
+
enableTerminationOnHeapCorruption();
// Get the path of our executable.
