Bug 215271 – [ macOS iOS wk 2 Debug ] storage/indexeddb/modern/new-database-after-user-delete.html is a flaky failure

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Hector Lopez]

storage/indexeddb/modern/new-database-after-user-delete.html

This test is a flaky failure according to history. First occurrence of failure on iOS is at r259341. First occurrence of failure for macOS is at r251207.

History:
https://results.webkit.org/?suite=layout-tests&test=storage%2Findexeddb%2Fmodern%2Fnew-database-after-user-delete.html&platform=ios&platform=mac&limit=50000&style=debug

Diff:
--- /Volumes/Data/slave/ipados-simulator-13-debug-tests-wk2/build/layout-test-results/storage/indexeddb/modern/new-database-after-user-delete-expected.txt
+++ /Volumes/Data/slave/ipados-simulator-13-debug-tests-wk2/build/layout-test-results/storage/indexeddb/modern/new-database-after-user-delete-actual.txt
@@ -13,6 +13,7 @@
 Version change transaction completed. Version is now 1. About to request clearAllDatabases
 Requested clearAllDatabases
 Database connection error'ed out. Opening a new connection.
+Database connection error'ed out. Opening a new connection.
 Reopened upgrade needed: Old version - 0 New version - 1
 [PASS] The database has no object stores, meaning it is new
 PASS successfullyParsed is true

[Radar WebKit Bug Importer]

<rdar://problem/66683545>

[Hector Lopez]

Test expectations while investigated:

https://trac.webkit.org/changeset/265374/webkit

[Cameron McCormack (:heycam)]

*** Bug 204489 has been marked as a duplicate of this bug. ***

[Cameron McCormack (:heycam)]

*** Bug 204489 has been marked as a duplicate of this bug. ***

[Cameron McCormack (:heycam)]

Created attachment 454805 [details]
Patch

[Sihui Liu]

Comment on attachment 454805 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=454805&action=review

r=me

> Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp:1460
> +    ASSERT(!m_versionChangeDatabaseConnection || openDatabaseConnections.contains(m_versionChangeDatabaseConnection.get()));
> +    m_versionChangeDatabaseConnection = nullptr;

hmm, the mac-AS-debug-wk2 seems related.

oh I see, there is still a case where we don't add m_versionChangeDatabaseConnection to m_openDatabaseConnections synchronously:
in UniqueIDBDatabase::performCurrentOpenOperationAfterSpaceCheck and when it falls to maybeNotifyConnectionsOfVersionChange() -- we would add it to the set after events are dispatched to existing open connections.

Let's change this to check if openDatabaseConnections has m_versionChangeDatabaseConnection.get(), and close it if it's not in the set and has not been closed.

[Sihui Liu]

(In reply to Sihui Liu from comment #6)
> Comment on attachment 454805 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=454805&action=review
> 
> r=me

(Sorry was about to to say r=me after the change)

> 
> > Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp:1460
> > +    ASSERT(!m_versionChangeDatabaseConnection || openDatabaseConnections.contains(m_versionChangeDatabaseConnection.get()));
> > +    m_versionChangeDatabaseConnection = nullptr;
> 
> hmm, the mac-AS-debug-wk2 seems related.
> 
> oh I see, there is still a case where we don't add
> m_versionChangeDatabaseConnection to m_openDatabaseConnections synchronously:
> in UniqueIDBDatabase::performCurrentOpenOperationAfterSpaceCheck and when it
> falls to maybeNotifyConnectionsOfVersionChange() -- we would add it to the
> set after events are dispatched to existing open connections.
> 
> Let's change this to check if openDatabaseConnections has
> m_versionChangeDatabaseConnection.get(), and close it if it's not in the set
> and has not been closed.

[Cameron McCormack (:heycam)]

Created attachment 454902 [details]
Patch

[Sihui Liu]

Comment on attachment 454902 [details]
Patch

r=me, let's wait for bots to be green before landing

[EWS]

Committed r291435 (248559@main): <https://commits.webkit.org/248559@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 454902 [details].