WebKitTestRunner has "short timeout" and "long timeout" for IPC, plus it has waitUntilDone timeout, plus run-webkit-tests itself has a timeout. There are two reasons why WebKitTestRunner IPC timeouts are helpful: 1. If WebProcess freezes between tests, we want to detect that, and not attribute the freeze to the next test. Expecting a "start test" message to be handled quickly achieves this. 2. If WebProcess is so broken that it fails to launch, it's nice to say so, and not just fail with a generic timeout. Both are handled by short timeout. Long timeout is only used when loading about:blank between tests, which I don't quite see the logic behind.
Created attachment 243875 [details] proposed patch
Created attachment 243882 [details] proposed patch With Efl and Gtk build fixes.
(In reply to comment #2) > Created attachment 243882 [details] > proposed patch > > With Efl and Gtk build fixes. ./../Tools/WebKitTestRunner/TestController.cpp:73:45: error: missing binary operator before token "(" #if defined (__has_feature) && __has_feature(address_sanitizer) ^ Unfortunately GCC doesn't support short circuit evaluation, so we need to use nested ifs for has_feature and has_include.
It's strange how GTK built it. I thought GTK uses GCC 4.8 too.
Created attachment 243895 [details] proposed patch OK, replaced with an ugly nested #if for now. I'm looking into adding a macro to WTF, but need to figure out how an existing ADDRESS_SANITIZER macro is used.
Comment on attachment 243895 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=243895&action=review > Tools/WebKitTestRunner/TestController.cpp:78 > +#else Missing #endif before this #else.
Created attachment 243901 [details] proposed patch
Comment on attachment 243901 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=243901&action=review I think it’s a little strange to compile in this 10 second vs. 5 second default timeout. > Tools/WebKitTestRunner/TestController.cpp:81 > +#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 I would have written: #if defined(__has_feature) && __has_feature(address_sanitizer) const double TestController::shortTimeout = 10; #else const double TestController::shortTimeout = 5; #endif But maybe the nested syntax is needed to compile on compilers that don't have __has_feature? > Tools/WebKitTestRunner/TestController.h:56 > + constexpr static double noTimeout = -1; Does this work on Windows? I had problems with constexpr on Windows.
Comment on attachment 243901 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=243901&action=review >> Tools/WebKitTestRunner/TestController.cpp:81 >> +#endif > > I would have written: > > #if defined(__has_feature) && __has_feature(address_sanitizer) > const double TestController::shortTimeout = 10; > #else > const double TestController::shortTimeout = 5; > #endif > > But maybe the nested syntax is needed to compile on compilers that don't have __has_feature? Oops, I see you discussing this earlier in this bug!
Committed <http://trac.webkit.org/r177870>. Removed the use of constexpr to be safe, as Windows EWS is not functional.