There are a bunch of tests in a the imported/w3c/web-platform-tests/IndexedDB/* when when one fails the majority will fail in the same way (generally over 300 tests will fail). Possible issue could be state being retained in the TestRunner. Sample Run: https://build.webkit.org/builders/Apple%20High%20Sierra%20Release%20WK2%20%28Tests%29/builds/5203/steps/layout-test/logs/stdio
Created attachment 343766 [details] Test order to reproduce
(In reply to Jonathan Bedard from comment #1) > Created attachment 343766 [details] > Test order to reproduce after further testing it turns out if these tests are performed in the same run (in any order) the test will fail. When any one of tests is removed it will pass. http/wpt/service-workers/persistent-importScripts.html http/wpt/cache-storage/cache-put-keys.https.any.worker.html imported/w3c/web-platform-tests/IndexedDB/close-in-upgradeneeded.html sample output: Regressions: Unexpected text-only failures (1) imported/w3c/web-platform-tests/IndexedDB/close-in-upgradeneeded.html [ Failure ]
Given that we haven't seen this until recently, I would suspect that <https://trac.webkit.org/changeset/232516/webkit> is the root cause.
<rdar://problem/41626526>
Pages and contexts that use IDB (Documents, Workers, etc) do not recover well from a StorageProcess crash. They don't crash, but they're unaware that the StorageProcess is gone and therefore their messages to do IDB stuff are lost in the ether and never answered. I have a fix to make Pages recover from this and send contexts into a mode where requests after the storage process crash will at least error out immediately.
Created attachment 344354 [details] Patch
Haven't even finished building this patch locally, but it should be final or near final. Trying EWS.
(whoops forgot test expectations)
Created attachment 344359 [details] Patch
Comment on attachment 344359 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=344359&action=review > Source/WebCore/Modules/indexeddb/client/IDBConnectionToServer.cpp:67 > +void IDBConnectionToServer::callResultFunctionLater(ResultFunction function, const IDBResourceIdentifier& requestIdentifier) I think this name should indicate that it will be called with an error. > Source/WebCore/Modules/indexeddb/client/IDBConnectionToServer.cpp:364 > + callOnMainThread([this, protectedThis = makeRef(*this), transactionIdentifier] { Why can't these use callResultFunctionLater? > Source/WebCore/Modules/indexeddb/client/IDBConnectionToServer.cpp:502 > + if (m_serverConnectionIsValid) > + m_delegate->abortOpenAndUpgradeNeeded(databaseConnectionIdentifier, transactionIdentifier); Couldn't the delegate check if its connection is valid instead of all these checks here? > LayoutTests/storage/indexeddb/modern/opendatabase-after-storage-crash.html:49 > + // Padding it makes this reliable. how reliable?
(In reply to Alex Christensen from comment #10) > Comment on attachment 344359 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=344359&action=review > > > Source/WebCore/Modules/indexeddb/client/IDBConnectionToServer.cpp:67 > > +void IDBConnectionToServer::callResultFunctionLater(ResultFunction function, const IDBResourceIdentifier& requestIdentifier) > > I think this name should indicate that it will be called with an error. Sure. > > > Source/WebCore/Modules/indexeddb/client/IDBConnectionToServer.cpp:364 > > + callOnMainThread([this, protectedThis = makeRef(*this), transactionIdentifier] { > > Why can't these use callResultFunctionLater? Because they have custom call signatures, not IDBResults. I played with full lambdas, full templetization, and mixes - This was the cleanest/most compact form. > > > Source/WebCore/Modules/indexeddb/client/IDBConnectionToServer.cpp:502 > > + if (m_serverConnectionIsValid) > > + m_delegate->abortOpenAndUpgradeNeeded(databaseConnectionIdentifier, transactionIdentifier); > > Couldn't the delegate check if its connection is valid instead of all these > checks here? That'd require a patch over twice as large, as we'd have to cover 2 delegates (WK1 and WK2) > > > LayoutTests/storage/indexeddb/modern/opendatabase-after-storage-crash.html:49 > > + // Padding it makes this reliable. > > how reliable? 100% reliable™
Created attachment 344362 [details] Patch
Comment on attachment 344362 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=344362&action=review > Source/WebCore/Modules/indexeddb/client/IDBConnectionToServer.cpp:67 > +void IDBConnectionToServer::callResultFunctionWithErrorLater(ResultFunction function, const IDBResourceIdentifier& requestIdentifier) It could be made more compact by passing the const IDBRequestData&
(In reply to Alex Christensen from comment #13) > Comment on attachment 344362 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=344362&action=review > > > Source/WebCore/Modules/indexeddb/client/IDBConnectionToServer.cpp:67 > > +void IDBConnectionToServer::callResultFunctionWithErrorLater(ResultFunction function, const IDBResourceIdentifier& requestIdentifier) > > It could be made more compact by passing the const IDBRequestData& Fewer characters in the signature, more in the function body. /me shrugs
Comment on attachment 344362 [details] Patch Attachment 344362 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/8449520 New failing tests: http/tests/security/contentSecurityPolicy/userAgentShadowDOM/allow-video.html http/tests/security/local-video-source-from-remote.html http/tests/preload/onload_event.html
Created attachment 344375 [details] Archive of layout-test-results from ews204 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews204 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
The commit queue is in quite a state right now, that's for sure.
The EWS bots were clearly just in a bad state... let's try this again.
Created attachment 344397 [details] Patch for EWS/Landing
Comment on attachment 344397 [details] Patch for EWS/Landing Attachment 344397 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/8452509 New failing tests: http/tests/security/video-poster-cross-origin-crash2.html
Created attachment 344401 [details] Archive of layout-test-results from ews204 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews204 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Nope this patch did not cause that failurw
Comment on attachment 344397 [details] Patch for EWS/Landing Clearing flags on attachment: 344397 Committed r233562: <https://trac.webkit.org/changeset/233562>
All reviewed patches have been landed. Closing bug.
It looks like the test storage/indexeddb/modern/opendatabase-after-storage-crash.html is failing both with timeouts and text failures test history: https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=storage%2Findexeddb%2Fmodern%2Fopendatabase-after-storage-crash.html
UPDATE: It looks like the test storage/indexeddb/modern/opendatabase-after-storage-crash.html is still a flakey failure, though the timeouts seem to be largely resolved. test history: https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=storage%2Findexeddb%2Fmodern%2Fopendatabase-after-storage-crash.html
The test is still failing about 50% of the time, tracked in bug 187648.