WebKit Bugzilla
Attachment 368367 Details for
Bug 197333
: [WKTR] Move test timeout handling to the UIProcess
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-197333-20190426165305.patch (text/plain), 17.81 KB, created by
Chris Dumez
on 2019-04-26 16:53:05 PDT
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Chris Dumez
Created:
2019-04-26 16:53:05 PDT
Size:
17.81 KB
patch
obsolete
>Subversion Revision: 244685 >diff --git a/Tools/ChangeLog b/Tools/ChangeLog >index 0de37da93985c630f606c2b53eb6427bd831fd93..eeab3ec3fa422e7881eaec0316d05390de925fd1 100644 >--- a/Tools/ChangeLog >+++ b/Tools/ChangeLog >@@ -1,3 +1,43 @@ >+2019-04-26 Chris Dumez <cdumez@apple.com> >+ >+ [WKTR] Move test timeout handling to the UIProcess >+ https://bugs.webkit.org/show_bug.cgi?id=197333 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ Move test timeout handling in WebKitTestRunner to the UIProcess to play nicely with PSON. Previously, >+ we'd start the timeout timer in the InjectedBundle, which would fail to account of the time spent in >+ every WebContent process in the case of swapping. >+ >+ Also, because of process caching, the timeout timer would sometime fire in a cached process and it >+ would lead to crashes when firing the timer. >+ >+ * WebKitTestRunner/InjectedBundle/InjectedBundle.cpp: >+ (WTR::InjectedBundle::didReceiveMessageToPage): >+ (WTR::InjectedBundle::done): >+ * WebKitTestRunner/InjectedBundle/InjectedBundle.h: >+ (WTR::InjectedBundle::shouldDumpPixels const): >+ * WebKitTestRunner/InjectedBundle/TestRunner.cpp: >+ (WTR::TestRunner::TestRunner): >+ (WTR::TestRunner::waitUntilDone): >+ (WTR::TestRunner::setWaitUntilDone): >+ * WebKitTestRunner/InjectedBundle/TestRunner.h: >+ * WebKitTestRunner/InjectedBundle/gtk/TestRunnerGtk.cpp: >+ * WebKitTestRunner/InjectedBundle/mac/TestRunnerMac.mm: >+ * WebKitTestRunner/InjectedBundle/win/TestRunnerWin.cpp: >+ * WebKitTestRunner/InjectedBundle/wpe/TestRunnerWPE.cpp: >+ * WebKitTestRunner/TestInvocation.cpp: >+ (WTR::TestInvocation::TestInvocation): >+ (WTR::TestInvocation::createTestSettingsDictionary): >+ (WTR::TestInvocation::didReceiveMessageFromInjectedBundle): >+ (WTR::TestInvocation::didReceiveSynchronousMessageFromInjectedBundle): >+ (WTR::TestInvocation::initializeWaitToDumpWatchdogTimerIfNeeded): >+ (WTR::TestInvocation::invalidateWaitToDumpWatchdogTimer): >+ (WTR::TestInvocation::waitToDumpWatchdogTimerFired): >+ (WTR::TestInvocation::setWaitUntilDone): >+ (WTR::TestInvocation::done): >+ * WebKitTestRunner/TestInvocation.h: >+ > 2019-04-25 Simon Fraser <simon.fraser@apple.com> > > REGRESSION (r234330): 3 legacy-animation-engine/compositing tests are flaky failures >diff --git a/Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp b/Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp >index b8397cf6e35553b6123e5e53a062012c0d253223..31d3b970eace6e2850293e4e2e69740bf91d73ae 100644 >--- a/Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp >+++ b/Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp >@@ -208,9 +208,6 @@ void InjectedBundle::didReceiveMessageToPage(WKBundlePageRef page, WKStringRef m > WKRetainPtr<WKStringRef> dumpPixelsKey = adoptWK(WKStringCreateWithUTF8CString("DumpPixels")); > m_dumpPixels = WKBooleanGetValue(static_cast<WKBooleanRef>(WKDictionaryGetItemForKey(messageBodyDictionary, dumpPixelsKey.get()))); > >- WKRetainPtr<WKStringRef> useWaitToDumpWatchdogTimerKey = adoptWK(WKStringCreateWithUTF8CString("UseWaitToDumpWatchdogTimer")); >- m_useWaitToDumpWatchdogTimer = WKBooleanGetValue(static_cast<WKBooleanRef>(WKDictionaryGetItemForKey(messageBodyDictionary, useWaitToDumpWatchdogTimerKey.get()))); >- > WKRetainPtr<WKStringRef> timeoutKey = adoptWK(WKStringCreateWithUTF8CString("Timeout")); > m_timeout = Seconds::fromMilliseconds(WKUInt64GetValue(static_cast<WKUInt64Ref>(WKDictionaryGetItemForKey(messageBodyDictionary, timeoutKey.get())))); > >@@ -543,8 +540,6 @@ void InjectedBundle::done() > page()->stopLoading(); > setTopLoadingFrame(0); > >- m_testRunner->invalidateWaitToDumpWatchdogTimer(); >- > #if HAVE(ACCESSIBILITY) > m_accessibilityController->resetToConsistentState(); > #endif >diff --git a/Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.h b/Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.h >index e387e3d2cc2f3c7505f40c6b968b91a50fcf74ef..26927d21a47073f2b880ed4cd168d7ac3148d990 100644 >--- a/Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.h >+++ b/Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.h >@@ -80,7 +80,6 @@ public: > void setTopLoadingFrame(WKBundleFrameRef frame) { m_topLoadingFrame = frame; } > > bool shouldDumpPixels() const { return m_dumpPixels; } >- bool useWaitToDumpWatchdogTimer() const { return m_useWaitToDumpWatchdogTimer; } > bool dumpJSConsoleLogInStdErr() const { return m_dumpJSConsoleLogInStdErr; }; > > void outputText(const String&); >@@ -194,7 +193,6 @@ private: > State m_state { Idle }; > > bool m_dumpPixels { false }; >- bool m_useWaitToDumpWatchdogTimer { true }; > bool m_useWorkQueue { false }; > bool m_pixelResultIsPending { false }; > bool m_dumpJSConsoleLogInStdErr { false }; >diff --git a/Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp b/Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp >index e9cc27c236491bbf2d34ef2a250db66dd2b11576..04eb70cf97414997d13abc5f6f5b07b591151d7f 100644 >--- a/Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp >+++ b/Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp >@@ -66,9 +66,6 @@ Ref<TestRunner> TestRunner::create() > > TestRunner::TestRunner() > : m_userStyleSheetLocation(adoptWK(WKStringCreateWithUTF8CString(""))) >-#if !PLATFORM(COCOA) >- , m_waitToDumpWatchdogTimer(RunLoop::main(), this, &TestRunner::waitToDumpWatchdogTimerFired) >-#endif > { > platformInitialize(); > } >@@ -161,16 +158,13 @@ void TestRunner::waitUntilDone() > RELEASE_ASSERT(injectedBundle.isTestRunning()); > > setWaitUntilDone(true); >- // FIXME: Watchdog timer should be moved to UI process in order to take the elapsed time in anotehr process into account. >- if (injectedBundle.useWaitToDumpWatchdogTimer()) >- initializeWaitToDumpWatchdogTimerIfNeeded(); > } > > void TestRunner::setWaitUntilDone(bool value) > { >- WKRetainPtr<WKStringRef> messsageName = adoptWK(WKStringCreateWithUTF8CString("SetWaitUntilDone")); >+ WKRetainPtr<WKStringRef> messageName = adoptWK(WKStringCreateWithUTF8CString("SetWaitUntilDone")); > WKRetainPtr<WKBooleanRef> messageBody = adoptWK(WKBooleanCreate(value)); >- WKBundlePostSynchronousMessage(InjectedBundle::singleton().bundle(), messsageName.get(), messageBody.get(), nullptr); >+ WKBundlePostSynchronousMessage(InjectedBundle::singleton().bundle(), messageName.get(), messageBody.get(), nullptr); > } > > bool TestRunner::shouldWaitUntilDone() const >@@ -182,19 +176,6 @@ bool TestRunner::shouldWaitUntilDone() const > return WKBooleanGetValue(adoptWK(static_cast<WKBooleanRef>(returnData)).get()); > } > >-void TestRunner::waitToDumpWatchdogTimerFired() >-{ >- invalidateWaitToDumpWatchdogTimer(); >- auto& injectedBundle = InjectedBundle::singleton(); >-#if PLATFORM(COCOA) >- char buffer[1024]; >- snprintf(buffer, sizeof(buffer), "#PID UNRESPONSIVE - %s (pid %d)\n", getprogname(), getpid()); >- injectedBundle.outputText(buffer); >-#endif >- injectedBundle.outputText("FAIL: Timed out waiting for notifyDone to be called\n\n"); >- injectedBundle.done(); >-} >- > void TestRunner::notifyDone() > { > auto& injectedBundle = InjectedBundle::singleton(); >diff --git a/Tools/WebKitTestRunner/InjectedBundle/TestRunner.h b/Tools/WebKitTestRunner/InjectedBundle/TestRunner.h >index c72be1804a6cfa53d9460b35fee149c349729228..5ee6b090616c5b9fdc7f9106e6558cb884fe1e2f 100644 >--- a/Tools/WebKitTestRunner/InjectedBundle/TestRunner.h >+++ b/Tools/WebKitTestRunner/InjectedBundle/TestRunner.h >@@ -36,18 +36,6 @@ > #include <wtf/Seconds.h> > #include <wtf/text/WTFString.h> > >-#if PLATFORM(COCOA) >-#include <wtf/RetainPtr.h> >-#include <CoreFoundation/CFRunLoop.h> >-typedef RetainPtr<CFRunLoopTimerRef> PlatformTimerRef; >-#else >-#include <wtf/RunLoop.h> >-namespace WTR { >-class TestRunner; >-typedef RunLoop::Timer<TestRunner> PlatformTimerRef; >-} >-#endif >- > namespace WTR { > > class TestRunner : public JSWrappable { >@@ -233,8 +221,6 @@ public: > void clearDidReceiveServerRedirectForProvisionalNavigation(); > > bool shouldWaitUntilDone() const; >- void waitToDumpWatchdogTimerFired(); >- void invalidateWaitToDumpWatchdogTimer(); > > // Downloads > bool shouldFinishAfterDownload() const { return m_shouldFinishAfterDownload; } >@@ -512,7 +498,6 @@ private: > TestRunner(); > > void platformInitialize(); >- void initializeWaitToDumpWatchdogTimerIfNeeded(); > > void setDumpPixels(bool); > void setWaitUntilDone(bool); >@@ -527,8 +512,6 @@ private: > WKRetainPtr<WKStringRef> m_userStyleSheetLocation; > WKRetainPtr<WKArrayRef> m_allowedHosts; > >- PlatformTimerRef m_waitToDumpWatchdogTimer; >- > double m_databaseDefaultQuota { -1 }; > double m_databaseMaxQuota { -1 }; > >diff --git a/Tools/WebKitTestRunner/InjectedBundle/gtk/TestRunnerGtk.cpp b/Tools/WebKitTestRunner/InjectedBundle/gtk/TestRunnerGtk.cpp >index 10c999f45858c5b7e8911f29ec9ad83f93c826ef..81b21588b85ccd0f0693b536865449bbf493af46 100644 >--- a/Tools/WebKitTestRunner/InjectedBundle/gtk/TestRunnerGtk.cpp >+++ b/Tools/WebKitTestRunner/InjectedBundle/gtk/TestRunnerGtk.cpp >@@ -39,19 +39,6 @@ void TestRunner::platformInitialize() > { > } > >-void TestRunner::invalidateWaitToDumpWatchdogTimer() >-{ >- m_waitToDumpWatchdogTimer.stop(); >-} >- >-void TestRunner::initializeWaitToDumpWatchdogTimerIfNeeded() >-{ >- if (m_waitToDumpWatchdogTimer.isActive()) >- return; >- >- m_waitToDumpWatchdogTimer.startOneShot(m_timeout); >-} >- > JSRetainPtr<JSStringRef> TestRunner::pathToLocalResource(JSStringRef url) > { > size_t urlSize = JSStringGetMaximumUTF8CStringSize(url); >diff --git a/Tools/WebKitTestRunner/InjectedBundle/mac/TestRunnerMac.mm b/Tools/WebKitTestRunner/InjectedBundle/mac/TestRunnerMac.mm >index a3be678a520df32a22b59ecba70209556e25d4c2..0486dc866879880e101f5a9dd5c60c8095880e9e 100644 >--- a/Tools/WebKitTestRunner/InjectedBundle/mac/TestRunnerMac.mm >+++ b/Tools/WebKitTestRunner/InjectedBundle/mac/TestRunnerMac.mm >@@ -36,30 +36,6 @@ void TestRunner::platformInitialize() > { > } > >-void TestRunner::invalidateWaitToDumpWatchdogTimer() >-{ >- if (!m_waitToDumpWatchdogTimer) >- return; >- >- CFRunLoopTimerInvalidate(m_waitToDumpWatchdogTimer.get()); >- m_waitToDumpWatchdogTimer = nullptr; >-} >- >-static void waitUntilDoneWatchdogTimerFired(CFRunLoopTimerRef timer, void* info) >-{ >- InjectedBundle::singleton().testRunner()->waitToDumpWatchdogTimerFired(); >-} >- >-void TestRunner::initializeWaitToDumpWatchdogTimerIfNeeded() >-{ >- if (m_waitToDumpWatchdogTimer) >- return; >- >- CFTimeInterval interval = m_timeout.seconds(); >- m_waitToDumpWatchdogTimer = adoptCF(CFRunLoopTimerCreate(kCFAllocatorDefault, CFAbsoluteTimeGetCurrent() + interval, 0, 0, 0, WTR::waitUntilDoneWatchdogTimerFired, NULL)); >- CFRunLoopAddTimer(CFRunLoopGetCurrent(), m_waitToDumpWatchdogTimer.get(), kCFRunLoopCommonModes); >-} >- > JSRetainPtr<JSStringRef> TestRunner::pathToLocalResource(JSStringRef url) > { > return JSStringRetain(url); // Do nothing on mac. >diff --git a/Tools/WebKitTestRunner/InjectedBundle/win/TestRunnerWin.cpp b/Tools/WebKitTestRunner/InjectedBundle/win/TestRunnerWin.cpp >index 25fbff5afab110ed7d7498097b4557ebe2cec164..793b59b8250abf9aa5531dc6cb3879f88332258d 100644 >--- a/Tools/WebKitTestRunner/InjectedBundle/win/TestRunnerWin.cpp >+++ b/Tools/WebKitTestRunner/InjectedBundle/win/TestRunnerWin.cpp >@@ -40,21 +40,10 @@ JSRetainPtr<JSStringRef> TestRunner::inspectorTestStubURL() > return JSStringCreateWithUTF8CString(""); > } > >-void TestRunner::invalidateWaitToDumpWatchdogTimer() >-{ >- m_waitToDumpWatchdogTimer.stop(); >-} >- > void TestRunner::platformInitialize() > { > } > >-void TestRunner::initializeWaitToDumpWatchdogTimerIfNeeded() >-{ >- if (!m_waitToDumpWatchdogTimer.isActive()) >- m_waitToDumpWatchdogTimer.startOneShot(m_timeout); >-} >- > void TestRunner::installFakeHelvetica(JSStringRef configuration) > { > WTR::installFakeHelvetica(toWK(configuration).get()); >diff --git a/Tools/WebKitTestRunner/InjectedBundle/wpe/TestRunnerWPE.cpp b/Tools/WebKitTestRunner/InjectedBundle/wpe/TestRunnerWPE.cpp >index 25fbff5afab110ed7d7498097b4557ebe2cec164..793b59b8250abf9aa5531dc6cb3879f88332258d 100644 >--- a/Tools/WebKitTestRunner/InjectedBundle/wpe/TestRunnerWPE.cpp >+++ b/Tools/WebKitTestRunner/InjectedBundle/wpe/TestRunnerWPE.cpp >@@ -40,21 +40,10 @@ JSRetainPtr<JSStringRef> TestRunner::inspectorTestStubURL() > return JSStringCreateWithUTF8CString(""); > } > >-void TestRunner::invalidateWaitToDumpWatchdogTimer() >-{ >- m_waitToDumpWatchdogTimer.stop(); >-} >- > void TestRunner::platformInitialize() > { > } > >-void TestRunner::initializeWaitToDumpWatchdogTimerIfNeeded() >-{ >- if (!m_waitToDumpWatchdogTimer.isActive()) >- m_waitToDumpWatchdogTimer.startOneShot(m_timeout); >-} >- > void TestRunner::installFakeHelvetica(JSStringRef configuration) > { > WTR::installFakeHelvetica(toWK(configuration).get()); >diff --git a/Tools/WebKitTestRunner/TestInvocation.cpp b/Tools/WebKitTestRunner/TestInvocation.cpp >index 6ffab3aaa1d5c9a978d376b018d58c1ddc6bf09b..5c96e832a6d5e93f4175a6482ca9891ab3fe58ce 100644 >--- a/Tools/WebKitTestRunner/TestInvocation.cpp >+++ b/Tools/WebKitTestRunner/TestInvocation.cpp >@@ -69,6 +69,7 @@ namespace WTR { > TestInvocation::TestInvocation(WKURLRef url, const TestOptions& options) > : m_options(options) > , m_url(url) >+ , m_waitToDumpWatchdogTimer(RunLoop::main(), this, &TestInvocation::waitToDumpWatchdogTimerFired) > { > WKRetainPtr<WKStringRef> urlString = adoptWK(WKURLCopyString(m_url.get())); > >@@ -138,10 +139,6 @@ WKRetainPtr<WKMutableDictionaryRef> TestInvocation::createTestSettingsDictionary > WKRetainPtr<WKBooleanRef> dumpPixelsValue = adoptWK(WKBooleanCreate(m_dumpPixels)); > WKDictionarySetItem(beginTestMessageBody.get(), dumpPixelsKey.get(), dumpPixelsValue.get()); > >- WKRetainPtr<WKStringRef> useWaitToDumpWatchdogTimerKey = adoptWK(WKStringCreateWithUTF8CString("UseWaitToDumpWatchdogTimer")); >- WKRetainPtr<WKBooleanRef> useWaitToDumpWatchdogTimerValue = adoptWK(WKBooleanCreate(TestController::singleton().useWaitToDumpWatchdogTimer())); >- WKDictionarySetItem(beginTestMessageBody.get(), useWaitToDumpWatchdogTimerKey.get(), useWaitToDumpWatchdogTimerValue.get()); >- > WKRetainPtr<WKStringRef> timeoutKey = adoptWK(WKStringCreateWithUTF8CString("Timeout")); > WKRetainPtr<WKUInt64Ref> timeoutValue = adoptWK(WKUInt64Create(m_timeout.milliseconds())); > WKDictionarySetItem(beginTestMessageBody.get(), timeoutKey.get(), timeoutValue.get()); >@@ -357,8 +354,7 @@ void TestInvocation::didReceiveMessageFromInjectedBundle(WKStringRef messageName > WKRetainPtr<WKStringRef> audioResultKey = adoptWK(WKStringCreateWithUTF8CString("AudioResult")); > m_audioResult = static_cast<WKDataRef>(WKDictionaryGetItemForKey(messageBodyDictionary, audioResultKey.get())); > >- m_gotFinalMessage = true; >- TestController::singleton().notifyDone(); >+ done(); > return; > } > >@@ -832,7 +828,7 @@ WKRetainPtr<WKTypeRef> TestInvocation::didReceiveSynchronousMessageFromInjectedB > > if (WKStringIsEqualToUTF8CString(messageName, "SetWaitUntilDone")) { > ASSERT(WKGetTypeID(messageBody) == WKBooleanGetTypeID()); >- m_waitUntilDone = static_cast<unsigned char>(WKBooleanGetValue(static_cast<WKBooleanRef>(messageBody))); >+ setWaitUntilDone(static_cast<unsigned char>(WKBooleanGetValue(static_cast<WKBooleanRef>(messageBody)))); > return nullptr; > } > if (WKStringIsEqualToUTF8CString(messageName, "GetWaitUntilDone")) >@@ -1829,4 +1825,46 @@ void TestInvocation::performCustomMenuAction() > WKPagePostMessageToInjectedBundle(TestController::singleton().mainWebView()->page(), messageName.get(), 0); > } > >+void TestInvocation::initializeWaitToDumpWatchdogTimerIfNeeded() >+{ >+ if (m_waitToDumpWatchdogTimer.isActive()) >+ return; >+ >+ m_waitToDumpWatchdogTimer.startOneShot(m_timeout); >+} >+ >+void TestInvocation::invalidateWaitToDumpWatchdogTimer() >+{ >+ m_waitToDumpWatchdogTimer.stop(); >+} >+ >+void TestInvocation::waitToDumpWatchdogTimerFired() >+{ >+ invalidateWaitToDumpWatchdogTimer(); >+ >+#if PLATFORM(COCOA) >+ char buffer[1024]; >+ snprintf(buffer, sizeof(buffer), "#PID UNRESPONSIVE - %s (pid %d)\n", getprogname(), getpid()); >+ outputText(buffer); >+#endif >+ outputText("FAIL: Timed out waiting for notifyDone to be called\n\n"); >+ done(); >+} >+ >+void TestInvocation::setWaitUntilDone(bool waitUntilDone) >+{ >+ m_waitUntilDone = waitUntilDone; >+ if (waitUntilDone && TestController::singleton().useWaitToDumpWatchdogTimer()) >+ initializeWaitToDumpWatchdogTimerIfNeeded(); >+} >+ >+void TestInvocation::done() >+{ >+ m_gotFinalMessage = true; >+ invalidateWaitToDumpWatchdogTimer(); >+ RunLoop::main().dispatch([] { >+ TestController::singleton().notifyDone(); >+ }); >+} >+ > } // namespace WTR >diff --git a/Tools/WebKitTestRunner/TestInvocation.h b/Tools/WebKitTestRunner/TestInvocation.h >index 9d3cf2fcffffa23edd44c0800717f695bd694497..c1a8b643bb627203419d0eceaacf00af8dba5e57 100644 >--- a/Tools/WebKitTestRunner/TestInvocation.h >+++ b/Tools/WebKitTestRunner/TestInvocation.h >@@ -33,6 +33,7 @@ > #include <WebKit/WKRetainPtr.h> > #include <string> > #include <wtf/Noncopyable.h> >+#include <wtf/RunLoop.h> > #include <wtf/Seconds.h> > #include <wtf/text/StringBuilder.h> > >@@ -93,6 +94,13 @@ public: > private: > WKRetainPtr<WKMutableDictionaryRef> createTestSettingsDictionary(); > >+ void waitToDumpWatchdogTimerFired(); >+ void initializeWaitToDumpWatchdogTimerIfNeeded(); >+ void invalidateWaitToDumpWatchdogTimer(); >+ >+ void done(); >+ void setWaitUntilDone(bool); >+ > void dumpResults(); > static void dump(const char* textToStdout, const char* textToStderr = 0, bool seenError = false); > enum class SnapshotResultType { WebView, WebContents }; >@@ -119,6 +127,7 @@ private: > > WKRetainPtr<WKURLRef> m_url; > WTF::String m_urlString; >+ RunLoop::Timer<TestInvocation> m_waitToDumpWatchdogTimer; > > std::string m_expectedPixelHash; >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 197333
:
368366
| 368367