[Win] Miscellaneous DRT fixes
authorbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 15 Jan 2015 22:49:26 +0000 (22:49 +0000)
committerbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 15 Jan 2015 22:49:26 +0000 (22:49 +0000)
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.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@178530 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Tools/ChangeLog
Tools/DumpRenderTree/cg/ImageDiffCG.cpp
Tools/DumpRenderTree/win/DumpRenderTree.cpp
Tools/DumpRenderTree/win/TestRunnerWin.cpp
Tools/TestWebKitAPI/win/main.cpp
Tools/win/DLLLauncher/DLLLauncherMain.cpp

index d12d83f..604796c 100644 (file)
@@ -1,3 +1,39 @@
+2015-01-15  Brent Fulgham  <bfulgham@apple.com>
+
+        [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  <ap@apple.com>
 
         Stop hardcoding mac-mountainlion in commit queue
index 8eeea3d..8205fc9 100644 (file)
@@ -225,7 +225,7 @@ int main(int argc, const char* argv[])
             } 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
index 7945409..183027b 100644 (file)
 #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 @@ static wstring lastPathComponentAsWString(CFURLRef url)
 
 wstring urlSuitableForTestResult(const wstring& urlString)
 {
+    if (urlString.empty())
+        return urlString;
+
     RetainPtr<CFURLRef> url = adoptCF(CFURLCreateWithBytes(kCFAllocatorDefault, 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 @@ static void dumpHistoryItem(IWebHistoryItem* item, int indent, bool current)
         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 = newURL;
+        url = _bstr_t(L"(file test):") + _bstr_t(start);
     }
 
     printf("%S", static_cast<wchar_t*>(url));
@@ -1038,25 +1041,22 @@ static void runTest(const string& inputLine)
     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 @@ IWebView* createWebViewAndOffscreenWindow(HWND* webViewWindow)
     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 @@ static void prepareConsistentTestingEnvironment(IWebPreferences* standardPrefere
 
 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>.
index 03ad90a..e44d508 100644 (file)
@@ -273,9 +273,9 @@ void TestRunner::notifyDone()
 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 @@ JSStringRef TestRunner::pathToLocalResource(JSContextRef context, JSStringRef ur
     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 @@ void TestRunner::setUserStyleSheetLocation(JSStringRef jsURL)
         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;
 
index 265471b..d9499c5 100644 (file)
@@ -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)
 {
index 375536f..79f3587 100644 (file)
@@ -48,9 +48,6 @@ using namespace std;
 #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,7 +184,12 @@ int main(int argc, const char* argv[])
 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();