According to the Web SQL Database spec and http://www.mail-archive.com/public-webapps@w3.org/msg06679.html, if a statement error callback returns 'undefined' (doesn't return anything), we should treat it as 'not false', and roll back the transaction. Currently we treat it as 'false'.
Created attachment 48944 [details] patch
Attachment 48944 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/bindings/v8/custom/V8CustomVoidCallback.cpp:101: Place brace on its own line for function definitions. [whitespace/braces] [4] WebCore/bindings/v8/custom/V8CustomVoidCallback.cpp:102: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] WebCore/bindings/v8/custom/V8CustomVoidCallback.cpp:103: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] WebCore/bindings/v8/custom/V8CustomVoidCallback.cpp:104: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] WebCore/bindings/v8/custom/V8CustomVoidCallback.cpp:105: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] WebCore/bindings/v8/custom/V8CustomVoidCallback.cpp:108: Place brace on its own line for function definitions. [whitespace/braces] [4] WebCore/bindings/v8/custom/V8CustomVoidCallback.cpp:109: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] WebCore/bindings/v8/custom/V8CustomVoidCallback.cpp:110: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] WebCore/bindings/v8/custom/V8CustomVoidCallback.cpp:111: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] WebCore/bindings/v8/custom/V8CustomVoidCallback.cpp:112: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 10 If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 48946 [details] patch
Comment on attachment 48946 [details] patch What about null? Should that still be false?
Created attachment 48953 [details] patch (In reply to comment #4) > (From update of attachment 48946 [details]) > What about null? Should that still be false? fixed. treating all non-boolean values as 'true' (since the spec seems to divide all return values into 'false' and 'everything else'). also added more test cases to storage/statement-error-callback.html to test all kind of non-boolean return types (including 'null').
Comment on attachment 48953 [details] patch > - return result.toBoolean(exec); > + return !result.isBoolean() || result.toBoolean(exec); While this will work, it's unnecessarily inefficient. The toBoolean function's job is to convert things from other types to boolean. The efficient way is to write: return result.isFalse(); The patch otherwise seems fine. r=me, but please make the fix I suggest above.
(In reply to comment #6) > (From update of attachment 48953 [details]) > > - return result.toBoolean(exec); > > + return !result.isBoolean() || result.toBoolean(exec); > > While this will work, it's unnecessarily inefficient. The toBoolean function's > job is to convert things from other types to boolean. The efficient way is to > write: > > return result.isFalse(); > > The patch otherwise seems fine. r=me, but please make the fix I suggest above. nice optimization! will change this in the JSC and V8 code before submitting.
Landed as r54981.