Bug 186214 – Have WebKitTestRunner check for abandoned documents

• Status: ASSIGNED
• Component: Tools / Tests

[Simon Fraser (smfr)]

When running tests, we should check that we're not abandoning Document objects (via retain cycles). These don't show up as leaks because they are rooted by Document::allDocuments(), but it's very easy to introduce bugs that trigger Document abandonment.

[Radar WebKit Bug Importer]

<rdar://problem/40740917>

[Simon Fraser (smfr)]

Created attachment 341812 [details]
Hacky patch

[Simon Fraser (smfr)]

We also have to wait for timers to fire; tests with media controls keep the document alive until images load.

[Simon Fraser (smfr)]

Created attachment 347422 [details]
Patch

[EWS Watchlist]

Attachment 347422 [details] did not pass style-queue:


ERROR: Tools/WebKitTestRunner/TestController.cpp:1379:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Tools/WebKitTestRunner/TestController.h:53:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Tools/WebKitTestRunner/TestController.h:73:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Tools/WebKitTestRunner/TestController.h:263:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Tools/Scripts/webkitpy/layout_tests/views/buildbot_results.py:177:  whitespace before '}'  [pep8/E202] [5]
Total errors found: 5 in 30 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 347422 [details]
Patch

Attachment 347422 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/8898077

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 347424 [details]
Archive of layout-test-results from ews102 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 347422 [details]
Patch

Attachment 347422 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/8898066

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 347425 [details]
Archive of layout-test-results from ews115 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Joseph Pecoraro]

Comment on attachment 347422 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=347422&action=review

This seems great! I couldn't follow much of the python, but I'd be happy to rubber-stamp that. Just get the bots green!

> Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:293
> +         # Defaulting to True just for EWS testing.

Style: Leading whitespace is weird and who knows what that does to python.

> Tools/WebKitTestRunner/TestController.cpp:951
> +void TestController::checkForAbandonedDocuments()

Typo: "Abandonded" => "Abandoned" (a few places)

> Tools/WebKitTestRunner/TestController.cpp:1519
> +    String currentTestURL = m_currentInvocation ? toWTFString(adoptWK(WKURLCopyString(m_currentInvocation->url()))) : "no test";

Style: "no test"_s;

> LayoutTests/fast/harness/results.html:1362
> +        cell.innerHTML = '<span class=expand-button onclick="controller.toggleExpectations(this)"><span class=expand-button-text>+</span></span>' + this._resultsController.testLink(testResult);

Style: It's a little weird seeing semi-dirty HTML like this. I'd expect class name attributes wrapped in double quotes, but you're following the existing pattern.

> LayoutTests/fast/harness/results.html:1385
> +        abandonedDocsDT.innerHTML = 'Abandoned documents<sup><a href="https://trac.webkit.org/wiki/Abandoned%20documents">?</a></sup>:';

Nice!

[Simon Fraser (smfr)]

Created attachment 347429 [details]
Patch

[EWS Watchlist]

Attachment 347429 [details] did not pass style-queue:


ERROR: Tools/WebKitTestRunner/TestController.cpp:1379:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Tools/WebKitTestRunner/TestController.h:53:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Tools/WebKitTestRunner/TestController.h:73:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Tools/WebKitTestRunner/TestController.h:263:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Tools/Scripts/webkitpy/layout_tests/views/buildbot_results.py:177:  whitespace before '}'  [pep8/E202] [5]
Total errors found: 5 in 32 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 347429 [details]
Patch

Attachment 347429 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/8899032

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 347431 [details]
Archive of layout-test-results from ews102 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 347429 [details]
Patch

Attachment 347429 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/8898996

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 347433 [details]
Archive of layout-test-results from ews117 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Simon Fraser (smfr)]

Created attachment 347445 [details]
Patch

[EWS Watchlist]

Attachment 347445 [details] did not pass style-queue:


