RESOLVED FIXED Bug 28872
[V8] SQLStatement Error Callback may not return the correct result to WebCore 
https://bugs.webkit.org/show_bug.cgi?id=28872
Summary [V8] SQLStatement Error Callback may not return the correct result to WebCore
Ben Murdoch
Reported 2009-09-01 06:05:28 PDT
In the custom V8 binding for SQLStatementErrorCallback, handleEvent() may not return the correct result to WebCore. It must return true to WebCore to signify that the transaction error steps should be executed in the event of the statement error callback throwing an exception or returning true.
Attachments
Proposed patch (1.55 KB, patch)
2009-09-01 06:29 PDT, Ben Murdoch 		eric: review-
DetailsFormatted DiffDiff
Proposed patch with layout test. (6.45 KB, patch)
2009-09-01 10:49 PDT, Ben Murdoch 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Ben Murdoch
Comment 1 2009-09-01 06:29:51 PDT
Created attachment 38863 [details] Proposed patch Corrects the return logic in handleEvent().
Eric Seidel (no email)
Comment 2 2009-09-01 07:39:48 PDT
Comment on attachment 38863 [details] Proposed patch Is this testable? How does this bug manifest itself to the user/page? We either need a test or an explanation of why it's untestable. :)
Ben Murdoch
Comment 3 2009-09-01 07:47:13 PDT
(In reply to comment #2) > (From update of attachment 38863 [details]) > Is this testable? How does this bug manifest itself to the user/page? We > either need a test or an explanation of why it's untestable. :) I should be able to put together a layout test. Essentially, without this fix the wrong result is returned to WebCore in the event that the statement error callback does not throw an exception and returns true. In this situation the binding will currently returns false to WebCore, signifying that processing of the next statement may begin. The spec states that if the statement callback returns true we should jump immediately to the transaction error callback.
Eric Seidel (no email)
Comment 4 2009-09-01 07:55:45 PDT
Perfect. If we layout test this then other implementations will be sure not to get this wrong. :)
Ben Murdoch
Comment 5 2009-09-01 10:49:01 PDT
Created attachment 38870 [details] Proposed patch with layout test.
Eric Seidel (no email)
Comment 6 2009-09-03 01:46:58 PDT
Comment on attachment 38870 [details] Proposed patch with layout test. LGTM.
Eric Seidel (no email)
Comment 7 2009-09-03 02:04:42 PDT
Comment on attachment 38870 [details] Proposed patch with layout test. Clearing flags on attachment: 38870 Committed r48008: <http://trac.webkit.org/changeset/48008>
Eric Seidel (no email)
Comment 8 2009-09-03 02:04:46 PDT
All reviewed patches have been landed. Closing bug.
Eric Seidel (no email)
Comment 9 2009-11-18 16:06:51 PST
Please don't remove the Reviewed by NOBODY(OOPS!) line, or our scripts won't correctly add the reviewer when landing. :(
Note You need to log in before you can comment on or make changes to this bug.