Changeset 177598 in webkit

Timestamp:

Dec 19, 2014, 3:25:46 PM (10 years ago)

Author:

ap@apple.com

Message:

REGRESSION (177368): Some tests started to immediately time out
​https://bugs.webkit.org/show_bug.cgi?id=139826

Reviewed by Daniel Bates.

Tools:

• WebKitTestRunner/InjectedBundle/InjectedBundle.cpp: (WTR::InjectedBundle::done):
• WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp: (WTR::InjectedBundlePage::dump):

Move invalidateWaitToDumpWatchdogTimer() call from dump() to done(). When a test
ends in a load failure, we don't dump, so we used to leak the timer.

• WebKitTestRunner/InjectedBundle/TestRunner.cpp: (WTR::TestRunner::notifyDone):

Added a comment.

LayoutTests:

• platform/wk2/TestExpectations: The culprit was this test that I unskipped to see

whether it passes. It in fact does pass, so let's remove the expectation entirely.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/wk2/TestExpectations (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp (modified) (1 diff)
• Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp (modified) (1 diff)
• Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2014-12-19  Alexey Proskuryakov  <ap@apple.com>
+
+        REGRESSION (177368): Some tests started to immediately time out
+        https://bugs.webkit.org/show_bug.cgi?id=139826
+
+        Reviewed by Daniel Bates.
+
+        * platform/wk2/TestExpectations: The culprit was this test that I unskipped to see
+        whether it passes. It in fact does pass, so let's remove the expectation entirely.
+
 2014-12-19  Brent Fulgham  <bfulgham@apple.com>
 

trunk/LayoutTests/platform/wk2/TestExpectations

@@ -76 +76 @@
 webkit.org/b/82665 webarchive/test-css-url-resources-in-stylesheets.html [ Skip ]
 webkit.org/b/82665 webarchive/test-css-url-resources-inline-styles.html [ Skip ]
-
-# This test has been skipped for a long time. Temporarily marking as flaky to see what bots think.
-webkit.org/b/96741 fast/loader/recursive-before-unload-crash.html [ Pass Failure ]
 
 # WebKitTestRunner needs textInputController

trunk/Tools/ChangeLog

@@ +1 @@
+2014-12-19  Alexey Proskuryakov  <ap@apple.com>
+
+        REGRESSION (177368): Some tests started to immediately time out
+        https://bugs.webkit.org/show_bug.cgi?id=139826
+
+        Reviewed by Daniel Bates.
+
+        * WebKitTestRunner/InjectedBundle/InjectedBundle.cpp: (WTR::InjectedBundle::done):
+        * WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp: (WTR::InjectedBundlePage::dump):
+        Move invalidateWaitToDumpWatchdogTimer() call from dump() to done(). When a test
+        ends in a load failure, we don't dump, so we used to leak the timer.
+        
+        * WebKitTestRunner/InjectedBundle/TestRunner.cpp: (WTR::TestRunner::notifyDone):
+        Added a comment.
+
 2014-12-19  Brent Fulgham  <bfulgham@apple.com>
 

trunk/Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp

@@ -290 +290 @@
     setTopLoadingFrame(0);
 
+    m_testRunner->invalidateWaitToDumpWatchdogTimer();
+
     m_accessibilityController->resetToConsistentState();
 

trunk/Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp

@@ -837 +837 @@
     ASSERT(InjectedBundle::shared().isTestRunning());
 
-    InjectedBundle::shared().testRunner()->invalidateWaitToDumpWatchdogTimer();
-
     // Force a paint before dumping. This matches DumpRenderTree on Windows. (DumpRenderTree on Mac
     // does this at a slightly different time.) See <http://webkit.org/b/55469> for details.

trunk/Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp

@@ -161 +161 @@
         InjectedBundle::shared().page()->dump();
 
+    // We don't call invalidateWaitToDumpWatchdogTimer() here, even if we continue to wait for a load to finish.
+    // The test is still subject to timeout checking - it is better to detect an async timeout inside WebKitTestRunner
+    // than to let webkitpy do that, because WebKitTestRunner will dump partial results.
+
     m_waitToDump = false;
 }