ERROR: Tools/WebKitTestRunner/TestController.cpp:1379:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Tools/WebKitTestRunner/TestController.h:53:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Tools/WebKitTestRunner/TestController.h:73:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Tools/WebKitTestRunner/TestController.h:263:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Tools/Scripts/webkitpy/layout_tests/views/buildbot_results.py:177:  whitespace before '}'  [pep8/E202] [5]
Total errors found: 5 in 33 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Simon Fraser (smfr)]

Some causes of false positives:

Workers:
1   0x641f5afc8 WebCore::Document::~Document()
2   0x642335285 WebCore::HTMLDocument::~HTMLDocument()
3   0x6423352a5 WebCore::HTMLDocument::~HTMLDocument()
4   0x642335349 WebCore::HTMLDocument::~HTMLDocument()
5   0x641f5eb14 WebCore::Document::decrementReferencingNodeCount()
6   0x641f5e81e WebCore::Document::removedLastRef()
7   0x6420a6d17 WebCore::Node::removedLastRef()
8   0x6420a6ccd WebCore::Node::deref(unsigned int)
9   0x641f3c04b WebCore::Document::derefScriptExecutionContext(unsigned int)
10  0x6414a4752 WebCore::ScriptExecutionContext::deref(unsigned int)
11  0x6414a46d4 void WTF::derefIfNotNull<WebCore::ScriptExecutionContext>(WebCore::ScriptExecutionContext*, unsigned int)
12  0x6414a4694 WTF::RefPtr<WebCore::ScriptExecutionContext, WTF::DumbPtrTraits<WebCore::ScriptExecutionContext> >::~RefPtr()
13  0x6414a2735 WTF::RefPtr<WebCore::ScriptExecutionContext, WTF::DumbPtrTraits<WebCore::ScriptExecutionContext> >::~RefPtr()
14  0x643873f69 WebCore::WorkerMessagingProxy::~WorkerMessagingProxy()
15  0x643874035 WebCore::WorkerMessagingProxy::~WorkerMessagingProxy()
16  0x6438740b9 WebCore::WorkerMessagingProxy::~WorkerMessagingProxy()
17  0x643876105 WTF::ThreadSafeRefCounted<WebCore::WorkerMessagingProxy, (WTF::DestructionThread)0>::deref(unsigned int) const
18  0x643876057 WebCore::WorkerMessagingProxy::workerGlobalScopeDestroyedInternal()
19  0x6438801ef WebCore::WorkerMessagingProxy::workerObjectDestroyed()::$_5::operator()(WebCore::ScriptExecutionContext&) const
20  0x643880174 WTF::Function<void (WebCore::ScriptExecutionContext&)>::CallableWrapper<WebCore::WorkerMessagingProxy::workerObjectDestroyed()::$_5>::call(WebCore::ScriptExecutionContext&)
21  0x6418eb2f2 WTF::Function<void (WebCore::ScriptExecutionContext&)>::operator()(WebCore::ScriptExecutionContext&) const
22  0x6418d7fad WebCore::ScriptExecutionContext::Task::performTask(WebCore::ScriptExecutionContext&)
23  0x641fec59a WebCore::Document::postTask(WebCore::ScriptExecutionContext::Task&&)::$_2::operator()()
24  0x641fec3b9 WTF::Function<void ()>::CallableWrapper<WebCore::Document::postTask(WebCore::ScriptExecutionContext::Task&&)::$_2>::call()
25  0x65077c18f WTF::Function<void ()>::operator()() const
26  0x6507aaf2f WTF::dispatchFunctionsFromMainThread()
27  0x6507ae341 WTF::timerFired(__CFRunLoopTimer*, void*)
28  0x7fff2e421014 __CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__
29  0x7fff2e420c87 __CFRunLoopDoTimer

Font caches (fast/css3-text/css3-text-decoration/repaint/underline-outside-of-layout-rect-altered.html):

