IndexedDB: Remove unused error handling clauses when writing to transaction
Created attachment 171315 [details] Patch
alecflett@, dgrogan@ - what do you think? This simplifies some of the code in IDBLevelDBBackingStore but it's not a significant win overall. I also worry that if we ever let transactions spill to disk we'd need to resurrect the error handlers.
Comment on attachment 171315 [details] Patch Attachment 171315 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14627288
Comment on attachment 171315 [details] Patch Attachment 171315 [details] did not pass cr-android-ews (chromium-android): Output: http://queues.webkit.org/results/14618878
Created attachment 171486 [details] Patch
New patch: * Removes unused non-template functions (which failed compile) * static function deleteRange() only writes to transaction so can be void * Made setUpMetadata, getNewDatabaseId, and createIDBDatabaseMetaData write via transactions - now all writing is via transactions, which simplifies error cases. This should make webkit.org/b/94861 and related cases where a file system error occurs easier handle properly. So... I'm now in favor of going forward with this. alecflett@, dgrogan@ - thoughts?
Comment on attachment 171486 [details] Patch Attachment 171486 [details] did not pass cr-android-ews (chromium-android): Output: http://queues.webkit.org/results/14632595
Created attachment 171492 [details] Patch
Comment on attachment 171492 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=171492&action=review LGTM > Source/WebCore/ChangeLog:15 > + (WebCore::putBool): Fork into "put to database" vs. "put to transaction" variants Seems like "put to database" was removed altogether?
Comment on attachment 171492 [details] Patch generally lgtm, one minor nit: View in context: https://bugs.webkit.org/attachment.cgi?id=171492&action=review > Source/WebCore/Modules/indexeddb/IDBLevelDBBackingStore.cpp:972 > + return true; Is it worth simplifying this return path too?
Comment on attachment 171492 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=171492&action=review >> Source/WebCore/ChangeLog:15 >> + (WebCore::putBool): Fork into "put to database" vs. "put to transaction" variants > > Seems like "put to database" was removed altogether? Yeah, older iteration. Comment updated. >> Source/WebCore/Modules/indexeddb/IDBLevelDBBackingStore.cpp:972 >> + return true; > > Is it worth simplifying this return path too? That API is exposed in IDBBackingStore so I left it alone for this first pass.
Created attachment 172443 [details] Patch
tony@ - r?
dglazkov@ - tony's out, would you mind r?
Created attachment 173098 [details] Patch for landing
Comment on attachment 173098 [details] Patch for landing Clearing flags on attachment: 173098 Committed r133940: <http://trac.webkit.org/changeset/133940>
All reviewed patches have been landed. Closing bug.