Bug 92952 – Layout Test storage/indexeddb/intversion-omit-parameter.html is flaky

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Alexander Pavlov (apavlov)]

@haraken: the patch author email ([e-mail]) is not known to bugzilla, so I'm assigning the bug to you as the reviewer. Hope you will handle it appropriately.

The following layout test is flakily crashing on Chromium Linux Debug bots:
storage/indexeddb/intversion-omit-parameter.html

with the following stderr:

crash log for DumpRenderTree (pid 15745):
STDOUT: <empty>
STDERR: ASSERTION FAILED: m_readyState == PENDING || m_readyState == DONE
STDERR: third_party/WebKit/Source/WebCore/Modules/indexeddb/IDBRequest.cpp(222) : virtual void WebCore::IDBRequest::onError(WTF::PassRefPtr<WebCore::IDBDatabaseError>)
STDERR: 1   0x7fbb1b57f8d5
STDERR: 2   0x7fbb1a17693c
STDERR: 3   0x7fbb1a10c060
STDERR: 4   0x7fbb1b53b1c7
STDERR: 5   0x7fbb1b53bd17
STDERR: 6   0x7fbb1b53cb4e
STDERR: 7   0x7fbb1a178b7e
STDERR: 8   0x7fbb1a10ebdb
STDERR: 9   0x7fbb1b536860
STDERR: 10  0x7fbb1b536766
STDERR: 11  0x7fbb1b536e6a
STDERR: 12  0x7fbb1af4e71f
STDERR: 13  0x7fbb1ae7fdf0
STDERR: 14  0x7fbb1ae80006
STDERR: 15  0x7fbb1bb4e8cc
STDERR: 16  0x7fbb1bb5109b
STDERR: 17  0x7fbb1a16634b
STDERR: 18  0x7fbb1a101e30
STDERR: 19  0x7fbb1a105936
STDERR: 20  0x7fbb1ba99d5b
STDERR: 21  0x7fbb1ba99219
STDERR: 22  0x7fbb1ba775c9
STDERR: 23  0x7fbb1ba7772e
STDERR: 24  0x7fbb1ba77c9f
STDERR: 25  0x7fbb1baaf607
STDERR: 26  0x7fbb1bac447b
STDERR: 27  0x7fbb1bab0ab4
STDERR: 28  0x7fbb1bac4d63
STDERR: 29  0x7fbb1b413cc2
STDERR: 30  0x7fbb15733c80
STDERR: 31  0x5e4c43
STDERR: [15745:15745:11351259856559:ERROR:process_util_posix.cc(143)] Received signal 11
STDERR: 	base::debug::StackTrace::StackTrace() [0x7fbb170f5156]
STDERR: 	base::(anonymous namespace)::StackDumpSignalHandler() [0x7fbb1715a5fd]
STDERR: 	0x7fbb10c7eaf0
STDERR: 	WebCore::IDBRequest::onError() [0x7fbb1b57f8df]
STDERR: 	WebKit::WebIDBCallbacksImpl::onError() [0x7fbb1a17693c]
STDERR: 	WebKit::IDBCallbacksProxy::onError() [0x7fbb1a10c060]
STDERR: 	WebCore::IDBDatabaseBackendImpl::setVersion() [0x7fbb1b53b1c7]
STDERR: 	WebCore::IDBDatabaseBackendImpl::processPendingCalls() [0x7fbb1b53bd17]
STDERR: 	WebCore::IDBDatabaseBackendImpl::close() [0x7fbb1b53cb4e]
STDERR: 	WebKit::WebIDBDatabaseImpl::close() [0x7fbb1a178b7e]
STDERR: 	WebKit::IDBDatabaseBackendProxy::close() [0x7fbb1a10ebdb]
STDERR: 	WebCore::IDBDatabase::closeConnection() [0x7fbb1b536860]
STDERR: 	WebCore::IDBDatabase::close() [0x7fbb1b536766]
STDERR: 	WebCore::IDBDatabase::stop() [0x7fbb1b536e6a]
STDERR: 	WebCore::ScriptExecutionContext::stopActiveDOMObjects() [0x7fbb1af4e71f]
STDERR: 	WebCore::Document::detach() [0x7fbb1ae7fdf0]
STDERR: 	WebCore::Document::prepareForDestruction() [0x7fbb1ae80006]
STDERR: 	WebCore::Frame::setView() [0x7fbb1bb4e8cc]
STDERR: 	WebCore::Frame::createView() [0x7fbb1bb5109b]
STDERR: 	WebKit::WebFrameImpl::createFrameView() [0x7fbb1a16634b]
STDERR: 	WebKit::FrameLoaderClientImpl::makeDocumentView() [0x7fbb1a101e30]
STDERR: 	WebKit::FrameLoaderClientImpl::transitionToCommittedForNewPage() [0x7fbb1a105936]
STDERR: 	WebCore::FrameLoader::transitionToCommitted() [0x7fbb1ba99d5b]
STDERR: 	WebCore::FrameLoader::commitProvisionalLoad() [0x7fbb1ba99219]
STDERR: 	WebCore::DocumentLoader::commitIfReady() [0x7fbb1ba775c9]
STDERR: 	WebCore::DocumentLoader::commitLoad() [0x7fbb1ba7772e]
STDERR: 	WebCore::DocumentLoader::receivedData() [0x7fbb1ba77c9f]
STDERR: 	WebCore::MainResourceLoader::addData() [0x7fbb1baaf607]
STDERR: 	WebCore::ResourceLoader::didReceiveData() [0x7fbb1bac447b]
STDERR: 	WebCore::MainResourceLoader::didReceiveData() [0x7fbb1bab0ab4]
STDERR: 	WebCore::ResourceLoader::didReceiveData() [0x7fbb1bac4d63]
STDERR: 	WebCore::ResourceHandleInternal::didReceiveData() [0x7fbb1b413cc2]
STDERR: 	webkit_glue::WebURLLoaderImpl::Context::OnReceivedData() [0x7fbb15733c80]
STDERR: 	(anonymous namespace)::RequestProxy::NotifyReceivedData() [0x5e4c43]
STDERR: 	base::internal::RunnableAdapter<>::Run() [0x5eb4f5]
STDERR: 	base::internal::InvokeHelper<>::MakeItSo() [0x5eaebe]
STDERR: 	base::internal::Invoker<>::Run() [0x5ea75c]
STDERR: 	base::Callback<>::Run() [0x7fbb170ed2f9]
STDERR: 	MessageLoop::RunTask() [0x7fbb1712edf6]
STDERR: 	MessageLoop::DeferOrRunPendingTask() [0x7fbb1712ef0e]
STDERR: 	MessageLoop::DoWork() [0x7fbb1712f729]
STDERR: 	base::MessagePumpGlib::HandleDispatch() [0x7fbb170d3521]
STDERR: 	(anonymous namespace)::WorkSourceDispatch() [0x7fbb170d2c3b]
STDERR: 	0x7fbb119d78c2
STDERR: 	0x7fbb119db748
STDERR: 	0x7fbb119db8fc
STDERR: 	base::MessagePumpGlib::RunWithDispatcher() [0x7fbb170d31d0]
STDERR: 	base::MessagePumpGlib::Run() [0x7fbb170d35fe]
STDERR: 	MessageLoop::RunInternal() [0x7fbb1712eabb]
STDERR: 	MessageLoop::RunHandler() [0x7fbb1712e972]
STDERR: 	base::RunLoop::Run() [0x7fbb17162246]
STDERR: 	MessageLoop::Run() [0x7fbb1712e2a0]
STDERR: 	webkit_support::RunMessageLoop() [0x516999]
STDERR: 	TestShell::waitTestFinished() [0x4ad60e]
STDERR: 	TestShell::runFileTest() [0x4a66c6]
STDERR: 	runTest() [0x4725f7]
STDERR: 	main [0x472fde]
STDERR: 	0x7fbb10c69c4d
STDERR: 	0x470d49

