Bug 37636 – [DRT/Chromium] Implement DRT/Chromium for Windows

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Roland Steiner]

Adding to Tamura Kent's upstreaming of DRT for Chromium, implement Windows-specific parts.

[Roland Steiner]

Created attachment 53407 [details]
patch 1 - fix compilation errors

(Broke up the patches for easier reviewing)

First patch: fix compilation errors

[Kent Tamura]

Comment on attachment 53407 [details]
patch 1 - fix compilation errors

The patch is ok.

> @@ -638,6 +642,7 @@ void LayoutTestController::pathToLocalResource(const CppArgumentList& arguments,
>      if (StartsWithASCII(url, "/tmp/", true)) {
>          // We want a temp file.
>          const unsigned tempPrefixLength = 5;
> +        // FIXME: replace the below with an existing library function such as file_util::GetTempDir()

nit: I don't think this FIXME is necessary. We should avoid to use file_util in upstreamed code.

[Tony Chang]

Comment on attachment 53407 [details]
patch 1 - fix compilation errors

>+#if !COMPILER(MSVC)
>+// FIXME: As of VS2005 MSVC doesn't honor the friend declaration below. (Find better workaround)
> private:
>+#endif
>     ~TestWebWorker() {}
>     friend class RefCounted<TestWebWorker>;

That's surprising.  Does it work if you include the WTF namespace?  It seems to work in src/webkit/tools/test_shell/test_web_worker.h.

[Roland Steiner]

Created attachment 53411 [details]
patch 2 - implement Windows TestShell methods, DumpRenderTree.gyp modifications

Second patch: adds TestShellWin.cpp with the implementation of TestShell::waitTestFinished().

Changes to DumpRenderTree.gyp to add this file and pull in 'wtf_config' definitions from JavaScriptCore.gyp

[WebKit Review Bot]

Attachment 53411 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebKitTools/DumpRenderTree/chromium/TestShellWin.cpp:33:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebKitTools/DumpRenderTree/chromium/TestShellWin.cpp:34:  Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
WebKitTools/DumpRenderTree/chromium/TestShellWin.cpp:42:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
WebKitTools/DumpRenderTree/chromium/TestShellWin.cpp:65:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
WebKitTools/DumpRenderTree/chromium/TestShellWin.cpp:76:  Use 0 instead of NULL.  [readability/null] [5]
WebKitTools/DumpRenderTree/chromium/TestShellWin.cpp:77:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
WebKitTools/DumpRenderTree/chromium/TestShellWin.cpp:77:  Use 0 instead of NULL.  [readability/null] [5]
WebKitTools/DumpRenderTree/chromium/TestShellWin.cpp:80:  Use 0 instead of NULL.  [readability/null] [5]
WebKitTools/DumpRenderTree/chromium/TestShellWin.cpp:86:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
WebKitTools/DumpRenderTree/chromium/TestShellWin.cpp:86:  Use 0 instead of NULL.  [readability/null] [5]
Total errors found: 10 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Roland Steiner]

Created attachment 53414 [details]
patch 1 - fix compilation errors

Updated patch 1 after Tony's remark: explicitly adding the namespace indeed did pacify the compiler.

[Roland Steiner]

Created attachment 53415 [details]
patch 2 - implement Windows TestShell methods, DumpRenderTree.gyp modifications

Patch 2: address style issues (no functional change to original version)

[WebKit Review Bot]

Attachment 53415 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebKitTools/DumpRenderTree/chromium/TestShellWin.cpp:35:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Roland Steiner]

Created attachment 53419 [details]
patch 1 - fix compilation errors

Some more patch cleanup: address Kent's comment, move compilation-fixing modification to DumpRenderTree.gyp into patch 1. Fix corruption that somehow crept into the last upload.

[Roland Steiner]

Created attachment 53420 [details]
patch 2 - implement Windows TestShell methods, DumpRenderTree.gyp modifications

Patch cleanup for patch 2: Following up on the cleanup for patch 1 this patch now contains only the Windows TestShell implementation.

[Dimitri Glazkov (Google)]

Comment on attachment 53419 [details]
patch 1 - fix compilation errors

ok.

[Dimitri Glazkov (Google)]

Comment on attachment 53420 [details]
patch 2 - implement Windows TestShell methods, DumpRenderTree.gyp modifications

ok.

[WebKit Commit Bot]

Comment on attachment 53419 [details]
patch 1 - fix compilation errors

Clearing flags on attachment: 53419

Committed r57648: <http://trac.webkit.org/changeset/57648>

[WebKit Commit Bot]

Comment on attachment 53420 [details]
patch 2 - implement Windows TestShell methods, DumpRenderTree.gyp modifications

Clearing flags on attachment: 53420

Committed r57649: <http://trac.webkit.org/changeset/57649>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.