IndexedDB: Allow createIndex/createObjectStore to be asynchronous
Created attachment 181784 [details] Patch
Comment on attachment 181784 [details] Patch oops, forgot one file
Created attachment 181791 [details] Patch
jsbell@ / dgrogan@ - one more big review? Lots of code removal in this one :)
Comment on attachment 181791 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=181791&action=review Overall LGTM... > Source/WebCore/Modules/indexeddb/IDBBackingStore.cpp:640 > + return false; This changes the semantics of this method from "plow ahead in case of error" to "fail the load completely". I don't disagree with this change, but it's orthogonal to the rest of the patch, isn't it? (Also, I'd rather that we just implemented the FIXME if we're going to change the behavior.) > Source/WebCore/Modules/indexeddb/IDBBackingStore.cpp:1017 > + return false; Ditto. > Source/WebCore/Modules/indexeddb/IDBBackingStore.cpp:1037 > return indexes; Should be return true ? > Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.cpp:56 > + const RefPtr<IDBBackingStore> m_backingStore; Not for this patch (because it's consistent with other Operations), but given that we can get to the backing store via the transaction which is passed into perform(), we can probably eliminate this member from this and other Operations. > Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.cpp:1347 > + m_metadata.objectStores.clear(); Is this ever called with non-empty m_metadata.objectStores? > Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.h:108 > class VersionChangeOperation; Remove this from IDBDatabaseBackendImpl class while the others are moving out too? > Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.h:111 > class VersionChangeAbortOperation; Ditto? > Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.h:46 > +namespace IDBObjectStoreBackendImpl { May want to add a FIXME indicating that this is temporary, and things in this namespace will move to ... ? > Source/WebKit/chromium/tests/IDBDatabaseBackendTest.cpp:-50 > -TEST(IDBDatabaseBackendTest, BackingStoreRetention) Any way to change this test rather than removing it? E.g. creating an IDBTransactionBackendImpl which holds a reference?
Comment on attachment 181791 [details] Patch Attachment 181791 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/15763293
Comment on attachment 181791 [details] Patch Attachment 181791 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/15764284
Created attachment 182026 [details] Patch
showed the updated IDBDatabaseBackendTest to jsbell in person he sez "lgtm" All other issues have been addressed. tony@ - r?
Comment on attachment 182026 [details] Patch Attachment 182026 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/15761681
Comment on attachment 182026 [details] Patch Attachment 182026 [details] did not pass cr-android-ews (chromium-android): Output: http://queues.webkit.org/results/15762748
Created attachment 182037 [details] Patch
Created attachment 182040 [details] Patch
Comment on attachment 182040 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=182040&action=review > Source/WebCore/ChangeLog:35 > + * Modules/indexeddb/IDBDatabaseBackendImpl.cpp: Add all create/delete operations. Nit: I would say where these are copied from. > Source/WebCore/Modules/indexeddb/IDBBackingStore.cpp:608 > +// FIXME: This should do some error handling rather than plowing ahead when bad data is encountered. Are there security implications of plowing ahead when bad data is encountered? > Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.h:69 > + virtual PassRefPtr<IDBObjectStoreBackendInterface> createObjectStore(int64_t id, const String& name, const IDBKeyPath&, bool autoIncrement, IDBTransactionBackendInterface*, ExceptionCode&) { ASSERT_NOT_REACHED(); return 0; } > + virtual void createObjectStore(int64_t transactionId, int64_t objectStoreId, const String& name, const IDBKeyPath&, bool autoIncrement); > + virtual void deleteObjectStore(int64_t, IDBTransactionBackendInterface*, ExceptionCode&) { ASSERT_NOT_REACHED(); } > + virtual void deleteObjectStore(int64_t transactionId, int64_t objectStoreId); Should these have OVERRIDE?
Comment on attachment 182040 [details] Patch Attachment 182040 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/15758795
Created attachment 182061 [details] Patch for landing
Comment on attachment 182061 [details] Patch for landing Rejecting attachment 182061 [details] from commit-queue. New failing tests: inspector-protocol/debugger-terminate-dedicated-worker-while-paused.html Full output: http://queues.webkit.org/results/15774678
Comment on attachment 182061 [details] Patch for landing sigh. Those are not my failures. trying again.
Comment on attachment 182061 [details] Patch for landing Clearing flags on attachment: 182061 Committed r139289: <http://trac.webkit.org/changeset/139289>
All reviewed patches have been landed. Closing bug.