Document 0x2e22011b8 reference stacks:
Backtrace for token 431
1   0x2c5c2208c WebCore::adopted(WebCore::Node*)
2   0x2c785461b WTF::Ref<WebCore::SVGDocument, WTF::DumbPtrTraits<WebCore::SVGDocument> > WTF::adoptRef<WebCore::SVGDocument, WTF::DumbPtrTraits<WebCore::SVGDocument> >(WebCore::SVGDocument&)
3   0x2c785175c WebCore::SVGDocument::create(WebCore::Frame*, WebCore::URL const&)
4   0x2c81fabc8 WebCore::CachedSVGFont::ensureCustomFontData(WTF::AtomicString const&)
5   0x2c75e906f WebCore::CSSFontFaceSource::fontLoaded(WebCore::CachedFont&)
6   0x2c81c1d30 WebCore::CachedFont::checkNotify()
7   0x2c81c0165 WebCore::CachedFont::finishLoading(WebCore::SharedBuffer*)
8   0x2c8159a48 WebCore::SubresourceLoader::didFinishLoading(WebCore::NetworkLoadMetrics const&)
9   0x2c0f6dffb WebKit::WebResourceLoader::didFinishResourceLoad(WebCore::NetworkLoadMetrics const&)
10  0x2c0f7267a void IPC::callMemberFunctionImpl<WebKit::WebResourceLoader, void (WebKit::WebResourceLoader::*)(WebCore::NetworkLoadMetrics const&), std::__1::tuple<WebCore::NetworkLoadMetrics>, 0ul>(WebKit::WebResourceLoader*, void (WebKit::WebResourceLoader::*)(WebCore::NetworkLoadMetrics const&), std::__1::tuple<WebCore::NetworkLoadMetrics>&&, std::__1::integer_sequence<unsigned long, 0ul>)
11  0x2c0f72500 void IPC::callMemberFunction<WebKit::WebResourceLoader, void (WebKit::WebResourceLoader::*)(WebCore::NetworkLoadMetrics const&), std::__1::tuple<WebCore::NetworkLoadMetrics>, std::__1::integer_sequence<unsigned long, 0ul> >(std::__1::tuple<WebCore::NetworkLoadMetrics>&&, WebKit::WebResourceLoader*, void (WebKit::WebResourceLoader::*)(WebCore::NetworkLoadMetrics const&))
12  0x2c0f716da void IPC::handleMessage<Messages::WebResourceLoader::DidFinishResourceLoad, WebKit::WebResourceLoader, void (WebKit::WebResourceLoader::*)(WebCore::NetworkLoadMetrics const&)>(IPC::Decoder&, WebKit::WebResourceLoader*, void (WebKit::WebResourceLoader::*)(WebCore::NetworkLoadMetrics const&))
13  0x2c0f70cec WebKit::WebResourceLoader::didReceiveWebResourceLoaderMessage(IPC::Connection&, IPC::Decoder&)
14  0x2c03c80c9 WebKit::NetworkProcessConnection::didReceiveMessage(IPC::Connection&, IPC::Decoder&)
15  0x2c0775b5c IPC::Connection::dispatchMessage(IPC::Decoder&)
16  0x2c07686cd IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)

Wrappers in a different world (fast/encoding/mailto-always-utf-8.html, fast/loader/policy-delegate-action-hit-test-zoomed.html etc)?

