jsbell notes: View in context: https://bugs.webkit.org/attachment.cgi?id=186862&action=review >> Source/WebCore/Modules/indexeddb/IDBCursor.cpp:99 >> return direction; > > No need for the local variable any more here. And FYI the directionToString method is only ever called from here now; it was used when we supported both legacy numeric enum and new string values when called from script. Now that the numeric enum values are not supported from script we can drop the ExceptionCode argument from this method. But that can be done in a separate patch. >> Source/WebCore/Modules/indexeddb/IDBTransaction.cpp:125 >> return mode; > > No need for the local variable any more here. And FYI, same as above. We can drop the ExceptionCode param from modeToStrng now.
Created attachment 187010 [details] Patch
Jochen, perhaps you can take a look at this patch? Joshua says it's the right thing to do, so I hope the details are trivially reviewable. :) It doesn't (yet) apply; I'll upload it again for the bots once https://bugs.webkit.org/show_bug.cgi?id=109044 lands.
Comment on attachment 187010 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=187010&action=review > Source/WebCore/ChangeLog:9 > + make use of the ExceptionCode they accept. This patch drops the unused Nitpicking the description: the methods *did* use it, but all callsites ignored the result other than asserting because these methods are now only used with pre-validated data. > Source/WebCore/Modules/indexeddb/IDBCursor.cpp:-329 > - ec = TypeError; Please add ASSERT_NOT_REACHED() here. > Source/WebCore/Modules/indexeddb/IDBTransaction.cpp:-365 > - ec = TypeError; Please add ASSERT_NOT_REACHED() here.
(In reply to comment #2) > Jochen, perhaps you can take a look at this patch? Joshua says it's the right thing to do, so I hope the details are trivially reviewable. :) I'm Joshua and I approve this patch (with those ASSERT_NOT_REACHED additions). Thanks, Jochen, and thanks Mike for putting it together!
Created attachment 187296 [details] Patch
This should address Joshua's comments. Jochen, if I haven't exhausted your patience yet today, would you mind taking a look at this patch as well?
Comment on attachment 187296 [details] Patch ok
Comment on attachment 187296 [details] Patch Clearing flags on attachment: 187296 Committed r142356: <http://trac.webkit.org/changeset/142356>
All reviewed patches have been landed. Closing bug.