Bug 112205 – REGRESSION (r144884): WebKit2.DOMWindowExtensionBasic API test is asserting

• Status: NEW
• Component: JavaScriptCore

[Simon Fraser (smfr)]

This test is timing out on the bots:
http://build.webkit.org/builders/Apple%20MountainLion%20Debug%20WK1%20%28Tests%29/builds/6305/steps/run-api-tests/logs/stdio

Note: Google Test filter = WebKit2.DOMWindowExtensionBasic
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from WebKit2
[ RUN      ] WebKit2.DOMWindowExtensionBasic
SHOULD NEVER BE REACHED
/Volumes/Data/slave/mountainlion-debug/build/Tools/TestWebKitAPI/Tests/WebKit2/DOMWindowExtensionBasic_Bundle.cpp(231) : void TestWebKitAPI::DOMWindowExtensionBasic::willDestroyGlobalObjectForDOMWindowExtension(WKBundleDOMWindowExtensionRef)
1   0x11087f337 TestWebKitAPI::DOMWindowExtensionBasic::willDestroyGlobalObjectForDOMWindowExtension(OpaqueWKBundleDOMWindowExtension const*)
2   0x11087ef84 TestWebKitAPI::willDestroyGlobalObjectForDOMWindowExtensionCallback(OpaqueWKBundlePage const*, OpaqueWKBundleDOMWindowExtension const*, void const*)
3   0x10608bc3a WebKit::InjectedBundlePageLoaderClient::willDestroyGlobalObjectForDOMWindowExtension(WebKit::WebPage*, WebCore::DOMWindowExtension*)
4   0x10626fdad WebKit::WebFrameLoaderClient::dispatchWillDestroyGlobalObjectForDOMWindowExtension(WebCore::DOMWindowExtension*)
5   0x108b9481b WebCore::DOMWindowExtension::willDestroyGlobalObjectInCachedFrame()
6   0x108b82d95 WebCore::DOMWindow::willDestroyCachedFrame()
7   0x1086cff5a WebCore::CachedFrame::destroy()
8   0x1086dbf3b WebCore::CachedPage::destroy()
9   0x1086dbe6c WebCore::CachedPage::~CachedPage()
10  0x1086dbe35 WebCore::CachedPage::~CachedPage()
11  0x108d131c9 WTF::RefCounted<WebCore::CachedPage>::deref()
12  0x108d13175 void WTF::derefIfNotNull<WebCore::CachedPage>(WebCore::CachedPage*)
13  0x109890337 WTF::RefPtr<WebCore::CachedPage>::clear()
14  0x10988ec3c WebCore::PageCache::remove(WebCore::HistoryItem*)
15  0x1061fffc2 WebKit::WebBackForwardListProxy::removeItem(unsigned long long)
16  0x1062cc559 WebKit::WebPage::didRemoveBackForwardItem(unsigned long long)
17  0x10630c3ba void CoreIPC::callMemberFunction<WebKit::WebPage, void (WebKit::WebPage::*)(unsigned long long), unsigned long long>(CoreIPC::Arguments1<unsigned long long> const&, WebKit::WebPage*, void (WebKit::WebPage::*)(unsigned long long))
18  0x106302722 void CoreIPC::handleMessage<Messages::WebPage::DidRemoveBackForwardItem, WebKit::WebPage, void (WebKit::WebPage::*)(unsigned long long)>(CoreIPC::MessageDecoder&, WebKit::WebPage*, void (WebKit::WebPage::*)(unsigned long long))
19  0x1062fc8c5 WebKit::WebPage::didReceiveWebPageMessage(CoreIPC::Connection*, CoreIPC::MessageDecoder&)
20  0x1062cbdd7 WebKit::WebPage::didReceiveMessage(CoreIPC::Connection*, CoreIPC::MessageDecoder&)
21  0x1062cbe17 non-virtual thunk to WebKit::WebPage::didReceiveMessage(CoreIPC::Connection*, CoreIPC::MessageDecoder&)
22  0x1060a09b7 CoreIPC::MessageReceiverMap::dispatchMessage(CoreIPC::Connection*, CoreIPC::MessageDecoder&)
23  0x1063a84ca WebKit::WebProcess::didReceiveMessage(CoreIPC::Connection*, CoreIPC::MessageDecoder&)
24  0x105ffeed3 CoreIPC::Connection::dispatchMessage(CoreIPC::MessageDecoder&)
25  0x105ffb6ba CoreIPC::Connection::dispatchMessage(WTF::PassOwnPtr<CoreIPC::MessageDecoder>)
26  0x105ffee6b CoreIPC::Connection::dispatchOneMessage()
27  0x10600a4e2 WTF::FunctionWrapper<void (CoreIPC::Connection::*)()>::operator()(CoreIPC::Connection*)
28  0x10600a465 WTF::BoundFunctionImpl<WTF::FunctionWrapper<void (CoreIPC::Connection::*)()>, void (CoreIPC::Connection*)>::operator()()
29  0x109c67e29 WTF::Function<void ()>::operator()() const
30  0x109c67bab WebCore::RunLoop::performWork()
31  0x109c68e4e WebCore::RunLoop::performWork(void*)

This may have regressed in http://trac.webkit.org/changeset/144884

[Radar WebKit Bug Importer]

<rdar://problem/13405530>

[Simon Fraser (smfr)]

This is the only think preventing some bots from being green:
http://build.webkit.org/builders/Apple%20MountainLion%20Debug%20WK1%20%28Tests%29/builds/6338

[Simon Fraser (smfr)]

Test disabled in http://trac.webkit.org/changeset/145981

[Geoffrey Garen]

This ASSERT looks incorrect:

void DOMWindowExtensionBasic::willDestroyGlobalObjectForDOMWindowExtension(WKBundleDOMWindowExtensionRef)
{
    // All of the items are candidates for the page cache and should not be evicted from the page
    // cache before the test completes.
    ASSERT_NOT_REACHED();
}

There's no reasoning given here for why items shouldn't be evicted from the page cache.

I think we should delete this ASSERT.

[Brady Eidson]

(In reply to comment #4)
> This ASSERT looks incorrect:
> 
> void DOMWindowExtensionBasic::willDestroyGlobalObjectForDOMWindowExtension(WKBundleDOMWindowExtensionRef)
> {
>     // All of the items are candidates for the page cache and should not be evicted from the page
>     // cache before the test completes.
>     ASSERT_NOT_REACHED();
> }
> 
> There's no reasoning given here for why items shouldn't be evicted from the page cache.
> 
> I think we should delete this ASSERT.

The point of this test was to test how this API works with the page cache.

If the items are either not going in to the page cache during the test or they are being evicted before the test is over, then the test isn't testing what it was supposed to.

The ASSERT catches this, and has revealed a real problem.

[Brady Eidson]

Looking at the change that caused this, I agree the test itself is not constructed well.  But the solution is to fix the test, not simply remove the assert.

[Brady Eidson]

I'm also weary of reentrancy now that we synchronously clear the page cache...

I don't think the solution to https://bugs.webkit.org/show_bug.cgi?id=111522 was to synchronously clear the page cache, but rather we should release on a 0-delay (instead of a bizarre "autorelease in the future" delay")