Backtrace for token 704
1   0x5f20a6a87 WebCore::Node::ref()
2   0x5f0463d68 WTF::Ref<WebCore::Document, WTF::DumbPtrTraits<WebCore::Document> >::Ref(WebCore::Document&)
3   0x5f0463d2d WTF::Ref<WebCore::Document, WTF::DumbPtrTraits<WebCore::Document> >::Ref(WebCore::Document&)
4   0x5f1a82d09 WebCore::toJS(JSC::ExecState*, WebCore::JSDOMGlobalObject*, WebCore::Document&)
5   0x5f1a99358 WebCore::createWrapperInline(JSC::ExecState*, WebCore::JSDOMGlobalObject*, WTF::Ref<WebCore::Node, WTF::DumbPtrTraits<WebCore::Node> >&&)
6   0x5f1a99080 WebCore::createWrapper(JSC::ExecState*, WebCore::JSDOMGlobalObject*, WTF::Ref<WebCore::Node, WTF::DumbPtrTraits<WebCore::Node> >&&)
7   0x5f03223be WebCore::toJS(JSC::ExecState*, WebCore::JSDOMGlobalObject*, WebCore::Node&)
8   0x5f037bb90 WebCore::toJS(JSC::ExecState*, WebCore::JSDOMGlobalObject*, WebCore::Node*)
9   0x5f1a70fb3 WebCore::JSDOMWindowBase::updateDocument()
10  0x5f1ac275e WebCore::ScriptController::initScriptForWindowProxy(WebCore::JSWindowProxy&)
11  0x5f1b43435 WebCore::WindowProxy::createJSWindowProxyWithInitializedScript(WebCore::DOMWrapperWorld&)
12  0x5f1ab30b6 WebCore::WindowProxy::jsWindowProxy(WebCore::DOMWrapperWorld&)
13  0x5f1ac154b WebCore::ScriptController::jsWindowProxy(WebCore::DOMWrapperWorld&)
14  0x105d5975d WebCore::ScriptController::globalObject(WebCore::DOMWrapperWorld&)
15  0x1063924f6 WebKit::WebFrame::jsContextForWorld(WebKit::InjectedBundleScriptWorld*)
16  0x1069e8552 WKBundleFrameGetJavaScriptContextForWorld
17  0x607c32249 WTR::dumpPath(OpaqueWKBundlePage const*, OpaqueWKBundleScriptWorld const*, OpaqueWKBundleNodeHandle const*)
18  0x607c31cbc WTR::InjectedBundlePage::decidePolicyForNavigationAction(OpaqueWKBundlePage const*, OpaqueWKBundleFrame const*, OpaqueWKBundleNavigationAction const*, OpaqueWKURLRequest const*, void const**)


Cached SVG images (fast/hidpi/video-controls-in-hidpi.html). Not sure why clearing the memory cache isn't releasing these:

Document 0x1df202370 13 reference stacks:
Backtrace for token 3007
1   0x1bfa52a67 WebCore::Node::ref()
2   0x1bed26f21 unsigned int WTF::refIfNotNull<WebCore::Document>(WebCore::Document*)
3   0x1bed26ed8 WTF::RefPtr<WebCore::Document, WTF::DumbPtrTraits<WebCore::Document> >::RefPtr(WebCore::Document*)
4   0x1bed26dcd WTF::RefPtr<WebCore::Document, WTF::DumbPtrTraits<WebCore::Document> >::RefPtr(WebCore::Document*)
5   0x1c0341bcf WTF::RefPtr<WebCore::Document, WTF::DumbPtrTraits<WebCore::Document> >::copyRef() const
6   0x1c034165c WebCore::Frame::setDocument(WTF::RefPtr<WebCore::Document, WTF::DumbPtrTraits<WebCore::Document> >&&)
7   0x1c015d18d WebCore::DocumentWriter::begin(WebCore::URL const&, bool, WebCore::Document*)
8   0x1c11f55b7 WebCore::SVGImage::dataChanged(bool)
9   0x1c06702f3 WebCore::Image::setData(WTF::RefPtr<WebCore::SharedBuffer, WTF::DumbPtrTraits<WebCore::SharedBuffer> >&&, bool)
10  0x1c025b789 WebCore::CachedImage::updateImageData(bool)
11  0x1c025bcd1 WebCore::CachedImage::finishLoading(WebCore::SharedBuffer*)
12  0x1c01eda28 WebCore::SubresourceLoader::didFinishLoading(WebCore::NetworkLoadMetrics const&)
13  0x1c01e7618 auto WebCore::ResourceLoader::loadDataURL()::$_9::operator()<std::optional<WebCore::DataURLDecoder::Result> >(std::optional<WebCore::DataURLDecoder::Result>)::'lambda'()::operator()()
14  0x1c01e7489 WTF::Function<void ()>::CallableWrapper<auto WebCore::ResourceLoader::loadDataURL()::$_9::operator()<std::optional<WebCore::DataURLDecoder::Result> >(std::optional<WebCore::DataURLDecoder::Result>)::'lambda'()>::call()
15  0x1bd9b995f WTF::Function<void ()>::operator()() const