IndexedDB: Abort transactions because of leveldb errors part 3
Created attachment 177395 [details] Patch
Created attachment 177606 [details] Patch
Josh or Alec, could you take a look?
Comment on attachment 177606 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=177606&action=review > Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:252 > + int64_t databaseId, int64_t objectStoreId, int64_t indexId, bool& canAddKeys, Just remove wrapping? > Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:253 > + const IDBKey* primaryKey = 0, String* errorMessage = 0) Add WARN_UNUSED_RETURN > Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:282 > + int64_t databaseId, int64_t objectStoreId, int64_t indexId, Just remove wrapping? > Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:283 > + const IDBKey* indexKey, const IDBKey* primaryKey, bool& allowed) const Add WARN_UNUSED_RETURN > Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:306 > +static bool makeIndexWriters(PassRefPtr<IDBTransactionBackendImpl> transaction, IDBObjectStoreBackendImpl* objectStore, PassRefPtr<IDBKey> primaryKey, bool keyWasGenerated, const Vector<int64_t>& indexIds, const Vector<IDBObjectStoreBackendInterface::IndexKeys>& indexKeys, Vector<OwnPtr<IndexWriter> >* indexWriters, String* errorMessage, bool& completed) Add WARN_UNUSED_RETURN > Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:329 > + bool noLevelDBErrors = indexWriter->verifyIndexKeys(*objectStore->backingStore(), I'd avoid using the "leveldb" name here. um... backingStoreSuccess ? > Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:330 > + transaction->backingStoreTransaction(), objectStore->databaseId(), Just remove wrapping? > Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:368 > + bool completed = false; Maybe rename to consistencySuccess ? Or something that's more clearly the inverse of constraintError ? > Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:371 > + transaction->abort(IDBDatabaseError::create(IDBDatabaseException::UnknownError, "Internal error in makeIndexWriters")); I wouldn't name the internal method explicitly here, just as it's one more thing to keep in sync. Maybe "Backing store error updating index keys." ? > Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:448 > + bool completed = false; Rename to constraintError? > Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:449 > + bool noLevelDBErrors = makeIndexWriters(transaction, objectStore.get(), key, keyWasGenerated, *indexIds, *indexKeys, &indexWriters, &errorMessage, completed); Ditto re: "leveldb" > Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:451 > + callbacks->onError(IDBDatabaseError::create(IDBDatabaseException::UnknownError, "Internal error in makeIndexWriters.")); Same as above.
Comment on attachment 177606 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=177606&action=review >> Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:252 >> + int64_t databaseId, int64_t objectStoreId, int64_t indexId, bool& canAddKeys, > > Just remove wrapping? check-webkit-style complained about the number of spaces. Do you know how to appease it? >> Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:253 >> + const IDBKey* primaryKey = 0, String* errorMessage = 0) > > Add WARN_UNUSED_RETURN Done >> Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:283 >> + const IDBKey* indexKey, const IDBKey* primaryKey, bool& allowed) const > > Add WARN_UNUSED_RETURN Done. >> Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:306 >> +static bool makeIndexWriters(PassRefPtr<IDBTransactionBackendImpl> transaction, IDBObjectStoreBackendImpl* objectStore, PassRefPtr<IDBKey> primaryKey, bool keyWasGenerated, const Vector<int64_t>& indexIds, const Vector<IDBObjectStoreBackendInterface::IndexKeys>& indexKeys, Vector<OwnPtr<IndexWriter> >* indexWriters, String* errorMessage, bool& completed) > > Add WARN_UNUSED_RETURN Done. Apparently it needs to go before the function here, maybe because of the static? >> Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:329 >> + bool noLevelDBErrors = indexWriter->verifyIndexKeys(*objectStore->backingStore(), > > I'd avoid using the "leveldb" name here. um... backingStoreSuccess ? Better, changed. >> Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:368 >> + bool completed = false; > > Maybe rename to consistencySuccess ? Or something that's more clearly the inverse of constraintError ? I went with obeysConstraints >> Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:371 >> + transaction->abort(IDBDatabaseError::create(IDBDatabaseException::UnknownError, "Internal error in makeIndexWriters")); > > I wouldn't name the internal method explicitly here, just as it's one more thing to keep in sync. Maybe "Backing store error updating index keys." ? Done. >> Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:448 >> + bool completed = false; > > Rename to constraintError? went with obeysConstraints >> Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:449 >> + bool noLevelDBErrors = makeIndexWriters(transaction, objectStore.get(), key, keyWasGenerated, *indexIds, *indexKeys, &indexWriters, &errorMessage, completed); > > Ditto re: "leveldb" done. >> Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:451 >> + callbacks->onError(IDBDatabaseError::create(IDBDatabaseException::UnknownError, "Internal error in makeIndexWriters.")); > > Same as above. Done.
Created attachment 177651 [details] Patch
Josh, I'm assuming the lack of lgtm means you want to review this again before I send it off to Tony. (Is that a generally accurate interpretation?)
(In reply to comment #7) > Josh, I'm assuming the lack of lgtm means you want to review this again before I send it off to Tony. (Is that a generally accurate interpretation?) Generally yeah but I just forgot. LGTM!
Tony, could you review this?
Comment on attachment 177651 [details] Patch Rejecting attachment 177651 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: set 108 lines). Hunk #2 succeeded at 387 (offset 108 lines). Hunk #3 succeeded at 411 (offset 108 lines). Hunk #4 succeeded at 433 (offset 108 lines). Hunk #5 succeeded at 473 (offset 108 lines). Hunk #6 FAILED at 445. 1 out of 6 hunks FAILED -- saving rejects to file Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp.rej Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Tony Chang']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue Full output: http://queues.webkit.org/results/15173025
Created attachment 177849 [details] Patch for landing
Comment on attachment 177849 [details] Patch for landing Clearing flags on attachment: 177849 Committed r136775: <http://trac.webkit.org/changeset/136775>
All reviewed patches have been landed. Closing bug.