Simplify WebKitTestRunner timeout tracking
authorap@apple.com <ap@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 2 Jan 2015 22:49:44 +0000 (22:49 +0000)
committerap@apple.com <ap@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 2 Jan 2015 22:49:44 +0000 (22:49 +0000)
https://bugs.webkit.org/show_bug.cgi?id=140036

Reviewed by Darin Adler.

The code for configuring timeouts was mostly dead, because run-webkit-tests never
passes the --timeout option to WebKitTestRunner.

* WebKitTestRunner/Options.h:
* WebKitTestRunner/Options.cpp:
(WTR::Options::Options):
(WTR::OptionsHandler::OptionsHandler):
(WTR::handleOptionTimeout): Deleted.
Removed support for --timeout. Timeouts are passed for each test individually,
and defaults are good enough for the rare cases where WebKitTestRunner is run
manually without run-webkit-tests.

* WebKitTestRunner/TestController.cpp:
(WTR::TestController::TestController):
(WTR::TestController::initialize):
(WTR::TestController::resetStateToConsistentValues):
(WTR::TestController::reattachPageToWebProcess):
(WTR::TestController::runUntil):
* WebKitTestRunner/TestController.h:
Simplified runUntil by passing the actual timeout, not an enum.
Increased short timeout for ASan enabled builds, as WebProcess launching takes
quite a while.

* WebKitTestRunner/TestInvocation.cpp:
(WTR::TestInvocation::invoke): Removed dead code that handled a timeout from NoTimeout.

* WebKitTestRunner/efl/TestControllerEfl.cpp:
(WTR::TestController::platformRunUntil):
* WebKitTestRunner/gtk/TestControllerGtk.cpp:
(WTR::TestController::platformRunUntil):
Build fixes.

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

Tools/ChangeLog
Tools/WebKitTestRunner/Options.cpp
Tools/WebKitTestRunner/Options.h
Tools/WebKitTestRunner/TestController.cpp
Tools/WebKitTestRunner/TestController.h
Tools/WebKitTestRunner/TestInvocation.cpp
Tools/WebKitTestRunner/efl/TestControllerEfl.cpp
Tools/WebKitTestRunner/gtk/TestControllerGtk.cpp

index c7c37f6..7662ca7 100644 (file)
@@ -1,3 +1,42 @@
+2015-01-01  Alexey Proskuryakov  <ap@apple.com>
+
+        Simplify WebKitTestRunner timeout tracking
+        https://bugs.webkit.org/show_bug.cgi?id=140036
+
+        Reviewed by Darin Adler.
+
+        The code for configuring timeouts was mostly dead, because run-webkit-tests never
+        passes the --timeout option to WebKitTestRunner.
+
+        * WebKitTestRunner/Options.h:
+        * WebKitTestRunner/Options.cpp:
+        (WTR::Options::Options):
+        (WTR::OptionsHandler::OptionsHandler):
+        (WTR::handleOptionTimeout): Deleted.
+        Removed support for --timeout. Timeouts are passed for each test individually,
+        and defaults are good enough for the rare cases where WebKitTestRunner is run
+        manually without run-webkit-tests.
+
+        * WebKitTestRunner/TestController.cpp:
+        (WTR::TestController::TestController):
+        (WTR::TestController::initialize):
+        (WTR::TestController::resetStateToConsistentValues):
+        (WTR::TestController::reattachPageToWebProcess):
+        (WTR::TestController::runUntil):
+        * WebKitTestRunner/TestController.h:
+        Simplified runUntil by passing the actual timeout, not an enum.
+        Increased short timeout for ASan enabled builds, as WebProcess launching takes
+        quite a while.
+
+        * WebKitTestRunner/TestInvocation.cpp:
+        (WTR::TestInvocation::invoke): Removed dead code that handled a timeout from NoTimeout.
+
+        * WebKitTestRunner/efl/TestControllerEfl.cpp:
+        (WTR::TestController::platformRunUntil):
+        * WebKitTestRunner/gtk/TestControllerGtk.cpp:
+        (WTR::TestController::platformRunUntil):
+        Build fixes.
+
 2015-01-02  Anders Carlsson  <andersca@apple.com>
 
         Remove now unused storage tracker glue from DumpRenderTree
