Manage WebSQL callbacks with OwnPtr instead of refcounting

Source/modules/webdatabase/Database.cpp

Index: Source/modules/webdatabase/Database.cpp
diff --git a/Source/modules/webdatabase/Database.cpp b/Source/modules/webdatabase/Database.cpp
index ce79895b1445a21d56a4981089adfbc7bc3ee9a3..27305315792cd0a0aef1e5140a884ead7b9c30c3 100644
--- a/Source/modules/webdatabase/Database.cpp
+++ b/Source/modules/webdatabase/Database.cpp
@@ -140,38 +140,38 @@ void Database::closeImmediately()
     }
 }
 
-void Database::changeVersion(const String& oldVersion, const String& newVersion, PassRefPtr<SQLTransactionCallback> callback, PassRefPtr<SQLTransactionErrorCallback> errorCallback, PassRefPtr<SQLVoidCallback> successCallback)
+void Database::changeVersion(const String& oldVersion, const String& newVersion, PassOwnPtr<SQLTransactionCallback> callback, PassOwnPtr<SQLTransactionErrorCallback> errorCallback, PassOwnPtr<SQLVoidCallback> successCallback)
 {
     ChangeVersionData data(oldVersion, newVersion);
     runTransaction(callback, errorCallback, successCallback, false, &data);
 }
 
-void Database::transaction(PassRefPtr<SQLTransactionCallback> callback, PassRefPtr<SQLTransactionErrorCallback> errorCallback, PassRefPtr<SQLVoidCallback> successCallback)
+void Database::transaction(PassOwnPtr<SQLTransactionCallback> callback, PassOwnPtr<SQLTransactionErrorCallback> errorCallback, PassOwnPtr<SQLVoidCallback> successCallback)
 {
     runTransaction(callback, errorCallback, successCallback, false);
 }
 
-void Database::readTransaction(PassRefPtr<SQLTransactionCallback> callback, PassRefPtr<SQLTransactionErrorCallback> errorCallback, PassRefPtr<SQLVoidCallback> successCallback)
+void Database::readTransaction(PassOwnPtr<SQLTransactionCallback> callback, PassOwnPtr<SQLTransactionErrorCallback> errorCallback, PassOwnPtr<SQLVoidCallback> successCallback)
 {
     runTransaction(callback, errorCallback, successCallback, true);
 }
 
-static void callTransactionErrorCallback(ExecutionContext*, PassRefPtr<SQLTransactionErrorCallback> callback, PassRefPtr<SQLError> error)
+static void callTransactionErrorCallback(ExecutionContext*, PassOwnPtr<SQLTransactionErrorCallback> callback, PassRefPtr<SQLError> error)
 {
     callback->handleEvent(error.get());
 }
 
-void Database::runTransaction(PassRefPtr<SQLTransactionCallback> callback, PassRefPtr<SQLTransactionErrorCallback> errorCallback,
-    PassRefPtr<SQLVoidCallback> successCallback, bool readOnly, const ChangeVersionData* changeVersionData)
+void Database::runTransaction(PassOwnPtr<SQLTransactionCallback> callback, PassOwnPtr<SQLTransactionErrorCallback> errorCallback,
+    PassOwnPtr<SQLVoidCallback> successCallback, bool readOnly, const ChangeVersionData* changeVersionData)
 {
-    RefPtr<SQLTransactionErrorCallback> anotherRefToErrorCallback = errorCallback;
-    RefPtr<SQLTransaction> transaction = SQLTransaction::create(this, callback, successCallback, anotherRefToErrorCallback, readOnly);
-
+    RefPtr<SQLTransaction> transaction = SQLTransaction::create(this, callback, successCallback, errorCallback, readOnly);
     RefPtr<SQLTransactionBackend> transactionBackend;
-    transactionBackend = backend()->runTransaction(transaction.release(), readOnly, changeVersionData);
-    if (!transactionBackend && anotherRefToErrorCallback) {
-        RefPtr<SQLError> error = SQLError::create(SQLError::UNKNOWN_ERR, "database has been closed");
-        executionContext()->postTask(createCallbackTask(&callTransactionErrorCallback, anotherRefToErrorCallback, error.release()));
+    transactionBackend = backend()->runTransaction(transaction, readOnly, changeVersionData);

[abarth-chromium, 2013/12/04 01:55:50]

Combine this line with the previous one?

[adamk, 2013/12/04 02:08:26]

Done.

+    if (!transactionBackend) {
+        if (OwnPtr<SQLTransactionErrorCallback> callback = transaction->releaseErrorCallback()) {

[abarth-chromium, 2013/12/04 01:55:50]

Will this always be non-zero if errorCallback was non-zero to start with?  That
was true in the old world.  Can we add an ASSERT to that effect?

[adamk, 2013/12/04 02:08:26]

Added an ASSERT, but it's ugly. Also added a FIXME; I think this whole function
should probably live in DatabaseBackend, but I can't figure out what that class
is even for (it's actually |this|).

+            RefPtr<SQLError> error = SQLError::create(SQLError::UNKNOWN_ERR, "database has been closed");
+            executionContext()->postTask(createCallbackTask(&callTransactionErrorCallback, callback.release(), error.release()));
+        }
     }
 }