WebCore: Change the V8 and JSC SQLStatementErrorCallback to interpret
authordumi@chromium.org <dumi@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 18 Feb 2010 20:52:07 +0000 (20:52 +0000)
committerdumi@chromium.org <dumi@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 18 Feb 2010 20:52:07 +0000 (20:52 +0000)
'undefined' return values as 'true', as required by the spec.

Reviewed by Darin Adler.

https://bugs.webkit.org/show_bug.cgi?id=35048

* bindings/js/JSCustomSQLStatementErrorCallback.cpp:
(WebCore::JSCustomSQLStatementErrorCallback::handleEvent):
* bindings/v8/custom/V8CustomSQLStatementErrorCallback.cpp:
(WebCore::V8CustomSQLStatementErrorCallback::handleEvent):
* bindings/v8/custom/V8CustomVoidCallback.cpp:
(WebCore::invokeCallbackHelper):
(WebCore::invokeCallback):
(WebCore::invokeCallbackTreatUndefinedAsTrue):
* bindings/v8/custom/V8CustomVoidCallback.h:

LayoutTests: 'undefined' return values from statement error callbacks are not
treated as 'true'. Fix the tests that did not use this
assumption.

Reviewed by NOBODY Darin Adler.

https://bugs.webkit.org/show_bug.cgi?id=35048

* storage/database-lock-after-reload.html:
* storage/private-browsing-readonly.html:
* storage/statement-error-callback.html:
* storage/statement-error-callback-expected.txt:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@54981 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/storage/database-lock-after-reload.html
LayoutTests/storage/private-browsing-readonly.html
LayoutTests/storage/statement-error-callback-expected.txt
LayoutTests/storage/statement-error-callback.html
WebCore/ChangeLog
WebCore/bindings/js/JSCustomSQLStatementErrorCallback.cpp
WebCore/bindings/v8/custom/V8CustomSQLStatementErrorCallback.cpp
WebCore/bindings/v8/custom/V8CustomVoidCallback.cpp
WebCore/bindings/v8/custom/V8CustomVoidCallback.h

index fa67e40324b63d8f4a5121e2e3e9fc35afe982dc..aed00d563fd07e678f1af0e0a3c98c9f66540a46 100644 (file)
@@ -1,3 +1,18 @@
+2010-02-17  Dumitru Daniliuc  <dumi@chromium.org>
+
+        Reviewed by NOBODY Darin Adler.
+
+        'undefined' return values from statement error callbacks are not
+        treated as 'true'. Fix the tests that did not use this
+        assumption.
+
+        https://bugs.webkit.org/show_bug.cgi?id=35048
+
+        * storage/database-lock-after-reload.html:
+        * storage/private-browsing-readonly.html:
+        * storage/statement-error-callback.html:
+        * storage/statement-error-callback-expected.txt:
+
 2010-02-17  Ojan Vafai  <ojan@chromium.org>
 
         Reviewed by Adam Barth.
index ad34d5bc74ce424868ea93c5bd3901b760d4bc80..8bdaddce26cf9783a0e452651e280a0db5e22679 100644 (file)
@@ -28,7 +28,7 @@ function addData(db)
         // Insert a large amount of data that will take a little while to run. Schedule a timout to run that will load a new page
         // whilst the transaction is still in progress, interrupting the transaction. This should not leave the database locked and on
         // the next page we should be able to insert some more data.
