Bug 186214 - Have WebKitTestRunner check for abandoned documents
Summary: Have WebKitTestRunner check for abandoned documents
Status: ASSIGNED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords: InRadar
Depends on: 189332 189648 188728 188994 189067 189088 189293 189437 189459
Blocks:
  Show dependency treegraph
 
Reported: 2018-06-01 17:46 PDT by Simon Fraser (smfr)
Modified: 2018-09-15 11:39 PDT (History)
9 users (show)

See Also:


Attachments
Hacky patch (10.45 KB, patch)
2018-06-01 17:54 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (70.76 KB, patch)
2018-08-17 19:11 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-sierra (320.87 KB, application/zip)
2018-08-17 19:59 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews115 for mac-sierra (323.57 KB, application/zip)
2018-08-17 20:08 PDT, EWS Watchlist
no flags Details
Patch (73.66 KB, patch)
2018-08-17 21:48 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-sierra (566.59 KB, application/zip)
2018-08-17 22:35 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews117 for mac-sierra (566.47 KB, application/zip)
2018-08-17 22:51 PDT, EWS Watchlist
no flags Details
Patch (75.31 KB, patch)
2018-08-18 10:52 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2018-06-01 17:46:38 PDT
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.
Comment 1 Radar WebKit Bug Importer 2018-06-01 17:51:18 PDT
<rdar://problem/40740917>
Comment 2 Simon Fraser (smfr) 2018-06-01 17:54:16 PDT
Created attachment 341812 [details]
Hacky patch
Comment 3 Simon Fraser (smfr) 2018-06-01 20:10:08 PDT
We also have to wait for timers to fire; tests with media controls keep the document alive until images load.
Comment 4 Simon Fraser (smfr) 2018-08-17 19:11:00 PDT
Created attachment 347422 [details]
Patch
Comment 5 EWS Watchlist 2018-08-17 19:13:46 PDT Comment hidden (obsolete)
Comment 6 EWS Watchlist 2018-08-17 19:59:17 PDT Comment hidden (obsolete)
Comment 7 EWS Watchlist 2018-08-17 19:59:19 PDT Comment hidden (obsolete)
Comment 8 EWS Watchlist 2018-08-17 20:08:15 PDT Comment hidden (obsolete)
Comment 9 EWS Watchlist 2018-08-17 20:08:17 PDT Comment hidden (obsolete)
Comment 10 Joseph Pecoraro 2018-08-17 20:57:05 PDT
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!
Comment 11 Simon Fraser (smfr) 2018-08-17 21:48:57 PDT
Created attachment 347429 [details]
Patch
Comment 12 EWS Watchlist 2018-08-17 21:50:26 PDT Comment hidden (obsolete)
Comment 13 EWS Watchlist 2018-08-17 22:35:55 PDT Comment hidden (obsolete)
Comment 14 EWS Watchlist 2018-08-17 22:35:57 PDT Comment hidden (obsolete)
Comment 15 EWS Watchlist 2018-08-17 22:51:46 PDT Comment hidden (obsolete)
Comment 16 EWS Watchlist 2018-08-17 22:51:48 PDT Comment hidden (obsolete)
Comment 17 Simon Fraser (smfr) 2018-08-18 10:52:43 PDT
Created attachment 347445 [details]
Patch
Comment 18 EWS Watchlist 2018-08-18 10:54:55 PDT Comment hidden (obsolete)
Comment 19 Simon Fraser (smfr) 2018-08-18 17:16:30 PDT
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