Changeset 206596 in webkit

Timestamp:

Sep 29, 2016, 11:44:54 AM (10 years ago)

Author:

ap@apple.com

Message:

Make WKTR short timeout dynamic
​https://bugs.webkit.org/show_bug.cgi?id=162733

Reviewed by Alex Christensen.

WebKitTestRunner has a hardcoded value for how long to wait for IPC responses,
which is 5 seconds, or 10 seconds under ASan. But some of the operations can be
fairly time consuming - e.g. launching Networking process on demand.
These may take longer under load, especially with GuardMalloc.

• WebKitTestRunner/TestController.cpp:

(WTR::TestController::resetStateToConsistentValues):
(WTR::TestController::reattachPageToWebProcess):

• WebKitTestRunner/TestController.h:
• WebKitTestRunner/TestInvocation.cpp:

(WTR::TestInvocation::shortTimeout):
(WTR::TestInvocation::invoke):
(WTR::TestInvocation::dumpResults):

• WebKitTestRunner/TestInvocation.h:

(WTR::TestInvocation::customTimeout): Deleted, this function was unused.

Location:

trunk/Tools

Files:

• ChangeLog (modified) (1 diff)
• WebKitTestRunner/TestController.cpp (modified) (3 diffs)
• WebKitTestRunner/TestController.h (modified) (1 diff)
• WebKitTestRunner/TestInvocation.cpp (modified) (3 diffs)
• WebKitTestRunner/TestInvocation.h (modified) (1 diff)

trunk/Tools/ChangeLog

@@ +1 @@
+2016-09-29  Alexey Proskuryakov  <ap@apple.com>
+
+        Make WKTR short timeout dynamic
+        https://bugs.webkit.org/show_bug.cgi?id=162733
+
+        Reviewed by Alex Christensen.
+
+        WebKitTestRunner has a hardcoded value for how long to wait for IPC responses,
+        which is 5 seconds, or 10 seconds under ASan. But some of the operations can be
+        fairly time consuming - e.g. launching Networking process on demand.
+        These may take longer under load, especially with GuardMalloc.
+
+        * WebKitTestRunner/TestController.cpp:
+        (WTR::TestController::resetStateToConsistentValues):
+        (WTR::TestController::reattachPageToWebProcess):
+        * WebKitTestRunner/TestController.h:
+        * WebKitTestRunner/TestInvocation.cpp:
+        (WTR::TestInvocation::shortTimeout):
+        (WTR::TestInvocation::invoke):
+        (WTR::TestInvocation::dumpResults):
+
+        * WebKitTestRunner/TestInvocation.h:
+        (WTR::TestInvocation::customTimeout): Deleted, this function was unused.
+
 2016-09-29  Alex Christensen  <achristensen@webkit.org>
 

trunk/Tools/WebKitTestRunner/TestController.cpp

@@ -90 +90 @@
 const unsigned TestController::w3cSVGViewHeight = 360;
 
-#if ASAN_ENABLED
-const double TestController::shortTimeout = 10.0;
-#else
-const double TestController::shortTimeout = 5.0;
-#endif
+const double TestController::defaultShortTimeout = 5.0;
 
 const double TestController::noTimeout = -1;
@@ -798 +794 @@
 
     WKPageLoadURL(m_mainWebView->page(), blankURL());
-    runUntil(m_doneResetting, shortTimeout);
+    runUntil(m_doneResetting, m_currentInvocation->shortTimeout());
     return m_doneResetting;
 }
@@ -812 +808 @@
     m_doneResetting = false;
     WKPageLoadURL(m_mainWebView->page(), blankURL());
-    runUntil(m_doneResetting, shortTimeout);
+    runUntil(m_doneResetting, m_currentInvocation->shortTimeout());
 }
 

trunk/Tools/WebKitTestRunner/TestController.h

@@ -58 +58 @@
     static const unsigned w3cSVGViewHeight;
 
-    static const double shortTimeout;
+    static const double defaultShortTimeout;
     static const double noTimeout;
 

trunk/Tools/WebKitTestRunner/TestInvocation.cpp

@@ -99 +99 @@
 }
 
+double TestInvocation::shortTimeout() const
+{
+    if (!m_timeout) {
+        // Running WKTR directly, without webkitpy.
+        return TestController::defaultShortTimeout;
+    }
+
+    // This is not exactly correct for the way short timeout is used - it should not depend on whether a test is "slow",
+    // but it currently does. There is no way to know what a normal test's timeout is, as webkitpy only passes timeouts
+    // for each test individually.
+    // But there shouldn't be any observable negative consequences from this.
+    return m_timeout / 1000. / 2;
+}
+
 bool TestInvocation::shouldLogFrameLoadDelegates() const
 {
@@ -150 +164 @@
     bool shouldOpenExternalURLs = false;
 
-    TestController::singleton().runUntil(m_gotInitialResponse, TestController::shortTimeout);
+    TestController::singleton().runUntil(m_gotInitialResponse, shortTimeout());
     if (!m_gotInitialResponse) {
         m_errorMessage = "Timed out waiting for initial response from web process\n";
@@ -254 +268 @@
             m_gotRepaint = false;
             WKPageForceRepaint(TestController::singleton().mainWebView()->page(), this, TestInvocation::forceRepaintDoneCallback);
-            TestController::singleton().runUntil(m_gotRepaint, TestController::shortTimeout);
+            TestController::singleton().runUntil(m_gotRepaint, shortTimeout());
             if (!m_gotRepaint) {
                 m_errorMessage = "Timed out waiting for pre-pixel dump repaint\n";

trunk/Tools/WebKitTestRunner/TestInvocation.h

@@ -50 +50 @@
     void setIsPixelTest(const std::string& expectedPixelHash);
 
+    // Milliseconds
     void setCustomTimeout(int duration) { m_timeout = duration; }
-    int customTimeout() const { return m_timeout; }
+
+    // Seconds
+    double shortTimeout() const;
 
     void invoke();