-        tx.executeSql("INSERT INTO DataTest (testData) VALUES (randomBlob(524200 ))", [], function(tx, result) { }, errorFunction);
+        tx.executeSql("INSERT INTO DataTest (testData) VALUES (ZEROBLOB(524200))", [], function(tx, result) { }, errorFunction);
         location.href = "./resources/database-lock-after-reload-2.html";
     }, errorFunction, function() { 
         finishTest();
index 70a209a473c2da3f9a62e2487f3fbdc5bdc7676d..972b123c095aad13a5cb4532423a4b1f0252c0d5 100644 (file)
@@ -46,6 +46,7 @@ function privateBrowsingErrorFunction(tx, error)
 {
     ++completed;
     writeMessageToLog("Private browsing statement " + completed + " completed with an error\n" + error.message);
+    return false;
 }
 
 function runSetup(transaction)
index ca117a8bca8b8191b8699c72c9727bf53061e27a..791dfb667b8a6bdc707416fba9563bf77b2dad39 100644 (file)
@@ -1,5 +1,10 @@
-CONSOLE MESSAGE: line 0: Exception in Statement error callback
-This test confirms that if the statement error callback returns true or throws an exception we do not execute any further statements in that transaction and instead execute the transaction error callback immediately.
+CONSOLE MESSAGE: line 0: Exception in statement error callback
+This test confirms that a transaction is immediately rolled back if and only if a statement's error callback throws an exception, returns true, or doesn't return any value.
+PASS - the transaction error callback was invoked.
+PASS - the transaction error callback was invoked.
+PASS - the transaction error callback was invoked.
+PASS - the transaction error callback was invoked.
+PASS - the transaction error callback was invoked.
 PASS - the transaction error callback was invoked.
 PASS - the transaction error callback was invoked.
 Test Complete
index 3675548d874debd21039c77bb545411848dd7d3d..060a881e81c91c174e28cd7e9eab7491a6fd9c38 100644 (file)
@@ -15,7 +15,8 @@ function finishTest()
 }
 
 var txCallbackCount = 0;
-var NUMBER_OF_TRANSACTIONS = 2;
+var NUMBER_OF_TRANSACTIONS = 7;
+var database;
 
 function transactionErrorFunction(error)
 {
@@ -31,6 +32,22 @@ function transactionSuccessFunction(message)
         finishTest();
 }
 
+function runTransactionExpectedToFail(statementErrorCallback)
+{
+    database.transaction(function(tx) {
+        tx.executeSql("CREATE TABLE IF NOT EXISTS StatementErrorCallbackTest (randomData)");
+        tx.executeSql("INSERT INTO StatementErrorCallbackTest (randomData) VALUES (?)", ['test']);
+        tx.executeSql("THIS STATEMENT WILL FAIL", [],
+            function(tx, data) {
+                log("FAIL - this statement should have failed");
+                finishTest();
+            }, statementErrorCallback);
+        tx.executeSql("INSERT INTO StatementErrorCallbackTest (randomData) VALUES (?)", ['test1'],
+            function(error) { log("FAIL - This statement should not have been executed"); },
+            function() { log("FAIL - This statement should not have been executed"); });
+    }, transactionErrorFunction, transactionSuccessFunction);
+}
+
 function runTest()
 {
     if (window.layoutTestController) {
@@ -38,29 +55,36 @@ function runTest()
         layoutTestController.dumpAsText();
         layoutTestController.waitUntilDone();
     }
-    
-    var database = openDatabase("bug-28872", "1.0", "statement error callback test", 1024);
-    
-    database.transaction(function(tx) {
-            tx.executeSql("CREATE TABLE IF NOT EXISTS StatementErrorCallbackTest (randomData)");
-            tx.executeSql("INSERT INTO StatementErrorCallbackTest (randomData) VALUES (?)", ['test']);
-            tx.executeSql("THIS STATEMENT WILL FAIL", [], function(message) { log("FAIL - this statement should have failed"); finishTest(); }, function(error) { return true; });
-            tx.executeSql("INSERT INTO StatementErrorCallbackTest (randomData) VALUES (?)", ['test1'], function(message) { log("FAIL - This statement should not have been executed"); }, function(message) { log("FAIL - This statement should not have been executed"); });
-        }, transactionErrorFunction, transactionSuccessFunction);
 
-    database.transaction(function(tx) {        
+    database = openDatabase("bug-28872", "1.0", "statement error callback test", 1024);
+    database.transaction(function(tx) {
         tx.executeSql("CREATE TABLE IF NOT EXISTS StatementErrorCallbackTest (randomData)");
-            tx.executeSql("INSERT INTO StatementErrorCallbackTest (randomData) VALUES (?)", ['test']);
-            tx.executeSql("THIS STATEMENT WILL FAIL", [], function(message) { log("FAIL - this statement should have failed"); finishTest(); }, function(error) { throw "Exception in Statement error callback"; return false; });
-            tx.executeSql("INSERT INTO StatementErrorCallbackTest (randomData) VALUES (?)", ['test1'], function(message) { log("FAIL - This statement should not have been executed"); }, function(message) { log("FAIL - This statement should not have been executed"); });
-        }, transactionErrorFunction, transactionSuccessFunction);
+        tx.executeSql("INSERT INTO StatementErrorCallbackTest (randomData) VALUES (?)", ['test']);
+        tx.executeSql("THIS STATEMENT WILL FAIL", [],
+            function(tx, data) {
+                log("FAIL - this statement should have failed");
+                finishTest();
+            }, function(tx, error) { return false; });
+        tx.executeSql("INSERT INTO StatementErrorCallbackTest (randomData) VALUES (?)", ['test1'],
+            function(tx, data) { },
+            function(tx, error) { log("FAIL - This statement should not have caused an error"); });
+    }, function(error) { log("FAIL - The transaction error callback should not have been invoked"); },
+    function() { });
+
+    runTransactionExpectedToFail(function(error) { return true; });
+    runTransactionExpectedToFail(function(error) { throw "Exception in statement error callback"; return false; });
+    runTransactionExpectedToFail(function(error) {});
+    runTransactionExpectedToFail(function(error) { return null; });
+    runTransactionExpectedToFail(function(error) { return "some string"; });
+    runTransactionExpectedToFail(function(error) { return 1234; });
+    runTransactionExpectedToFail(function(error) { return {a: 2, b: "abc"}; });
 }
 
 </script>
 </head>
 
 <body onload="runTest()">
