Bug 112205 - REGRESSION (r144884): WebKit2.DOMWindowExtensionBasic API test is asserting
Summary: REGRESSION (r144884): WebKit2.DOMWindowExtensionBasic API test is asserting
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-03-12 15:58 PDT by Simon Fraser (smfr)
Modified: 2013-03-18 09:51 PDT (History)
6 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2013-03-12 15:58:13 PDT
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
Comment 1 Radar WebKit Bug Importer 2013-03-12 16:01:08 PDT
<rdar://problem/13405530>
Comment 2 Simon Fraser (smfr) 2013-03-13 15:32:00 PDT
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
Comment 3 Simon Fraser (smfr) 2013-03-15 21:40:28 PDT
Test disabled in http://trac.webkit.org/changeset/145981
Comment 4 Geoffrey Garen 2013-03-16 17:40:57 PDT
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.
Comment 5 Brady Eidson 2013-03-18 09:47:18 PDT
(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.
Comment 6 Brady Eidson 2013-03-18 09:48:53 PDT
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.
Comment 7 Brady Eidson 2013-03-18 09:51:01 PDT
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")