index 8fa6e6e..be3e237 100644 (file)
 
 namespace WTR {
 
-Options::Options(double defaultLongTimeout, double defaultShortTimeout)
-    : longTimeout(defaultLongTimeout)
-    , shortTimeout(defaultShortTimeout)
-    , useWaitToDumpWatchdogTimer(true)
+Options::Options()
+    : useWaitToDumpWatchdogTimer(true)
     , forceNoTimeout(false)
     , verbose(false)
     , gcBetweenTests(false)
@@ -43,19 +41,9 @@ Options::Options(double defaultLongTimeout, double defaultShortTimeout)
     , forceComplexText(false)
     , shouldUseAcceleratedDrawing(false)
     , shouldUseRemoteLayerTree(false)
-    , defaultLongTimeout(defaultLongTimeout)
-    , defaultShortTimeout(defaultShortTimeout)
 {
 }
 
-bool handleOptionTimeout(Options& options, const char*, const char* argument)
-{
-    options.longTimeout = atoi(argument);
-    // Scale up the short timeout to match.
-    options.shortTimeout = options.defaultShortTimeout * options.longTimeout / options.defaultLongTimeout;
-    return true;
-}
-
 bool handleOptionNoTimeout(Options& options, const char*, const char*)
 {
     options.useWaitToDumpWatchdogTimer = false;
@@ -122,8 +110,7 @@ bool handleOptionUnmatched(Options& options, const char* option, const char*)
 OptionsHandler::OptionsHandler(Options& o)
     : options(o)
 {
-    optionList.append(Option("--timeout", "Sets long timeout to <param> and scales short timeout.", handleOptionTimeout, true));
-    optionList.append(Option("--no-timeout", "Disables timeout.", handleOptionNoTimeout));
+    optionList.append(Option("--no-timeout", "Disables waitUntilDone timeout.", handleOptionNoTimeout));
     optionList.append(Option("--no-timeout-at-all", "Disables all timeouts.", handleOptionNoTimeoutAtAll));
     optionList.append(Option("--verbose", "Turns on messages.", handleOptionVerbose));
     optionList.append(Option("--gc-between-tests", "Garbage collection between tests.", handleOptionGcBetweenTests));
index a89d25c..5ba6aad 100644 (file)
@@ -36,9 +36,7 @@
 namespace WTR {
 
 struct Options {
-    Options(double, double);
-    double longTimeout;
-    double shortTimeout;
+    Options();
     bool useWaitToDumpWatchdogTimer;
     bool forceNoTimeout;
     bool verbose;
@@ -49,8 +47,6 @@ struct Options {
     bool shouldUseAcceleratedDrawing;
     bool shouldUseRemoteLayerTree;
     std::vector<std::string> paths;
-    double defaultLongTimeout;
-    double defaultShortTimeout;
 };
 
 class Option {
index d25d91e..c117c22 100644 (file)
@@ -70,13 +70,17 @@ const unsigned TestController::viewHeight = 600;
 const unsigned TestController::w3cSVGViewWidth = 480;
 const unsigned TestController::w3cSVGViewHeight = 360;
 
-// defaultLongTimeout + defaultShortTimeout should be less than 35,
-// the default timeout value of the test harness so we can detect an
-// unresponsive web process.
-// These values are only used by ports that don't have --timeout option passed to WebKitTestRunner.
-static const double defaultLongTimeout = 25;
-static const double defaultShortTimeout = 5;
-static const double defaultNoTimeout = -1;
+#if defined(__has_feature)
+#if __has_feature(address_sanitizer)
+const double TestController::shortTimeout = 10.0;
+#else
+const double TestController::shortTimeout = 5.0;
+#endif
+#else
+const double TestController::shortTimeout = 5.0;
+#endif
+
+const double TestController::noTimeout = -1;
 
 static WKURLRef blankURL()
 {
@@ -106,9 +110,6 @@ TestController::TestController(int argc, const char* argv[])
     , m_shouldDumpPixelsForAllTests(false)
     , m_state(Initial)
     , m_doneResetting(false)
-    , m_longTimeout(defaultLongTimeout)
-    , m_shortTimeout(defaultShortTimeout)
-    , m_noTimeout(defaultNoTimeout)
     , m_useWaitToDumpWatchdogTimer(true)
     , m_forceNoTimeout(false)
     , m_didPrintWebProcessCrashedMessage(false)
@@ -337,7 +338,7 @@ void TestController::initialize(int argc, const char* argv[])
 {
     platformInitialize();
 
-    Options options(defaultLongTimeout, defaultShortTimeout);
+    Options options;
     OptionsHandler optionsHandler(options);
 
     if (argc < 2) {
@@ -347,8 +348,6 @@ void TestController::initialize(int argc, const char* argv[])
     if (!optionsHandler.parse(argc, argv))
         exit(1);
 
-    m_longTimeout = options.longTimeout;
-    m_shortTimeout = options.shortTimeout;
     m_useWaitToDumpWatchdogTimer = options.useWaitToDumpWatchdogTimer;
     m_forceNoTimeout = options.forceNoTimeout;
     m_verbose = options.verbose;
@@ -740,7 +739,7 @@ bool TestController::resetStateToConsistentValues()
     m_doneResetting = false;
 
     WKPageLoadURL(m_mainWebView->page(), blankURL());
-    runUntil(m_doneResetting, ShortTimeout);
+    runUntil(m_doneResetting, shortTimeout);
     return m_doneResetting;
 }
 
@@ -754,7 +753,7 @@ void TestController::reattachPageToWebProcess()
     // Loading a web page is the only way to reattach an existing page to a process.
     m_doneResetting = false;
     WKPageLoadURL(m_mainWebView->page(), blankURL());
-    runUntil(m_doneResetting, LongTimeout);
+    runUntil(m_doneResetting, shortTimeout);
 }
 
 void TestController::updateWebViewSizeForTest(const TestInvocation& test)
@@ -962,23 +961,10 @@ void TestController::run()
     }
 }
 
-void TestController::runUntil(bool& done, TimeoutDuration timeoutDuration)
+void TestController::runUntil(bool& done, double timeout)
 {
-    double timeout = m_noTimeout;
-    if (!m_forceNoTimeout) {
-        switch (timeoutDuration) {
-        case ShortTimeout:
-            timeout = m_shortTimeout;
-            break;
-        case LongTimeout:
-            timeout = m_longTimeout;
-            break;
-        case NoTimeout:
-        default:
-            timeout = m_noTimeout;
-            break;
-        }
-    }
+    if (m_forceNoTimeout)
+        timeout = noTimeout;
 
     platformRunUntil(done, timeout);
 }
index f01117e..b738fe5 100644 (file)
@@ -51,6 +51,9 @@ public:
     static const unsigned w3cSVGViewWidth;
     static const unsigned w3cSVGViewHeight;
 
+    static const double shortTimeout;
+    static const double noTimeout;
+
     TestController(int argc, const char* argv[]);
     ~TestController();
 
@@ -68,9 +71,8 @@ public:
     bool shouldUseRemoteLayerTree() const { return m_shouldUseRemoteLayerTree; }
     
     // Runs the run loop until `done` is true or the timeout elapses.
-    enum TimeoutDuration { ShortTimeout, LongTimeout, NoTimeout };
     bool useWaitToDumpWatchdogTimer() { return m_useWaitToDumpWatchdogTimer; }
-    void runUntil(bool& done, TimeoutDuration);
+    void runUntil(bool& done, double timeoutSeconds);
     void notifyDone();
     
     void configureViewForTest(const TestInvocation&);
@@ -223,9 +225,6 @@ private:
     State m_state;
     bool m_doneResetting;
 
-    double m_longTimeout;
-    double m_shortTimeout;
-    double m_noTimeout;
     bool m_useWaitToDumpWatchdogTimer;
     bool m_forceNoTimeout;
 
index 1c9c295..406e0c9 100644 (file)
@@ -160,7 +160,7 @@ void TestInvocation::invoke()
 
     WKContextPostMessageToInjectedBundle(TestController::shared().context(), messageName.get(), beginTestMessageBody.get());
 
-    TestController::shared().runUntil(m_gotInitialResponse, TestController::ShortTimeout);
+    TestController::shared().runUntil(m_gotInitialResponse, TestController::shortTimeout);
     if (!m_gotInitialResponse) {
         m_errorMessage = "Timed out waiting for initial response from web process\n";
         m_webProcessIsUnresponsive = true;
@@ -171,13 +171,7 @@ void TestInvocation::invoke()
 
     WKPageLoadURL(TestController::shared().mainWebView()->page(), m_url.get());
 
-    TestController::shared().runUntil(m_gotFinalMessage, TestController::NoTimeout);
-
-    if (!m_gotFinalMessage) {
-        m_errorMessage = "Timed out waiting for final message from web process\n";
-        m_webProcessIsUnresponsive = true;
-        goto end;
-    }
+    TestController::shared().runUntil(m_gotFinalMessage, TestController::noTimeout);
     if (m_error)
         goto end;
 
@@ -254,7 +248,7 @@ void TestInvocation::dumpResults()
         if (PlatformWebView::windowSnapshotEnabled()) {
             m_gotRepaint = false;
             WKPageForceRepaint(TestController::shared().mainWebView()->page(), this, TestInvocation::forceRepaintDoneCallback);
-            TestController::shared().runUntil(m_gotRepaint, TestController::ShortTimeout);
+            TestController::shared().runUntil(m_gotRepaint, TestController::shortTimeout);
             if (!m_gotRepaint) {
                 m_errorMessage = "Timed out waiting for pre-pixel dump repaint\n";
                 m_webProcessIsUnresponsive = true;
index f7f32f1..3088a51 100644 (file)
@@ -69,7 +69,7 @@ void TestController::platformWillRunTest(const TestInvocation&)
 
 void TestController::platformRunUntil(bool& condition, double timeout)
 {
-    if (timeout == m_noTimeout) {
+    if (timeout <= 0) {
         // Never timeout if we are debugging or not meant to timeout.
         while (!condition)
             ecore_main_loop_iterate();
index 5fc83e2..21fce92 100644 (file)
@@ -57,7 +57,7 @@ void TestController::platformWillRunTest(const TestInvocation&)
 
 void TestController::platformRunUntil(bool&, double timeout)
 {
-    if (timeout != m_noTimeout) {
+    if (timeout > 0) {
         timeoutSource.scheduleAfterDelay("[WTR] Test timeout source", [] {
             fprintf(stderr, "FAIL: TestControllerRunLoop timed out.\n");
             gtk_main_quit();