1. Make messageloop-based timeout work on Mac/Linux,

2. Save the results of timing out tests and show them in the results.html file.

One arguably negative side effect of this is that a test that times out and has
image/text failures will count as both a timeout failure and an image/text failure.

I'm not 100% certain the RevokeAll change makes sense. It matches what the old
code's comments claimed it did, but the code didn't actually do. The idea is
that if the test finishes before the timeout is hit, we should cancel the timeout.
Review URL: http://codereview.chromium.org/303007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@29576 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 54 insertions, 33 deletions

diff --git a/webkit/tools/layout_tests/layout_package/test_shell_thread.py b/webkit/tools/layout_tests/layout_package/test_shell_thread.py
index c6bf2d3..e3ffee8 100644
--- a/webkit/tools/layout_tests/layout_package/test_shell_thread.py
+++ b/webkit/tools/layout_tests/layout_package/test_shell_thread.py
@@ -39,7 +39,7 @@ def ProcessOutput(proc, test_info, test_types, test_args, target):
   """
   outlines = []
   failures = []
-  crash_or_timeout = False
+  crash = False
 
   # Some test args, such as the image hash, may be added or changed on a
   # test-by-test basis.
@@ -63,7 +63,7 @@ def ProcessOutput(proc, test_info, test_types, test_args, target):
       if (-1073741510 == proc.returncode or
           -signal.SIGINT == proc.returncode):
         raise KeyboardInterrupt
-      crash_or_timeout = True
+      crash = True
       break
 
     # Don't include #URL lines in our output
@@ -78,7 +78,6 @@ def ProcessOutput(proc, test_info, test_types, test_args, target):
       local_test_args.hash = line.rstrip()[5:]
     elif line.startswith("#TEST_TIMED_OUT"):
       # Test timed out, but we still need to read until #EOF.
-      crash_or_timeout = True
       failures.append(test_failures.FailureTimeout())
     else:
       outlines.append(line)
@@ -95,9 +94,10 @@ def ProcessOutput(proc, test_info, test_types, test_args, target):
                                            ''.join(outlines),
                                            local_test_args,
                                            target)
-    # Don't add any more failures if we already have a crash or timeout, so
-    # we don't double-report those tests.
-    if not crash_or_timeout:
+    # Don't add any more failures if we already have a crash, so we don't
+    # double-report those tests. We do double-report for timeouts since we still
+    # want to see the text and image output.
+    if not crash:
       failures.extend(new_failures)
     time_for_diffs[test_type.__class__.__name__] = (
         time.time() - start_diff_time)
diff --git a/webkit/tools/test_shell/layout_test_controller.cc b/webkit/tools/test_shell/layout_test_controller.cc
index cac9065..48ec6f0 100644
--- a/webkit/tools/test_shell/layout_test_controller.cc
+++ b/webkit/tools/test_shell/layout_test_controller.cc
@@ -34,20 +34,6 @@ using WebKit::WebConsoleMessage;
 using WebKit::WebScriptSource;
 using WebKit::WebString;
 
-#if defined(OS_WIN)
-namespace {
-
-// Stops the test from running and prints a brief warning to stdout.  Called
-// when the timer for loading a layout test expires.
-VOID CALLBACK TestTimeout(HWND hwnd, UINT msg, UINT_PTR timer_id, DWORD ms) {
-  puts("#TEST_TIMED_OUT\n");
-  reinterpret_cast<TestShell*>(timer_id)->TestFinished();
-  // Print a warning to be caught by the layout-test script.
-}
-
-}
-#endif
-
 TestShell* LayoutTestController::shell_ = NULL;
 // Most of these flags need to be cleared in Reset() so that they get turned
 // off between each test run.
@@ -70,7 +56,8 @@ LayoutTestController::WorkQueue LayoutTestController::work_queue_;
 CppVariant LayoutTestController::globalFlag_;
 CppVariant LayoutTestController::webHistoryItemCount_;
 
-LayoutTestController::LayoutTestController(TestShell* shell) {
+LayoutTestController::LayoutTestController(TestShell* shell) :
+    ALLOW_THIS_IN_INITIALIZER_LIST(timeout_factory_(this)) {
   // Set static shell_ variable since we can't do it in an initializer list.
   // We also need to be careful not to assign shell_ to new windows which are
   // temporary.
@@ -271,30 +258,47 @@ void LayoutTestController::setAcceptsEditing(
 
 void LayoutTestController::waitUntilDone(
     const CppArgumentList& args, CppVariant* result) {
+  bool is_debugger_present = false;
 #if defined(OS_WIN)
-  // Set a timer in case something hangs.  We use a custom timer rather than
-  // the one managed by the message loop so we can kill it when the load
-  // finishes successfully.
-  if (!::IsDebuggerPresent()) {
-    UINT_PTR timer_id = reinterpret_cast<UINT_PTR>(shell_);
-    SetTimer(shell_->mainWnd(), timer_id, shell_->GetLayoutTestTimeout(),
-             &TestTimeout);
-  }
-#else
-  // TODO(port): implement timer here
+  // TODO(ojan): Make cross-platform.
+  is_debugger_present = ::IsDebuggerPresent();
 #endif
+
+  if (!is_debugger_present) {
+    // TODO(ojan): Use base::OneShotTimer. For some reason, using OneShotTimer
+    // seems to cause layout test failures on the try bots.
+    MessageLoop::current()->PostDelayedTask(FROM_HERE,
+        timeout_factory_.NewRunnableMethod(
+            &LayoutTestController::notifyDoneTimedOut),
+        shell_->GetLayoutTestTimeout());
+  }
+
   wait_until_done_ = true;
   result->SetNull();
 }
 
 void LayoutTestController::notifyDone(
     const CppArgumentList& args, CppVariant* result) {
+  // Test didn't timeout. Kill the timeout timer.
+  timeout_factory_.RevokeAll();
+
+  completeNotifyDone(false);
+  result->SetNull();
+}
+
+void LayoutTestController::notifyDoneTimedOut() {
+  completeNotifyDone(true);
+}
+
+void LayoutTestController::completeNotifyDone(bool is_timeout) {
   if (shell_->layout_test_mode() && wait_until_done_ &&
       !shell_->delegate()->top_loading_frame() && work_queue_.empty()) {
-    shell_->TestFinished();
+    if (is_timeout)
+      shell_->TestTimedOut();
+    else
+      shell_->TestFinished();
   }
   wait_until_done_ = false;
-  result->SetNull();
 }
 
 class WorkItemBackForward : public LayoutTestController::WorkItem {
diff --git a/webkit/tools/test_shell/layout_test_controller.h b/webkit/tools/test_shell/layout_test_controller.h
index 5aafc85..7f9f628 100644
--- a/webkit/tools/test_shell/layout_test_controller.h
+++ b/webkit/tools/test_shell/layout_test_controller.h
@@ -74,6 +74,7 @@ class LayoutTestController : public CppBoundClass {
   // to delay the completion of the test until notifyDone is called.
   void waitUntilDone(const CppArgumentList& args, CppVariant* result);
   void notifyDone(const CppArgumentList& args, CppVariant* result);
+  void notifyDoneTimedOut();
 
   // Methods for adding actions to the work queue.  Used in conjunction with
   // waitUntilDone/notifyDone above.
@@ -290,6 +291,12 @@ class LayoutTestController : public CppBoundClass {
 
   void LogErrorToConsole(const std::string& text);
 
+  void completeNotifyDone(bool is_timeout);
+
+  // Used for test timeouts.
+  // TODO(ojan): Use base::OneShotTimer.
+  ScopedRunnableMethodFactory<LayoutTestController> timeout_factory_;
+
   // Non-owning pointer.  The LayoutTestController is owned by the host.
   static TestShell* shell_;
 
diff --git a/webkit/tools/test_shell/test_shell.cc b/webkit/tools/test_shell/test_shell.cc
index 97c2eba..e208eaf 100644
--- a/webkit/tools/test_shell/test_shell.cc
+++ b/webkit/tools/test_shell/test_shell.cc
@@ -453,6 +453,11 @@ bool TestShell::RemoveWindowFromList(gfx::NativeWindow window) {
   return false;
 }
 
+void TestShell::TestTimedOut() {
+  puts("#TEST_TIMED_OUT\n");
+  TestFinished();
+}
+
 void TestShell::Show(WebNavigationPolicy policy) {
   delegate_->show(policy);
 }
diff --git a/webkit/tools/test_shell/test_shell.h b/webkit/tools/test_shell/test_shell.h
index 9ff142b..6f1e06c 100644
--- a/webkit/tools/test_shell/test_shell.h
+++ b/webkit/tools/test_shell/test_shell.h
@@ -106,6 +106,11 @@ public:
     // Called by the LayoutTestController to signal test completion.
     void TestFinished();
 
+    // Called by LayoutTestController when a test hits the timeout, but does not
+    // cause a hang. We can avoid killing TestShell in this case and still dump
+    // the test results.
+    void TestTimedOut();
+
     // Called to block the calling thread until TestFinished is called.
     void WaitTestFinished();