-This test confirms that if the statement error callback returns true or throws an exception we do not execute any further statements in that transaction and instead execute the transaction error callback immediately.
+This test confirms that a transaction is immediately rolled back if and only if a statement's error callback throws an exception, returns true, or doesn't return any value.
 <pre id="console">
 </pre>
 </body>
index 93f07aeabab9bf0681f059032d3df366fbfd7698..a61b2463ff9f557cee234ae39e5420ae25e1580b 100644 (file)
@@ -1,3 +1,22 @@
+2010-02-17  Dumitru Daniliuc  <dumi@chromium.org>
+
+        Reviewed by Darin Adler.
+
+        Change the V8 and JSC SQLStatementErrorCallback to interpret
+        'undefined' return values as 'true', as required by the spec.
+
+        https://bugs.webkit.org/show_bug.cgi?id=35048
+
+        * bindings/js/JSCustomSQLStatementErrorCallback.cpp:
+        (WebCore::JSCustomSQLStatementErrorCallback::handleEvent):
+        * bindings/v8/custom/V8CustomSQLStatementErrorCallback.cpp:
+        (WebCore::V8CustomSQLStatementErrorCallback::handleEvent):
+        * bindings/v8/custom/V8CustomVoidCallback.cpp:
+        (WebCore::invokeCallbackHelper):
+        (WebCore::invokeCallback):
+        (WebCore::invokeCallbackTreatUndefinedAsTrue):
+        * bindings/v8/custom/V8CustomVoidCallback.h:
+
 2010-02-17  Ojan Vafai  <ojan@chromium.org>
 
         Reviewed by Adam Barth.
index 61785092bf571e9c30a53eb73d6d4fe340638e86..4d5de796503a69ed1b786484ab2debd037dbb0c3 100644 (file)
@@ -77,7 +77,7 @@ bool JSCustomSQLStatementErrorCallback::handleEvent(SQLTransaction* transaction,
         // Therefore an exception and returning true are the same thing - so, return true on an exception
         return true;
     }
-    return result.toBoolean(exec);
+    return !result.isFalse();
 }
 
 }
