IndexedDB: Error codes, phase two
Created attachment 146655 [details] Patch
jsbell@ / dgrogan@ - sanity check? This is a few small things that span a lot of files. They're all kind of interconnected, and would be a mess to try to decouple: - fix tests that try to create transactions in the setversion transaction (removal of that, along with a patch that verifies that you CANNOT do that, is coming up when this lands) - removes bit about I/O errors and error codes - spec now says UNKNOWN_ERR is correct - moves some readonly checks into the frontend, and puts an assert in the backend - this helps reduce RPCs as well as consolidates error checks (otherwise, trying to implement which exception takes precedence over other exceptions is near-impossible)
Created attachment 146899 [details] Patch
Comment on attachment 146899 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=146899&action=review Overall lgtm, just nits. > Source/WebCore/ChangeLog:17 > + already correct. Need to close the parentheses. > Source/WebCore/Modules/indexeddb/IDBCursor.cpp:209 > + ec = IDBDatabaseException::READ_ONLY_ERR; FYI, in one of my in-progress patches I added IDBTransaction::isVersionChange() as a convenience. Maybe add IDBTransaction::isReadOnly() ? > LayoutTests/storage/indexeddb/mozilla/index-prev-no-duplicate.html:85 > +function createObjectStore(request) The onsuccess callback parameter is an event, not a request. This is also not necessary as the request object from line 80 is global, and the eval in line 89 has a global context - the parameter would not be visible to it anyway. > LayoutTests/storage/indexeddb/mozilla/index-prev-no-duplicate.html:107 > + } Should have a ; after the brace. > LayoutTests/storage/indexeddb/resources/cursor-advance.js:51 > +function createObjectStore(request) Ditto.
Created attachment 146955 [details] Patch
tony@ - r? cq?
Comment on attachment 146955 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=146955&action=review > Source/WebCore/Modules/indexeddb/IDBTransaction.h:73 > + bool isReadOnly() const { return m_mode == READ_ONLY; }; Nit: The semicolon at the end isn't necessary. > Source/WebCore/Modules/indexeddb/IDBTransactionBackendImpl.h:79 > + const unsigned short m_mode; Nit: Can we change this from unsigned short to Mode in a follow up change?
Created attachment 147114 [details] Patch
Comment on attachment 147114 [details] Patch tony@ or jsbell@ - cq? (nit addressed - we're cleaning up the whole mode thing over a series of patches, and I'll try to make the enum thing part of that)
Comment on attachment 147114 [details] Patch Rejecting attachment 147114 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1 ERROR: /mnt/git/webkit-commit-queue/LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: http://queues.webkit.org/results/12946582
Comment on attachment 147114 [details] Patch sorry tony@ - one more try? At least my committer paperwork is in the mail :)
(In reply to comment #10) > (From update of attachment 147114 [details]) > Rejecting attachment 147114 [details] from commit-queue. > > Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1 > > ERROR: /mnt/git/webkit-commit-queue/LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). > > Full output: http://queues.webkit.org/results/12946582 You should be able to use "webkit-patch land-safely" when you have r+ now. It will fill in the reviewer name, upload the patch and set cq?.
Comment on attachment 147114 [details] Patch Clearing flags on attachment: 147114 Committed r120114: <http://trac.webkit.org/changeset/120114>
All reviewed patches have been landed. Closing bug.