Bug 28872

Summary:

[V8] SQLStatement Error Callback may not return the correct result to WebCore

Product:

WebKit

Reporter:

Ben Murdoch <benm>

Component:

WebCore Misc.

Assignee:

Ben Murdoch <benm>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

dglazkov, eric

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Proposed patch	eric: review-
Proposed patch with layout test.	none

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-	Details Formatted Diff Diff
Proposed patch with layout test. (6.45 KB, patch) 2009-09-01 10:49 PDT, Ben Murdoch	no flags	Details Formatted Diff Diff
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.