index 84a3b962e21113bb28e5d2c368b1721797861688..f733ede3d815be98489eceddc70ec7f91642e736 100644 (file)
@@ -75,10 +75,9 @@ bool V8CustomSQLStatementErrorCallback::handleEvent(SQLTransaction* transaction,
     // statement, if any, or onto the next overall step otherwise. Otherwise,
     // the error callback did not return false, or there was no error callback.
     // Jump to the last step in the overall steps.
-    return invokeCallback(m_callback, 2, argv, callbackReturnValue) || callbackReturnValue;
+    return invokeCallbackTreatOnlyExplicitFalseAsFalse(m_callback, 2, argv, callbackReturnValue) || callbackReturnValue;
 }
 
 } // namespace WebCore
 
 #endif
-
index 94cb10479c9a46da686b84bd4eee66dfc2f708e0..8c69e765e6d9e008ed4f443efe45312568b5adc0 100644 (file)
@@ -64,7 +64,7 @@ void V8CustomVoidCallback::handleEvent()
     invokeCallback(m_callback, 0, 0, callbackReturnValue);
 }
 
-bool invokeCallback(v8::Persistent<v8::Object> callback, int argc, v8::Handle<v8::Value> argv[], bool& callbackReturnValue)
+static bool invokeCallbackHelper(v8::Persistent<v8::Object> callback, int argc, v8::Handle<v8::Value> argv[], v8::Handle<v8::Value>& returnValue)
 {
     v8::TryCatch exceptionCatcher;
 
@@ -87,9 +87,7 @@ bool invokeCallback(v8::Persistent<v8::Object> callback, int argc, v8::Handle<v8
     V8Proxy* proxy = V8Proxy::retrieve();
     ASSERT(proxy);
 
-    v8::Handle<v8::Value> result = proxy->callFunction(callbackFunction, thisObject, argc, argv);
-
-    callbackReturnValue = !result.IsEmpty() && result->IsBoolean() && result->BooleanValue();
+    returnValue = proxy->callFunction(callbackFunction, thisObject, argc, argv);
 
     if (exceptionCatcher.HasCaught()) {
         v8::Local<v8::Message> message = exceptionCatcher.Message();
@@ -100,4 +98,20 @@ bool invokeCallback(v8::Persistent<v8::Object> callback, int argc, v8::Handle<v8
     return false;
 }
 
+bool invokeCallback(v8::Persistent<v8::Object> callback, int argc, v8::Handle<v8::Value> argv[], bool& callbackReturnValue)
+{
+    v8::Handle<v8::Value> returnValue;
+    bool result = invokeCallbackHelper(callback, argc, argv, returnValue);
+    callbackReturnValue = !returnValue.IsEmpty() && returnValue->IsBoolean() && returnValue->BooleanValue();
+    return result;
+}
+
+bool invokeCallbackTreatOnlyExplicitFalseAsFalse(v8::Persistent<v8::Object> callback, int argc, v8::Handle<v8::Value> argv[], bool& callbackReturnValue)
+{
+    v8::Handle<v8::Value> returnValue;
+    bool result = invokeCallbackHelper(callback, argc, argv, returnValue);
+    callbackReturnValue = !returnValue.IsEmpty() && !returnValue->IsFalse();
+    return result;
+}
+
 } // namespace WebCore
index 586296b7006d003d76f55a5b4dd54315b55673d3..6b7b3e85da8f03d27f65dad565109139067413a0 100644 (file)
@@ -60,6 +60,7 @@ private:
 
 // Returns false if callback failed (null, wrong type, or threw exception).
 bool invokeCallback(v8::Persistent<v8::Object> callback, int argc, v8::Handle<v8::Value> argv[], bool& callbackReturnValue);
+bool invokeCallbackTreatOnlyExplicitFalseAsFalse(v8::Persistent<v8::Object> callback, int argc, v8::Handle<v8::Value> argv[], bool& callbackReturnValue);
 
 } // namespace WebCore