Move basic IDBBackingStoreTransaction operations to IDBServerConnection Important step towards completely abstracting away the BackingStoreTransaction from the frontend.
Created attachment 216739 [details] Patch v1
Comment on attachment 216739 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=216739&action=review > Source/WebCore/Modules/indexeddb/IDBTransactionBackend.cpp:70 > + m_database->serverConnection().openTransaction(id, objectStoreIds, mode, [self, this](bool success) { No need to capture ‘this’ here, just call everything on self. (Also, I think you should call self “backend” or something that isn’t a reserved keyword). > Source/WebCore/Modules/indexeddb/IDBTransactionBackend.cpp:238 > + m_database->serverConnection().commitTransaction(m_id, [self, this, committed, unused](bool success) mutable { Ditto. > Source/WebCore/Modules/indexeddb/leveldb/IDBServerConnectionLevelDB.cpp:87 > + successCallback(true); I think that it’s weird that all these immediately call the completion callback instead of doing it asynchronously. > Source/WebCore/Modules/indexeddb/leveldb/IDBServerConnectionLevelDB.cpp:92 > + RefPtr<IDBBackingStoreTransactionLevelDB> transaction = m_backingStoreTransactions.get(transactionID); Where is this transaction removed from the map? > Source/WebCore/Modules/indexeddb/leveldb/IDBServerConnectionLevelDB.cpp:96 > + completionCallback(); I think that it’s weird that all these immediately call the completion callback instead of doing it asynchronously. > Source/WebCore/Modules/indexeddb/leveldb/IDBServerConnectionLevelDB.cpp:101 > + RefPtr<IDBBackingStoreTransactionLevelDB> transaction = m_backingStoreTransactions.get(transactionID); Where is this transaction removed from the map? > Source/WebCore/Modules/indexeddb/leveldb/IDBServerConnectionLevelDB.cpp:104 > + successCallback(transaction->commit()); I think that it’s weird that all these immediately call the completion callback instead of doing it asynchronously. > Source/WebCore/Modules/indexeddb/leveldb/IDBServerConnectionLevelDB.cpp:109 > + RefPtr<IDBBackingStoreTransactionLevelDB> transaction = m_backingStoreTransactions.get(transactionID); Where is this transaction removed from the map? > Source/WebCore/Modules/indexeddb/leveldb/IDBServerConnectionLevelDB.cpp:113 > + completionCallback(); I think that it’s weird that all these immediately call the completion callback instead of doing it asynchronously. > Source/WebCore/Modules/indexeddb/leveldb/IDBServerConnectionLevelDB.cpp:118 > + RefPtr<IDBBackingStoreTransactionLevelDB> transaction = m_backingStoreTransactions.get(transactionID); Where is this transaction removed from the map? > Source/WebCore/Modules/indexeddb/leveldb/IDBServerConnectionLevelDB.cpp:122 > + completionCallback(); Ditto.
Comment on attachment 216739 [details] Patch v1 Attachment 216739 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/22539795
(In reply to comment #2) > (From update of attachment 216739 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=216739&action=review > > > Source/WebCore/Modules/indexeddb/IDBTransactionBackend.cpp:70 > > + m_database->serverConnection().openTransaction(id, objectStoreIds, mode, [self, this](bool success) { > > No need to capture ‘this’ here, just call everything on self. (Also, I think you should call self “backend” or something that isn’t a reserved keyword). > > > Source/WebCore/Modules/indexeddb/IDBTransactionBackend.cpp:238 > > + m_database->serverConnection().commitTransaction(m_id, [self, this, committed, unused](bool success) mutable { > > Ditto. Okay! > > Source/WebCore/Modules/indexeddb/leveldb/IDBServerConnectionLevelDB.cpp:87 > > + successCallback(true); > > I think that it’s weird that all these immediately call the completion callback instead of doing it asynchronously. > > Source/WebCore/Modules/indexeddb/leveldb/IDBServerConnectionLevelDB.cpp:96 > > + completionCallback(); > > I think that it’s weird that all these immediately call the completion callback instead of doing it asynchronously. > > > Source/WebCore/Modules/indexeddb/leveldb/IDBServerConnectionLevelDB.cpp:104 > > + successCallback(transaction->commit()); > > I think that it’s weird that all these immediately call the completion callback instead of doing it asynchronously. > > > Source/WebCore/Modules/indexeddb/leveldb/IDBServerConnectionLevelDB.cpp:113 > > + completionCallback(); > > I think that it’s weird that all these immediately call the completion callback instead of doing it asynchronously. > > > Source/WebCore/Modules/indexeddb/leveldb/IDBServerConnectionLevelDB.cpp:122 > > + completionCallback(); > > Ditto. Per IRC convo, I'll add some callOnMainThread() action here to make the callbacks asynchronous. > > Source/WebCore/Modules/indexeddb/leveldb/IDBServerConnectionLevelDB.cpp:92 > > + RefPtr<IDBBackingStoreTransactionLevelDB> transaction = m_backingStoreTransactions.get(transactionID); > > Where is this transaction removed from the map? > > Source/WebCore/Modules/indexeddb/leveldb/IDBServerConnectionLevelDB.cpp:101 > > + RefPtr<IDBBackingStoreTransactionLevelDB> transaction = m_backingStoreTransactions.get(transactionID); > > Where is this transaction removed from the map? > > Source/WebCore/Modules/indexeddb/leveldb/IDBServerConnectionLevelDB.cpp:109 > > + RefPtr<IDBBackingStoreTransactionLevelDB> transaction = m_backingStoreTransactions.get(transactionID); > > Where is this transaction removed from the map? > > Source/WebCore/Modules/indexeddb/leveldb/IDBServerConnectionLevelDB.cpp:118 > > + RefPtr<IDBBackingStoreTransactionLevelDB> transaction = m_backingStoreTransactions.get(transactionID); > > Where is this transaction removed from the map? In removeBackingStoreTransaction() which isn't implemented, and that's probably what the efl/gtk bots are complaining about. *slaps forehead* Fix coming
Created attachment 216744 [details] Patch v2
Attachment 216744 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/Modules/indexeddb/IDBBackingStoreInterface.h', u'Source/WebCore/Modules/indexeddb/IDBDatabaseBackend.cpp', u'Source/WebCore/Modules/indexeddb/IDBServerConnection.h', u'Source/WebCore/Modules/indexeddb/IDBTransactionBackend.cpp', u'Source/WebCore/Modules/indexeddb/IDBTransactionBackend.h', u'Source/WebCore/Modules/indexeddb/IDBTransactionBackendOperations.cpp', u'Source/WebCore/Modules/indexeddb/leveldb/IDBBackingStoreCursorLevelDB.cpp', u'Source/WebCore/Modules/indexeddb/leveldb/IDBBackingStoreCursorLevelDB.h', u'Source/WebCore/Modules/indexeddb/leveldb/IDBBackingStoreLevelDB.cpp', u'Source/WebCore/Modules/indexeddb/leveldb/IDBBackingStoreLevelDB.h', u'Source/WebCore/Modules/indexeddb/leveldb/IDBBackingStoreTransactionLevelDB.cpp', u'Source/WebCore/Modules/indexeddb/leveldb/IDBBackingStoreTransactionLevelDB.h', u'Source/WebCore/Modules/indexeddb/leveldb/IDBFactoryBackendLevelDB.cpp', u'Source/WebCore/Modules/indexeddb/leveldb/IDBFactoryBackendLevelDB.h', u'Source/WebCore/Modules/indexeddb/leveldb/IDBLevelDBCoding.cpp', u'Source/WebCore/Modules/indexeddb/leveldb/IDBLevelDBCoding.h', u'Source/WebCore/Modules/indexeddb/leveldb/IDBServerConnectionLevelDB.cpp', u'Source/WebCore/Modules/indexeddb/leveldb/IDBServerConnectionLevelDB.h', u'Source/WebCore/platform/DatabaseStrategy.cpp', u'Source/WebCore/platform/leveldb/LevelDBComparator.h', u'Source/WebCore/platform/leveldb/LevelDBDatabase.h', u'Source/WebCore/platform/leveldb/LevelDBIterator.h', u'Source/WebCore/platform/leveldb/LevelDBSlice.h', u'Source/WebCore/platform/leveldb/LevelDBTransaction.h', u'Source/WebCore/platform/leveldb/LevelDBWriteBatch.h']" exit_code: 1 Source/WebCore/Modules/indexeddb/leveldb/IDBServerConnectionLevelDB.cpp:83: More than one command on the same line [whitespace/newline] [4] Source/WebCore/Modules/indexeddb/leveldb/IDBServerConnectionLevelDB.cpp:88: More than one command on the same line [whitespace/newline] [4] Source/WebCore/Modules/indexeddb/leveldb/IDBServerConnectionLevelDB.cpp:106: More than one command on the same line [whitespace/newline] [4] Total errors found: 3 in 26 files If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #6) > Attachment 216744 [details] did not pass style-queue: > > Source/WebCore/Modules/indexeddb/leveldb/IDBServerConnectionLevelDB.cpp:83: More than one command on the same line [whitespace/newline] [4] > Source/WebCore/Modules/indexeddb/leveldb/IDBServerConnectionLevelDB.cpp:88: More than one command on the same line [whitespace/newline] [4] > Source/WebCore/Modules/indexeddb/leveldb/IDBServerConnectionLevelDB.cpp:106: More than one command on the same line [whitespace/newline] [4] > Total errors found: 3 in 26 files Don't like this complaint... but fixed locally.
Whoops, this patch has bogus testing-on-mac code changes. ignore.
Created attachment 216745 [details] Patch v3
Comment on attachment 216745 [details] Patch v3 Attachment 216745 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/22369803
Created attachment 216747 [details] Patch v4
I forgot to address the self/this feedback - Have done so locally.
http://trac.webkit.org/changeset/159177