Probable cause:
http://trac.webkit.org/changeset/124402

The flakiness dashboard link: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=storage%2Findexeddb%2Fintversion-omit-parameter.html

Suppressing for now.

[Peter Kasting]

This test was recently added, in r124383.  Perhaps the test is inherently flaky?  Hopefully one of the CCed folks has the expertise to look into this.

[David Grogan]

Xingnan, don't worry about this, it's not related to your patch.

[Xingnan Wang]

(In reply to comment #2)
> Xingnan, don't worry about this, it's not related to your patch.

That`s fine. BTW, I cannot reproduce it on my ubuntu11.10.

[David Grogan]

I can get content_shell (without any upgradeneeded code) to fail an assert on a good portion of IDB layout tests by hitting refresh a couple times (IDB's constant nemesis) or even just back <-> forward.

The pattern is
* There's a pending call on the backend, either open/close/deleteDatabase or just an openCursor that hasn't been processed
* ActiveDOMObjects get stop()'ped, IDBRequest gets marked EarlyDeath
* Backend proceeds with pending call
* IDBRequest::on{Success,Error} is called with the results of the pending call
* ASSERT(m_readyState == PENDING || m_readyState == DONE) fails because IDBRequest.m_readyState == EarlyDeath

Josh, do you remember off the top of your head if we had a mechanism in place to prevent this flow?

[Joshua Bell]

(In reply to comment #4)
> I can get content_shell (without any upgradeneeded code) to fail an assert on a good portion of IDB layout tests by hitting refresh a couple times (IDB's constant nemesis) or even just back <-> forward.
> 
> The pattern is
> * There's a pending call on the backend, either open/close/deleteDatabase or just an openCursor that hasn't been processed
> * ActiveDOMObjects get stop()'ped, IDBRequest gets marked EarlyDeath
> * Backend proceeds with pending call
> * IDBRequest::on{Success,Error} is called with the results of the pending call
> * ASSERT(m_readyState == PENDING || m_readyState == DONE) fails because IDBRequest.m_readyState == EarlyDeath
> 
> Josh, do you remember off the top of your head if we had a mechanism in place to prevent this flow?

Not specifically. That assert is new, though - I didn't consider the stopped case. :(

The asserts in any of the onXXX callbacks can probably be removed (inspect that I didn't over-simplify the code though). The enque/dispatch code handles the stopped case. I'll tackle this on Monday.

[Joshua Bell]

Created attachment 156708 [details]
Patch

[Joshua Bell]

Created attachment 156711 [details]
Patch

[Joshua Bell]

A direct layout test for this might be possible: initiate IDB operations (like in the omit test) then close the document before the pending events can be processed. Possibly in an iframe, so the top-level page persists and can see the eventual success events.

I'll play with it. In the mean time, dgrogan@ can you take a look and try the patch on your system to see if it does resolve the content shell repro you had?

[Joshua Bell]

Created attachment 156975 [details]
Patch

[Joshua Bell]

(In reply to comment #8)
> A direct layout test for this might be possible: initiate IDB operations (like in the omit test) then close the document before the pending events can be processed. Possibly in an iframe, so the top-level page persists and can see the eventual success events.

I gave up on the layout test and went with a Chromium unit test instead (in latest patch).

alecflett@, dgrogan@ - please take a look?

Most of the IDBRequest::onXXX() callbacks now have the same preamble. I considered extracting a private method that would encapsulate the "if stopped or aborted then bail early" logic and state assertions, e.g.  "if (!shouldProcessEvent()) return;" - thoughts?

[Alec Flett]

+1 for consolidation 

I seem to have just started hitting these ASSERTs (that are being removed here) in my createIndex patch. (way more in the browser tests than in DRT) I was in the process of trying to break it up when I started hitting them and this will certainly keep me from hitting them - is this happening more with the recent patches from david? Mostly I'm just trying to figure out if this what I'm running into, or if I'm inadvertently introducing bugs as I break up my mega patch :(

[Joshua Bell]

(In reply to comment #11)
> I seem to have just started hitting these ASSERTs (that are being removed here) in my createIndex patch. (way more in the browser tests than in DRT) I was in the process of trying to break it up when I started hitting them and this will certainly keep me from hitting them - is this happening more with the recent patches from david? Mostly I'm just trying to figure out if this what I'm running into, or if I'm inadvertently introducing bugs as I break up my mega patch :(

Yeah, dgrogan@'s patch makes the pending calls queue around database open/version/close/delete more necessarily more complex, and introduces more opportunities for events to get triggered after context destruction which is the trigger for these asserts.

I'll re-up the patch with common logic factored out.

[Joshua Bell]

Created attachment 157003 [details]
Patch

[Joshua Bell]

tony@ - r?

[David Grogan]

LGTM, thanks for fixing this.

[WebKit Review Bot]

Comment on attachment 157003 [details]
Patch

Clearing flags on attachment: 157003

Committed r124974: <http://trac.webkit.org/changeset/124974>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.