Attachment 163946 Details for Bug 96687 – Patch

[patch] Patch

bug-96687-20120913210757.patch (text/plain), 17.94 KB, created by Peter Beverloo on 2012-09-13 13:08:25 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Peter Beverloo

Created: 2012-09-13 13:08:25 PDT

Size: 17.94 KB

patch

obsolete

>Subversion Revision: 128476
>diff --git a/Source/WebKit/chromium/ChangeLog b/Source/WebKit/chromium/ChangeLog
>index e3bc30b20f25377a4df57c956eee52992c2d6576..bf7b968bc63daaeabeae7ab2f8e486a5133a6cd4 100644
>--- a/Source/WebKit/chromium/ChangeLog
>+++ b/Source/WebKit/chromium/ChangeLog
>@@ -1,3 +1,38 @@
>+2012-09-13  Peter Beverloo  <peter@chromium.org>
>+
>+        [Chromium] Support the --{in,out,err}-fifo arguments on TestWebKitAPI and webkit_unit_tests
>+        https://bugs.webkit.org/show_bug.cgi?id=96687
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Android's DumpRenderTree currently supports these arguments, implemented
>+        as part of TestShellAndroid:
>+        http://trac.webkit.org/browser/trunk/Tools/DumpRenderTree/chromium/TestShellAndroid.cpp?rev=128496
>+
>+        They're used by the layout test runner to get the STDOUT and STDERR while
>+        a layout test run is in process, which is a safer alternative to parsing
>+        all the logcat output manually. The implementation can be seen here:
>+        http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py?rev=128496#L590
>+
>+        This patch generalizes parsing of and applying the effects of these arguments
>+        so that they can be used for TestWebKitAPI and webkit_unit_tests as well.
>+        After this patch, this will make it possible to pull out the output-reading
>+        code from Android's layout test port and generalize it so it can be re-used
>+        in the new test-runner for the other two test suites.
>+
>+        This has no effect when compiling and running these tests as part of Chromium
>+        code, which has a much more advanced test-runner that does parse complete log
>+        output, but also directly depends on code licensed under Apache 2.
>+
>+        * WebKit.gypi:
>+        * tests/ForwardIOStreamsAndroid.cpp: Added.
>+        (WebKit):
>+        (WebKit::maybeInitIOStreamForwardingForAndroid):
>+        * tests/ForwardIOStreamsAndroid.h: Added.
>+        (WebKit):
>+        * tests/RunAllTests.cpp:
>+        (main):
>+
> 2012-09-13  Ilya Tikhonovsky  <loislo@chromium.org>
> 
>         Web Inspector: NMI: instrument KURL directly.
>diff --git a/Source/WebKit/chromium/WebKit.gypi b/Source/WebKit/chromium/WebKit.gypi
>index 5d3f850d5a1eb35b7d76b61aa1e19df3e5f9ed41..0a1e4b91fd3a406f0ed8e17e13659584d0f18b30 100644
>--- a/Source/WebKit/chromium/WebKit.gypi
>+++ b/Source/WebKit/chromium/WebKit.gypi
>@@ -240,6 +240,12 @@
>                     'tests/ScrollAnimatorNoneTest.cpp',
>                 ],
>             }],
>+            ['OS=="android"', {
>+                'webkit_unittest_files': [
>+                    'tests/ForwardIOStreamsAndroid.cpp',
>+                    'tests/ForwardIOStreamsAndroid.h',
>+                ],
>+            }],
>             ['os_posix==1 and OS!="mac"', {
>                 'webkit_unittest_files': [
>                     'tests/LocalizedDateICUTest.cpp',
>diff --git a/Source/WebKit/chromium/tests/ForwardIOStreamsAndroid.cpp b/Source/WebKit/chromium/tests/ForwardIOStreamsAndroid.cpp
>new file mode 100644
>index 0000000000000000000000000000000000000000..1bba17de66654f35335e0a412fc9a52297d6809e
>--- /dev/null
>+++ b/Source/WebKit/chromium/tests/ForwardIOStreamsAndroid.cpp
>@@ -0,0 +1,120 @@
>+/*
>+ * Copyright (C) 2012 Google Inc. All rights reserved.
>+ *
>+ * Redistribution and use in source and binary forms, with or without
>+ * modification, are permitted provided that the following conditions
>+ * are met:
>+ *
>+ * 1.  Redistributions of source code must retain the above copyright
>+ *      notice, this list of conditions and the following disclaimer.
>+ * 2.  Redistributions in binary form must reproduce the above copyright
>+ *      notice, this list of conditions and the following disclaimer in the
>+ *      documentation and/or other materials provided with the distribution.
>+ *
>+ * THIS SOFTWARE IS PROVIDED BY APPLE AND ITS CONTRIBUTORS "AS IS" AND ANY
>+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
>+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
>+ * DISCLAIMED. IN NO EVENT SHALL APPLE OR ITS CONTRIBUTORS BE LIABLE FOR ANY
>+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
>+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
>+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
>+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
>+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>+ */
>+
>+#include "config.h"
>+
>+#include "ForwardIOStreamsAndroid.h"
>+
>+#include <android/log.h>
>+#include <errno.h>
>+#include <fcntl.h>
>+#include <stdio.h>
>+#include <sys/stat.h>
>+#include <sys/types.h>
>+#include <unistd.h>
>+#include <wtf/StdLibExtras.h>
>+
>+namespace {
>+
>+const char optionInFIFO[] = "--in-fifo=";
>+const char optionOutFIFO[] = "--out-fifo=";
>+const char optionErrFIFO[] = "--err-fifo=";
>+
>+void androidLogError(const char* format, ...) WTF_ATTRIBUTE_PRINTF(1, 2);
>+void androidLogError(const char* format, ...)
>+{
>+    va_list args;
>+    va_start(args, format);
>+    __android_log_vprint(ANDROID_LOG_ERROR, "WebKit", format, args);
>+    va_end(args);
>+}
>+
>+void removeArg(int index, int* argc, char*** argv)
>+{
>+    for (int i = index; i < *argc; ++i)
>+        (*argv)[i] = (*argv)[i + 1];
>+    --*argc;
>+}
>+
>+void createFIFO(const char* fifoPath)
>+{
>+    unlink(fifoPath);
>+    // 0666 is rw-rw-rw-, to allow adb shell to read/write the fifo.
>+    // Explicitly call chmod to ensure the mode is set despite umask.
>+    if (mkfifo(fifoPath, 0666) || chmod(fifoPath, 0666)) {
>+        androidLogError("Failed to create fifo %s: %s\n", fifoPath, strerror(errno));
>+        exit(EXIT_FAILURE);
>+    }
>+}
>+
>+void redirect(FILE* stream, const char* path, const char* mode)
>+{
>+    if (!freopen(path, mode, stream)) {
>+        androidLogError("Failed to redirect stream to file: %s: %s\n", path, strerror(errno));
>+        exit(EXIT_FAILURE);
>+    }
>+}
>+
>+} // namespace
>+
>+namespace WebKit {
>+
>+void maybeInitIOStreamForwardingForAndroid(int* argc, char*** argv)
>+{
>+    const char* inFIFO = 0;
>+    const char* outFIFO = 0;
>+    const char* errFIFO = 0;
>+    for (int i = 1; i < *argc; ) {
>+        const char* argument = (*argv)[i];
>+        if (strstr(argument, optionInFIFO) == argument) {
>+            inFIFO = argument + WTF_ARRAY_LENGTH(optionInFIFO) - 1;
>+            createFIFO(inFIFO);
>+            removeArg(i, argc, argv);
>+        } else if (strstr(argument, optionOutFIFO) == argument) {
>+            outFIFO = argument + WTF_ARRAY_LENGTH(optionOutFIFO) - 1;
>+            createFIFO(outFIFO);
>+            removeArg(i, argc, argv);
>+        } else if (strstr(argument, optionErrFIFO) == argument) {
>+            errFIFO = argument + WTF_ARRAY_LENGTH(optionErrFIFO) - 1;
>+            createFIFO(errFIFO);
>+            removeArg(i, argc, argv);
>+        } else
>+            ++i;
>+    }
>+
>+    // The order of createFIFO() and redirectToFIFO() is important to avoid deadlock.
>+    if (outFIFO)
>+        redirect(stdout, outFIFO, "w");
>+    if (inFIFO)
>+        redirect(stdin, inFIFO, "r");
>+    if (errFIFO)
>+        redirect(stderr, errFIFO, "w");
>+    else {
>+        // Redirect stderr to stdout.
>+        dup2(1, 2);
>+    }
>+}
>+
>+} // namespace WebKit
>diff --git a/Source/WebKit/chromium/tests/ForwardIOStreamsAndroid.h b/Source/WebKit/chromium/tests/ForwardIOStreamsAndroid.h
>new file mode 100644
>index 0000000000000000000000000000000000000000..6dbc638b4d429abc3f5c3628f853bb04ac18398a
>--- /dev/null
>+++ b/Source/WebKit/chromium/tests/ForwardIOStreamsAndroid.h
>@@ -0,0 +1,40 @@
>+/*
>+ * Copyright (C) 2012 Google Inc. All rights reserved.
>+ *
>+ * Redistribution and use in source and binary forms, with or without
>+ * modification, are permitted provided that the following conditions
>+ * are met:
>+ *
>+ * 1.  Redistributions of source code must retain the above copyright
>+ *      notice, this list of conditions and the following disclaimer.
>+ * 2.  Redistributions in binary form must reproduce the above copyright
>+ *      notice, this list of conditions and the following disclaimer in the
>+ *      documentation and/or other materials provided with the distribution.
>+ *
>+ * THIS SOFTWARE IS PROVIDED BY APPLE AND ITS CONTRIBUTORS "AS IS" AND ANY
>+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
>+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
>+ * DISCLAIMED. IN NO EVENT SHALL APPLE OR ITS CONTRIBUTORS BE LIABLE FOR ANY
>+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
>+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
>+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
>+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
>+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>+ */
>+
>+#ifndef ForwardIOStreamsAndroid_h
>+#define ForwardIOStreamsAndroid_h
>+
>+namespace WebKit {
>+
>+// The test executables for Android support three additional command line flags
>+// (--in-fifo, --out-fifo and --err-fifo) to make it possible to retrieve input
>+// from a file instead of STDIN, and to forward STDOUT and STDERR to files. When
>+// running DumpRenderTree, TestWebKitAPI or webkit_unit_tests in WebKit
>+// infrastructure, these will be used instead of parsing all log output.
>+void maybeInitIOStreamForwardingForAndroid(int* argc, char*** argv);
>+
>+} // namespace WebKit
>+
>+#endif // ForwardIOStreamsAndroid_h
>diff --git a/Source/WebKit/chromium/tests/RunAllTests.cpp b/Source/WebKit/chromium/tests/RunAllTests.cpp
>index 78fd7da3d310b5e2b95851467e9430e06d7f4599..be4c7db290e06311fb2bebe73aaf58f4e75b2778 100644
>--- a/Source/WebKit/chromium/tests/RunAllTests.cpp
>+++ b/Source/WebKit/chromium/tests/RunAllTests.cpp
>@@ -43,6 +43,10 @@
> #include "WebUnitTests.h"
> #endif
> 
>+#if defined(OS_ANDROID)
>+#include "ForwardIOStreamsAndroid.h"
>+#endif
>+
> #include <gmock/gmock.h>
> 
> // TestSuite must be created before SetUpTestEnvironment so it performs
>@@ -59,6 +63,9 @@ int main(int argc, char** argv)
>     WebKit::DeleteTestSuite();
> #else
>     ::testing::InitGoogleMock(&argc, argv);
>+#if defined(OS_ANDROID)
>+    WebKit::maybeInitIOStreamForwardingForAndroid(&argc, &argv);
>+#endif
>     TestSuite testSuite(argc, argv);
>     webkit_support::SetUpTestEnvironmentForUnitTests();
>     int result = testSuite.Run();
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 7b89dc621ed0f9158fed1f382a7aa417c381540b..5a433fda0b28df84dc142faf39cf57873378dfa2 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,34 @@
>+2012-09-13  Peter Beverloo  <peter@chromium.org>
>+
>+        [Chromium] Support the --{in,out,err}-fifo arguments on TestWebKitAPI and webkit_unit_tests
>+        https://bugs.webkit.org/show_bug.cgi?id=96687
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Android's DumpRenderTree currently supports these arguments, implemented
>+        as part of TestShellAndroid:
>+        http://trac.webkit.org/browser/trunk/Tools/DumpRenderTree/chromium/TestShellAndroid.cpp?rev=128496
>+
>+        They're used by the layout test runner to get the STDOUT and STDERR while
>+        a layout test run is in process, which is a safer alternative to parsing
>+        all the logcat output manually. The implementation can be seen here:
>+        http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py?rev=128496#L590
>+
>+        This patch generalizes parsing of and applying the effects of these arguments
>+        so that they can be used for TestWebKitAPI and webkit_unit_tests as well.
>+        After this patch, this will make it possible to pull out the output-reading
>+        code from Android's layout test port and generalize it so it can be re-used
>+        in the new test-runner for the other two test suites.
>+
>+        This has no effect when compiling and running these tests as part of Chromium
>+        code, which has a much more advanced test-runner that does parse complete log
>+        output, but also directly depends on code licensed under Apache 2.
>+
>+        * DumpRenderTree/DumpRenderTree.gyp/DumpRenderTree.gyp:
>+        * DumpRenderTree/chromium/TestShellAndroid.cpp:
>+        (platformInit):
>+        * TestWebKitAPI/TestWebKitAPI.gyp/TestWebKitAPI.gyp:
>+
> 2012-09-13  Simon Hausmann  <simon.hausmann@nokia.com>
> 
>         [Qt] Make WebKit2 work on Windows with Qt 5
>diff --git a/Tools/DumpRenderTree/DumpRenderTree.gyp/DumpRenderTree.gyp b/Tools/DumpRenderTree/DumpRenderTree.gyp/DumpRenderTree.gyp
>index 8e3347ca82ac7d51617d22caf38194bdfe3e22bf..111e46090b48b645f845d0f0948537ce589c460b 100644
>--- a/Tools/DumpRenderTree/DumpRenderTree.gyp/DumpRenderTree.gyp
>+++ b/Tools/DumpRenderTree/DumpRenderTree.gyp/DumpRenderTree.gyp
>@@ -300,6 +300,10 @@
>                             '<(INTERMEDIATE_DIR)/repack/DumpRenderTree.pak',
>                         ]
>                     }],
>+                    'sources': [
>+                        '<(chromium_src_dir)/tests/ForwardIOStreamsAndroid.cpp',
>+                        '<(chromium_src_dir)/tests/ForwardIOStreamsAndroid.h',
>+                    ],
>                 }, { # OS!="android"
>                     'sources/': [
>                         ['exclude', 'Android\\.cpp$'],
>diff --git a/Tools/DumpRenderTree/chromium/TestShellAndroid.cpp b/Tools/DumpRenderTree/chromium/TestShellAndroid.cpp
>index 188158440ba681691bfc98f1922e0c6eb9be4e9c..8f8eedc3dbc47b2b9eb3110db7492033f01741c0 100644
>--- a/Tools/DumpRenderTree/chromium/TestShellAndroid.cpp
>+++ b/Tools/DumpRenderTree/chromium/TestShellAndroid.cpp
>@@ -32,15 +32,8 @@
> #include "TestShell.h"
> 
> #include "linux/WebFontRendering.h"
>+#include "tests/ForwardIOStreamsAndroid.h"
> #include "third_party/skia/include/ports/SkTypeface_android.h"
>-#include <android/log.h>
>-#include <errno.h>
>-#include <fcntl.h>
>-#include <stdio.h>
>-#include <sys/stat.h>
>-#include <sys/types.h>
>-#include <unistd.h>
>-#include <wtf/Assertions.h>
> 
> namespace {
> 
>@@ -51,46 +44,6 @@ const char fontMainConfigFile[] = DEVICE_DRT_DIR "android_main_fonts.xml";
> const char fontFallbackConfigFile[] = DEVICE_DRT_DIR "android_fallback_fonts.xml";
> const char fontsDir[] = DEVICE_DRT_DIR "fonts/";
> 
>-const char optionInFIFO[] = "--in-fifo=";
>-const char optionOutFIFO[] = "--out-fifo=";
>-const char optionErrFIFO[] = "--err-fifo=";
>-
>-void androidLogError(const char* format, ...) WTF_ATTRIBUTE_PRINTF(1, 2);
>-
>-void androidLogError(const char* format, ...)
>-{
>-    va_list args;
>-    va_start(args, format);
>-    __android_log_vprint(ANDROID_LOG_ERROR, "DumpRenderTree", format, args);
>-    va_end(args);
>-}
>-
>-void removeArg(int index, int* argc, char*** argv)
>-{
>-    for (int i = index; i < *argc; ++i)
>-        (*argv)[i] = (*argv)[i + 1];
>-    --*argc;
>-}
>-
>-void createFIFO(const char* fifoPath)
>-{
>-    unlink(fifoPath);
>-    // 0666 is rw-rw-rw-, to allow adb shell to read/write the fifo.
>-    // Explicitly call chmod to ensure the mode is set despite umask.
>-    if (mkfifo(fifoPath, 0666) || chmod(fifoPath, 0666)) {
>-        androidLogError("Failed to create fifo %s: %s\n", fifoPath, strerror(errno));
>-        exit(EXIT_FAILURE);
>-    }
>-}
>-
>-void redirect(FILE* stream, const char* path, const char* mode)
>-{
>-    if (!freopen(path, mode, stream)) {
>-        androidLogError("Failed to redirect stream to file: %s: %s\n", path, strerror(errno));
>-        exit(EXIT_FAILURE);
>-    }
>-}
>-
> } // namespace
> 
> void platformInit(int* argc, char*** argv)
>@@ -98,38 +51,8 @@ void platformInit(int* argc, char*** argv)
>     // Initialize skia with customized font config files.
>     SkUseTestFontConfigFile(fontMainConfigFile, fontFallbackConfigFile, fontsDir);
> 
>-    const char* inFIFO = 0;
>-    const char* outFIFO = 0;
>-    const char* errFIFO = 0;
>-    for (int i = 1; i < *argc; ) {
>-        const char* argument = (*argv)[i];
>-        if (strstr(argument, optionInFIFO) == argument) {
>-            inFIFO = argument + WTF_ARRAY_LENGTH(optionInFIFO) - 1;
>-            createFIFO(inFIFO);
>-            removeArg(i, argc, argv);
>-        } else if (strstr(argument, optionOutFIFO) == argument) {
>-            outFIFO = argument + WTF_ARRAY_LENGTH(optionOutFIFO) - 1;
>-            createFIFO(outFIFO);
>-            removeArg(i, argc, argv);
>-        } else if (strstr(argument, optionErrFIFO) == argument) {
>-            errFIFO = argument + WTF_ARRAY_LENGTH(optionErrFIFO) - 1;
>-            createFIFO(errFIFO);
>-            removeArg(i, argc, argv);
>-        } else
>-            ++i;
>-    }
>-
>-    // The order of createFIFO() and redirectToFIFO() is important to avoid deadlock.
>-    if (outFIFO)
>-        redirect(stdout, outFIFO, "w");
>-    if (inFIFO)
>-        redirect(stdin, inFIFO, "r");
>-    if (errFIFO)
>-        redirect(stderr, errFIFO, "w");
>-    else {
>-        // Redirect stderr to stdout.
>-        dup2(1, 2);
>-    }
>+    // Set up IO stream forwarding if necessary.
>+    WebKit::maybeInitIOStreamForwardingForAndroid(argc, argv);
> 
>     // Disable auto hint and use normal hinting in layout test mode to produce the same font metrics as chromium-linux.
>     WebKit::WebFontRendering::setAutoHint(false);
>diff --git a/Tools/TestWebKitAPI/TestWebKitAPI.gyp/TestWebKitAPI.gyp b/Tools/TestWebKitAPI/TestWebKitAPI.gyp/TestWebKitAPI.gyp
>index 28e82ac071d60e4f4a46e6f7ee2099b3b649d91e..f6bd1b1edfe4026f18814514f03ed32eaea0dc8e 100644
>--- a/Tools/TestWebKitAPI/TestWebKitAPI.gyp/TestWebKitAPI.gyp
>+++ b/Tools/TestWebKitAPI/TestWebKitAPI.gyp/TestWebKitAPI.gyp
>@@ -89,6 +89,13 @@
>                         '<(chromium_src_dir)/testing/android/native_test.gyp:native_test_native_code',
>                     ],
>                 }],
>+                ['OS=="android"', {
>+                    'sources': [
>+                        # Reuse the IO stream forwarding procedures from webkit_unit_tests.
>+                        '<(source_dir)/WebKit/chromium/tests/ForwardIOStreamsAndroid.cpp',
>+                        '<(source_dir)/WebKit/chromium/tests/ForwardIOStreamsAndroid.h',
>+                    ],
>+                }],
>             ],
>         },
>     ], # targets

Actions: View | Formatted Diff | Diff

Attachments on bug 96687: 163946 | 164158